From d6bec499343146ae2468a71cfa77af5301de251d Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Fri, 9 Sep 2022 13:12:43 -0400 Subject: [PATCH] Fix parallel variant ordering clash (#9282) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove remnants of the user layer It hasn’t been used in a while * Rewrite sort offset generation * wip * wip wip * Handle parasite utilities * wip * wip * Make parallel variants sorting more resillient It’s not perfect but it’s close * fix * remove todo it adds a new bit so it can’t * Simplify getClassOrder usage * Simplify oops oops * Add parasite utility for `dark` dark mode class name * Cleanup * Cleanup * Simplify * format files * Fix prettier plugin to use git build of Tailwind CSS Symlink and build instead of adding a recursive dev dependency It breaks node < 16 * Fix prettier error * wip * fix test * Update changelog Co-authored-by: Robin Malfait --- .github/workflows/nodejs.yml | 7 + CHANGELOG.md | 1 + src/lib/expandApplyAtRules.js | 16 +- src/lib/expandTailwindAtRules.js | 48 +----- src/lib/generateRules.js | 20 ++- src/lib/offsets.js | 239 +++++++++++++++++++++++++++++ src/lib/setupContextUtils.js | 107 ++++++------- tests/arbitrary-properties.test.js | 8 +- tests/arbitrary-variants.test.js | 2 +- tests/prefers-contrast.test.js | 2 +- 10 files changed, 321 insertions(+), 129 deletions(-) create mode 100644 src/lib/offsets.js diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index abb81dad4a8a..c00672f686de 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -41,6 +41,13 @@ jobs: env: CI: true + - name: Link and build Tailwind CSS + run: | + npm run swcify + ln -nfs `pwd` node_modules/tailwindcss + env: + CI: true + - name: Test run: npm test env: diff --git a/CHANGELOG.md b/CHANGELOG.md index 003147be00c0..afce0c45970a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix `fontFamily` config TypeScript types ([#9214](https://github.com/tailwindlabs/tailwindcss/pull/9214)) - Handle variants on complex selector utilities ([#9262](https://github.com/tailwindlabs/tailwindcss/pull/9262)) - Don't mutate shared config objects ([#9294](https://github.com/tailwindlabs/tailwindcss/pull/9294)) +- Fix ordering of parallel variants ([#9282](https://github.com/tailwindlabs/tailwindcss/pull/9282)) ## [3.1.8] - 2022-08-05 diff --git a/src/lib/expandApplyAtRules.js b/src/lib/expandApplyAtRules.js index 7c4d156e06e7..7dad021f0d82 100644 --- a/src/lib/expandApplyAtRules.js +++ b/src/lib/expandApplyAtRules.js @@ -2,7 +2,6 @@ import postcss from 'postcss' import parser from 'postcss-selector-parser' import { resolveMatches } from './generateRules' -import bigSign from '../util/bigSign' import escapeClassName from '../util/escapeClassName' /** @typedef {Map} ApplyCache */ @@ -149,9 +148,7 @@ function buildLocalApplyCache(root, context) { /** @type {ApplyCache} */ let cache = new Map() - let highestOffset = context.layerOrder.user >> 4n - - root.walkRules((rule, idx) => { + root.walkRules((rule) => { // Ignore rules generated by Tailwind for (let node of pathToRoot(rule)) { if (node.raws.tailwind?.layer !== undefined) { @@ -161,6 +158,7 @@ function buildLocalApplyCache(root, context) { // Clone what's required to represent this singular rule in the tree let container = nestedClone(rule) + let sort = context.offsets.create('user') for (let className of extractClasses(rule)) { let list = cache.get(className) || [] @@ -169,7 +167,7 @@ function buildLocalApplyCache(root, context) { list.push([ { layer: 'user', - sort: BigInt(idx) + highestOffset, + sort, important: false, }, container, @@ -553,16 +551,12 @@ function processApply(root, context, localCache) { } // Insert it - siblings.push([ - // Ensure that when we are sorting, that we take the layer order into account - { ...meta, sort: meta.sort | context.layerOrder[meta.layer] }, - root.nodes[0], - ]) + siblings.push([meta.sort, root.nodes[0]]) } } // Inject the rules, sorted, correctly - let nodes = siblings.sort(([a], [z]) => bigSign(a.sort - z.sort)).map((s) => s[1]) + let nodes = context.offsets.sort(siblings).map((s) => s[1]) // `parent` refers to the node at `.abc` in: .abc { @apply mt-2 } parent.after(nodes) diff --git a/src/lib/expandTailwindAtRules.js b/src/lib/expandTailwindAtRules.js index 30b470800626..040f713b0e62 100644 --- a/src/lib/expandTailwindAtRules.js +++ b/src/lib/expandTailwindAtRules.js @@ -1,7 +1,6 @@ import LRU from 'quick-lru' import * as sharedState from './sharedState' import { generateRules } from './generateRules' -import bigSign from '../util/bigSign' import log from '../util/log' import cloneNodes from '../util/cloneNodes' import { defaultExtractor } from './defaultExtractor' @@ -74,8 +73,13 @@ function getClassCandidates(content, extractor, candidates, seen) { } } +/** + * + * @param {[import('./offsets.js').RuleOffset, import('postcss').Node][]} rules + * @param {*} context + */ function buildStylesheet(rules, context) { - let sortedRules = rules.sort(([a], [z]) => bigSign(a - z)) + let sortedRules = context.offsets.sort(rules) let returnValue = { base: new Set(), @@ -83,48 +87,10 @@ function buildStylesheet(rules, context) { components: new Set(), utilities: new Set(), variants: new Set(), - - // All the CSS that is not Tailwind related can be put in this bucket. This - // will make it easier to later use this information when we want to - // `@apply` for example. The main reason we do this here is because we - // still need to make sure the order is correct. Last but not least, we - // will make sure to always re-inject this section into the css, even if - // certain rules were not used. This means that it will look like a no-op - // from the user's perspective, but we gathered all the useful information - // we need. - user: new Set(), } for (let [sort, rule] of sortedRules) { - if (sort >= context.minimumScreen) { - returnValue.variants.add(rule) - continue - } - - if (sort & context.layerOrder.base) { - returnValue.base.add(rule) - continue - } - - if (sort & context.layerOrder.defaults) { - returnValue.defaults.add(rule) - continue - } - - if (sort & context.layerOrder.components) { - returnValue.components.add(rule) - continue - } - - if (sort & context.layerOrder.utilities) { - returnValue.utilities.add(rule) - continue - } - - if (sort & context.layerOrder.user) { - returnValue.user.add(rule) - continue - } + returnValue[sort.layer].add(rule) } return returnValue diff --git a/src/lib/generateRules.js b/src/lib/generateRules.js index fa07238ad402..558cd47ebfa3 100644 --- a/src/lib/generateRules.js +++ b/src/lib/generateRules.js @@ -146,9 +146,9 @@ function applyVariant(variant, matches, context) { let fn = parseVariant(selector) - let sort = Array.from(context.variantOrder.values()).pop() << 1n + let sort = context.offsets.recordVariant(variant) + context.variantMap.set(variant, [[sort, fn]]) - context.variantOrder.set(variant, sort) } if (context.variantMap.has(variant)) { @@ -227,11 +227,7 @@ function applyVariant(variant, matches, context) { // where you keep handling jobs until everything is done and each job can queue more // jobs if needed. variantFunctionTuples.push([ - // TODO: This could have potential bugs if we shift the sort order from variant A far - // enough into the sort space of variant B. The chances are low, but if this happens - // then this might be the place too look at. One potential solution to this problem is - // reserving additional X places for these 'unknown' variants in between. - variantSort | BigInt(idx << ruleWithVariant.length), + context.offsets.applyParallelOffset(variantSort, idx), variantFunction, // If the clone has been modified we have to pass that back @@ -298,7 +294,7 @@ function applyVariant(variant, matches, context) { let withOffset = [ { ...meta, - sort: variantSort | meta.sort, + sort: context.offsets.applyVariantOffset(meta.sort, variantSort), collectedFormats: (meta.collectedFormats ?? []).concat(collectedFormats), isArbitraryVariant: isArbitraryValue(variant), }, @@ -409,9 +405,11 @@ function extractArbitraryProperty(classCandidate, context) { return null } + let sort = context.offsets.arbitraryProperty() + return [ [ - { sort: context.arbitraryPropertiesSort, layer: 'utilities' }, + { sort, layer: 'utilities' }, () => ({ [asClass(classCandidate)]: { [property]: normalized, @@ -704,7 +702,7 @@ function generateRules(candidates, context) { context.candidateRuleCache.set(candidate, rules) for (const match of matches) { - let [{ sort, layer, options }, rule] = match + let [{ sort, options }, rule] = match if (options.respectImportant && strategy) { let container = postcss.root({ nodes: [rule.clone()] }) @@ -712,7 +710,7 @@ function generateRules(candidates, context) { rule = container.nodes[0] } - let newEntry = [sort | context.layerOrder[layer], rule] + let newEntry = [sort, rule] rules.add(newEntry) context.ruleCache.add(newEntry) allRules.push(newEntry) diff --git a/src/lib/offsets.js b/src/lib/offsets.js new file mode 100644 index 000000000000..1aea30a698f9 --- /dev/null +++ b/src/lib/offsets.js @@ -0,0 +1,239 @@ +// @ts-check + +import bigSign from '../util/bigSign' + +/** + * @typedef {'base' | 'defaults' | 'components' | 'utilities' | 'variants' | 'user'} Layer + */ + +/** + * @typedef {object} RuleOffset + * @property {Layer} layer The layer that this rule belongs to + * @property {Layer} parentLayer The layer that this rule originally belonged to. Only different from layer if this is a variant. + * @property {bigint} arbitrary 0n if false, 1n if true + * @property {bigint} variants Dynamic size. 1 bit per registered variant. 0n means no variants + * @property {bigint} parallelIndex Rule index for the parallel variant. 0 if not applicable. + * @property {bigint} index Index of the rule / utility in it's given *parent* layer. Monotonically increasing. + */ + +export class Offsets { + constructor() { + /** + * Offsets for the next rule in a given layer + * + * @type {Record} + */ + this.offsets = { + defaults: 0n, + base: 0n, + components: 0n, + utilities: 0n, + variants: 0n, + user: 0n, + } + + /** + * Positions for a given layer + * + * @type {Record} + */ + this.layerPositions = { + defaults: 0n, + base: 1n, + components: 2n, + utilities: 3n, + + // There isn't technically a "user" layer, but we need to give it a position + // Because it's used for ordering user-css from @apply + user: 4n, + + variants: 5n, + } + + /** + * The total number of functions currently registered across all variants (including arbitrary variants) + * + * @type {bigint} + */ + this.reservedVariantBits = 0n + + /** + * Positions for a given variant + * + * @type {Map} + */ + this.variantOffsets = new Map() + } + + /** + * @param {Layer} layer + * @returns {RuleOffset} + */ + create(layer) { + return { + layer, + parentLayer: layer, + arbitrary: 0n, + variants: 0n, + parallelIndex: 0n, + index: this.offsets[layer]++, + } + } + + /** + * @returns {RuleOffset} + */ + arbitraryProperty() { + return { + ...this.create('utilities'), + arbitrary: 1n, + } + } + + /** + * Get the offset for a variant + * + * @param {string} variant + * @param {number} index + * @returns {RuleOffset} + */ + forVariant(variant, index = 0) { + let offset = this.variantOffsets.get(variant) + if (offset === undefined) { + throw new Error(`Cannot find offset for unknown variant ${variant}`) + } + + return { + ...this.create('variants'), + variants: offset << BigInt(index), + } + } + + /** + * @param {RuleOffset} rule + * @param {RuleOffset} variant + * @returns {RuleOffset} + */ + applyVariantOffset(rule, variant) { + return { + ...rule, + layer: 'variants', + parentLayer: rule.layer === 'variants' ? rule.parentLayer : rule.layer, + variants: rule.variants | variant.variants, + + // TODO: Technically this is wrong. We should be handling parallel index on a per variant basis. + // We'll take the max of all the parallel indexes for now. + // @ts-ignore + parallelIndex: max([rule.parallelIndex, variant.parallelIndex]), + } + } + + /** + * @param {RuleOffset} offset + * @param {number} parallelIndex + * @returns {RuleOffset} + */ + applyParallelOffset(offset, parallelIndex) { + return { + ...offset, + parallelIndex: BigInt(parallelIndex), + } + } + + /** + * Each variant gets 1 bit per function / rule registered. + * This is because multiple variants can be applied to a single rule and we need to know which ones are present and which ones are not. + * Additionally, every unique group of variants is grouped together in the stylesheet. + * + * This grouping is order-independent. For instance, we do not differentiate between `hover:focus` and `focus:hover`. + * + * @param {string[]} variants + * @param {(name: string) => number} getLength + */ + recordVariants(variants, getLength) { + for (const variant of variants) { + this.recordVariant(variant, getLength(variant)) + } + } + + /** + * The same as `recordVariants` but for a single arbitrary variant at runtime. + * @param {string} variant + * @param {number} fnCount + * + * @returns {RuleOffset} The highest offset for this variant + */ + recordVariant(variant, fnCount = 1) { + this.variantOffsets.set(variant, 1n << this.reservedVariantBits) + + // Ensure space is reserved for each "function" in the parallel variant + // by offsetting the next variant by the number of parallel variants + // in the one we just added. + + // Single functions that return parallel variants are NOT handled separately here + // They're offset by 1 (or the number of functions) as usual + // And each rule returned is tracked separately since the functions are evaluated lazily. + // @see `RuleOffset.parallelIndex` + this.reservedVariantBits += BigInt(fnCount) + + return { + ...this.create('variants'), + variants: 1n << this.reservedVariantBits, + } + } + + /** + * @param {RuleOffset} a + * @param {RuleOffset} b + * @returns {bigint} + */ + compare(a, b) { + // Sort layers together + if (a.layer !== b.layer) { + return this.layerPositions[a.layer] - this.layerPositions[b.layer] + } + + // Sort variants in the order they were registered + if (a.variants !== b.variants) { + return a.variants - b.variants + } + + // Make sure each rule returned by a parallel variant is sorted in ascending order + if (a.parallelIndex !== b.parallelIndex) { + return a.parallelIndex - b.parallelIndex + } + + // Always sort arbitrary properties after other utilities + if (a.arbitrary !== b.arbitrary) { + return a.arbitrary - b.arbitrary + } + + // Sort utilities, components, etc… in the order they were registered + return a.index - b.index + } + + /** + * @template T + * @param {[RuleOffset, T][]} list + * @returns {[RuleOffset, T][]} + */ + sort(list) { + return list.sort(([a], [b]) => bigSign(this.compare(a, b))) + } +} + +/** + * + * @param {bigint[]} nums + * @returns {bigint|null} + */ +function max(nums) { + let max = null + + for (const num of nums) { + max = max ?? num + max = max > num ? max : num + } + + return max +} diff --git a/src/lib/setupContextUtils.js b/src/lib/setupContextUtils.js index 1fd72e301c4e..e8519ad95f9f 100644 --- a/src/lib/setupContextUtils.js +++ b/src/lib/setupContextUtils.js @@ -12,7 +12,6 @@ import isPlainObject from '../util/isPlainObject' import escapeClassName from '../util/escapeClassName' import nameClass, { formatClass } from '../util/nameClass' import { coerceValue } from '../util/pluginUtils' -import bigSign from '../util/bigSign' import { variantPlugins, corePlugins } from '../corePlugins' import * as sharedState from './sharedState' import { env } from './sharedState' @@ -22,6 +21,7 @@ import negateValue from '../util/negateValue' import isValidArbitraryValue from '../util/isValidArbitraryValue' import { generateRules } from './generateRules' import { hasContentChanged } from './cacheInvalidation.js' +import { Offsets } from './offsets.js' let MATCH_VARIANT = Symbol() @@ -201,6 +201,13 @@ export function parseVariant(variant) { } } +/** + * + * @param {any} tailwindConfig + * @param {any} context + * @param {object} param2 + * @param {Offsets} param2.offsets + */ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offsets, classList }) { function getConfigValue(path, defaultValue) { return path ? dlv(tailwindConfig, path, defaultValue) : tailwindConfig @@ -255,7 +262,7 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs addBase(base) { for (let [identifier, rule] of withIdentifiers(base)) { let prefixedIdentifier = prefixIdentifier(identifier, {}) - let offset = offsets.base++ + let offset = offsets.create('base') if (!context.candidateRuleMap.has(prefixedIdentifier)) { context.candidateRuleMap.set(prefixedIdentifier, []) @@ -284,7 +291,7 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs context.candidateRuleMap .get(prefixedIdentifier) - .push([{ sort: offsets.base++, layer: 'defaults' }, rule]) + .push([{ sort: offsets.create('defaults'), layer: 'defaults' }, rule]) } }, addComponents(components, options) { @@ -307,7 +314,7 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs context.candidateRuleMap .get(prefixedIdentifier) - .push([{ sort: offsets.components++, layer: 'components', options }, rule]) + .push([{ sort: offsets.create('components'), layer: 'components', options }, rule]) } }, addUtilities(utilities, options) { @@ -330,7 +337,7 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs context.candidateRuleMap .get(prefixedIdentifier) - .push([{ sort: offsets.utilities++, layer: 'utilities', options }, rule]) + .push([{ sort: offsets.create('utilities'), layer: 'utilities', options }, rule]) } }, matchUtilities: function (utilities, options) { @@ -341,7 +348,7 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs options = { ...defaultOptions, ...options } - let offset = offsets.utilities++ + let offset = offsets.create('utilities') for (let identifier in utilities) { let prefixedIdentifier = prefixIdentifier(identifier, options) @@ -393,7 +400,7 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs options = { ...defaultOptions, ...options } - let offset = offsets.components++ + let offset = offsets.create('components') for (let identifier in components) { let prefixedIdentifier = prefixIdentifier(identifier, options) @@ -641,13 +648,10 @@ function resolvePlugins(context, root) { function registerPlugins(plugins, context) { let variantList = [] let variantMap = new Map() - let offsets = { - defaults: 0n, - base: 0n, - components: 0n, - utilities: 0n, - user: 0n, - } + context.variantMap = variantMap + + let offsets = new Offsets() + context.offsets = offsets let classList = new Set() @@ -668,49 +672,17 @@ function registerPlugins(plugins, context) { } } - let highestOffset = ((args) => args.reduce((m, e) => (e > m ? e : m)))([ - offsets.base, - offsets.defaults, - offsets.components, - offsets.utilities, - offsets.user, - ]) - let reservedBits = BigInt(highestOffset.toString(2).length) - - // A number one less than the top range of the highest offset area - // so arbitrary properties are always sorted at the end. - context.arbitraryPropertiesSort = ((1n << reservedBits) << 0n) - 1n - - context.layerOrder = { - defaults: (1n << reservedBits) << 0n, - base: (1n << reservedBits) << 1n, - components: (1n << reservedBits) << 2n, - utilities: (1n << reservedBits) << 3n, - user: (1n << reservedBits) << 4n, - } - - reservedBits += 5n - - let offset = 0 - context.variantOrder = new Map( - variantList - .map((variant, i) => { - let variantFunctions = variantMap.get(variant).length - let bits = (1n << BigInt(i + offset)) << reservedBits - offset += variantFunctions - 1 - return [variant, bits] - }) - .sort(([, a], [, z]) => bigSign(a - z)) - ) - - context.minimumScreen = [...context.variantOrder.values()].shift() + // Make sure to record bit masks for every variant + offsets.recordVariants(variantList, (variant) => variantMap.get(variant).length) // Build variantMap for (let [variantName, variantFunctions] of variantMap.entries()) { - let sort = context.variantOrder.get(variantName) context.variantMap.set( variantName, - variantFunctions.map((variantFunction, idx) => [sort << BigInt(idx), variantFunction]) + variantFunctions.map((variantFunction, idx) => [ + offsets.forVariant(variantName, idx), + variantFunction, + ]) ) } @@ -816,26 +788,41 @@ function registerPlugins(plugins, context) { } } + let darkClassName = [].concat(context.tailwindConfig.darkMode ?? 'media')[1] ?? 'dark' + // A list of utilities that are used by certain Tailwind CSS utilities but // that don't exist on their own. This will result in them "not existing" and // sorting could be weird since you still require them in order to make the - // host utitlies work properly. (Thanks Biology) - let parasiteUtilities = new Set([prefix(context, 'group'), prefix(context, 'peer')]) + // host utilities work properly. (Thanks Biology) + let parasiteUtilities = [ + prefix(context, darkClassName), + prefix(context, 'group'), + prefix(context, 'peer'), + ] context.getClassOrder = function getClassOrder(classes) { - let sortedClassNames = new Map() - for (let [sort, rule] of generateRules(new Set(classes), context)) { - if (sortedClassNames.has(rule.raws.tailwind.candidate)) continue - sortedClassNames.set(rule.raws.tailwind.candidate, sort) + // Non-util classes won't be generated, so we default them to null + let sortedClassNames = new Map(classes.map((className) => [className, null])) + + // Sort all classes in order + // Non-tailwind classes won't be generated and will be left as `null` + let rules = generateRules(new Set(classes), context) + rules = context.offsets.sort(rules) + + let idx = BigInt(parasiteUtilities.length) + + for (const [, rule] of rules) { + sortedClassNames.set(rule.raws.tailwind.candidate, idx++) } return classes.map((className) => { let order = sortedClassNames.get(className) ?? null + let parasiteIndex = parasiteUtilities.indexOf(className) - if (order === null && parasiteUtilities.has(className)) { + if (order === null && parasiteIndex !== -1) { // This will make sure that it is at the very beginning of the // `components` layer which technically means 'before any // components'. - order = context.layerOrder.components + order = BigInt(parasiteIndex) } return [className, order] diff --git a/tests/arbitrary-properties.test.js b/tests/arbitrary-properties.test.js index e3a4f02a5bce..a7d1b5bbf72d 100644 --- a/tests/arbitrary-properties.test.js +++ b/tests/arbitrary-properties.test.js @@ -324,7 +324,7 @@ it('should be possible to read theme values in arbitrary properties (without quo it('should be possible to read theme values in arbitrary properties (with quotes)', () => { let config = { - content: [{ raw: html`
` }], + content: [{ raw: html`
` }], corePlugins: { preflight: false }, } @@ -338,12 +338,12 @@ it('should be possible to read theme values in arbitrary properties (with quotes return expect(result.css).toMatchFormattedCss(css` ${defaults} - .\[--a\:theme\(\'colors\.blue\.500\'\)\] { - --a: #3b82f6; - } .\[color\:var\(--a\)\] { color: var(--a); } + .\[--a\:theme\(\'colors\.blue\.500\'\)\] { + --a: #3b82f6; + } `) }) }) diff --git a/tests/arbitrary-variants.test.js b/tests/arbitrary-variants.test.js index a145ee5b4731..c17016a5bccb 100644 --- a/tests/arbitrary-variants.test.js +++ b/tests/arbitrary-variants.test.js @@ -107,7 +107,7 @@ test('variants without & or an at-rule are ignored', () => { test('arbitrary variants are sorted after other variants', () => { let config = { - content: [{ raw: html`
` }], + content: [{ raw: html`
` }], corePlugins: { preflight: false }, } diff --git a/tests/prefers-contrast.test.js b/tests/prefers-contrast.test.js index e20e86146d3e..362096078b39 100644 --- a/tests/prefers-contrast.test.js +++ b/tests/prefers-contrast.test.js @@ -3,7 +3,7 @@ import { run, html, css, defaults } from './util/run' it('should be possible to use contrast-more and contrast-less variants', () => { let config = { content: [ - { raw: html`
` }, + { raw: html`
` }, ], corePlugins: { preflight: false }, }