From 2951408c41202106bc66297c1c056c2d08e60e3a Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 6 Jan 2022 20:50:36 +0100 Subject: [PATCH 1/5] partition nodes as soon as possible Time to write another story on `@apply`... When we write code like this: ```css .a { @apply b; } .b { @apply uppercase; color: red; } ``` Then we create 2 Nodes in our context to keep track of. One has identifier `a`, the other has identifier `b`. However, when we have an `@apply` and it contains multiple declarations/atrules, then we have to split up the (aka partition) node into multiple nodes so that we can guarantee the correct expected sort order. This means that the above example technically looks like this: ```css .a { @apply b; } .b { @apply uppercase; } .b { color: red; } ``` If this was your input, then we would still have 1 node for identifier 'a', but we would have 2 nodes for identifier 'b'. As mentioned earlier, this is important to guarantee the correct order, here is an example: ```css .b { @apply md:font-bold xl:font-normal; /* Here we can sort by our internal rules. This means that the `md` comes before `xl`. */ } ``` ... however ```css .b { @apply xl:font-normal; /* This now exists _before_ the example below */ } .b { @apply md:font-bold; /* Because we respect the order of the user's css */ } ``` So to guarantee the order when doing this: ```css .b { @apply xl:font-normal; @apply lg:font-normal; } ``` We also split this up into 2 nodes like this: ```css .b { @apply xl:font-normal; } .b { @apply lg:font-normal; } ``` The tricky part is that now only 1 empty `.b` node exists in our context because we partitioned the orginal node into multiple nodes and moved the children to the new nodes and because they are new nodes it means that they have a different identity. This partitioning used to happen in the expandApplyAtRules code, but this is a bit too late because the context has already been filled at this time. Instead, we move the code more to the front, as if you wrote those separated blocks yourself. Now the code to inject those nodes into the context happens in a single spot instead of multiple places. Another good part about this is that we have better consistency between each layer because it turns out that these two examples generated different results... ```css .a { @apply b; } .b { @apply uppercase; color: red; } ``` ... is different compared to: ```css @tailwind components; @layer components { .a { @apply b; } .b { @apply uppercase; color: red; } } ``` Even if both `a` and `b` are being used in one of your content paths... Yeah.. *sigh* --- src/lib/expandApplyAtRules.js | 42 ----------------- src/lib/setupContextUtils.js | 82 ++++++++++++++++++++++++++++++---- src/processTailwindFeatures.js | 3 +- 3 files changed, 76 insertions(+), 51 deletions(-) diff --git a/src/lib/expandApplyAtRules.js b/src/lib/expandApplyAtRules.js index 579f80593dac..d9db3d0123bf 100644 --- a/src/lib/expandApplyAtRules.js +++ b/src/lib/expandApplyAtRules.js @@ -72,47 +72,6 @@ function extractApplyCandidates(params) { return [candidates, false] } -function partitionApplyParents(root) { - let applyParents = new Set() - - root.walkAtRules('apply', (rule) => { - applyParents.add(rule.parent) - }) - - for (let rule of applyParents) { - let nodeGroups = [] - let lastGroup = [] - - for (let node of rule.nodes) { - if (node.type === 'atrule' && node.name === 'apply') { - if (lastGroup.length > 0) { - nodeGroups.push(lastGroup) - lastGroup = [] - } - nodeGroups.push([node]) - } else { - lastGroup.push(node) - } - } - - if (lastGroup.length > 0) { - nodeGroups.push(lastGroup) - } - - if (nodeGroups.length === 1) { - continue - } - - for (let group of [...nodeGroups].reverse()) { - let newParent = rule.clone({ nodes: [] }) - newParent.append(group) - rule.after(newParent) - } - - rule.remove() - } -} - function processApply(root, context) { let applyCandidates = new Set() @@ -343,7 +302,6 @@ function processApply(root, context) { export default function expandApplyAtRules(context) { return (root) => { - partitionApplyParents(root) processApply(root, context) } } diff --git a/src/lib/setupContextUtils.js b/src/lib/setupContextUtils.js index f86f5e3d31d9..c979c8df7fa0 100644 --- a/src/lib/setupContextUtils.js +++ b/src/lib/setupContextUtils.js @@ -20,6 +20,58 @@ import log from '../util/log' import negateValue from '../util/negateValue' import isValidArbitraryValue from '../util/isValidArbitraryValue' +function partitionRules(root) { + if (!root.walkAtRules) return [root] + + let applyParents = new Set() + let rules = [] + + root.walkAtRules('apply', (rule) => { + applyParents.add(rule.parent) + }) + + if (applyParents.size === 0) { + rules.push(root) + } + + for (let rule of applyParents) { + let nodeGroups = [] + let lastGroup = [] + + for (let node of rule.nodes) { + if (node.type === 'atrule' && node.name === 'apply') { + if (lastGroup.length > 0) { + nodeGroups.push(lastGroup) + lastGroup = [] + } + nodeGroups.push([node]) + } else { + lastGroup.push(node) + } + } + + if (lastGroup.length > 0) { + nodeGroups.push(lastGroup) + } + + if (nodeGroups.length === 1) { + rules.push(rule) + continue + } + + for (let group of [...nodeGroups].reverse()) { + let clone = rule.clone({ nodes: [] }) + clone.append(group) + rules.unshift(clone) + rule.after(clone) + } + + rule.remove() + } + + return rules +} + function parseVariantFormatString(input) { if (input.includes('{')) { if (!isBalanced(input)) throw new Error(`Your { and } are unbalanced.`) @@ -232,7 +284,9 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs context.candidateRuleMap.set(identifier, []) } - context.candidateRuleMap.get(identifier).push([{ sort: offset, layer: 'user' }, rule]) + context.candidateRuleMap + .get(identifier) + .push(...partitionRules(rule).map((rule) => [{ sort: offset, layer: 'user' }, rule])) } }, addBase(base) { @@ -246,7 +300,7 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs context.candidateRuleMap .get(prefixedIdentifier) - .push([{ sort: offset, layer: 'base' }, rule]) + .push(...partitionRules(rule).map((rule) => [{ sort: offset, layer: 'base' }, rule])) } }, /** @@ -260,7 +314,6 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs for (let [identifier, rule] of withIdentifiers(groups)) { let prefixedIdentifier = prefixIdentifier(identifier, {}) - let offset = offsets.base++ if (!context.candidateRuleMap.has(prefixedIdentifier)) { context.candidateRuleMap.set(prefixedIdentifier, []) @@ -268,7 +321,12 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs context.candidateRuleMap .get(prefixedIdentifier) - .push([{ sort: offset, layer: 'defaults' }, rule]) + .push( + ...partitionRules(rule).map((rule) => [ + { sort: offsets.base++, layer: 'defaults' }, + rule, + ]) + ) } }, addComponents(components, options) { @@ -281,7 +339,6 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs for (let [identifier, rule] of withIdentifiers(components)) { let prefixedIdentifier = prefixIdentifier(identifier, options) - let offset = offsets.components++ classList.add(prefixedIdentifier) @@ -291,7 +348,12 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs context.candidateRuleMap .get(prefixedIdentifier) - .push([{ sort: offset, layer: 'components', options }, rule]) + .push( + ...partitionRules(rule).map((rule) => [ + { sort: offsets.components++, layer: 'components', options }, + rule, + ]) + ) } }, addUtilities(utilities, options) { @@ -304,7 +366,6 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs for (let [identifier, rule] of withIdentifiers(utilities)) { let prefixedIdentifier = prefixIdentifier(identifier, options) - let offset = offsets.utilities++ classList.add(prefixedIdentifier) @@ -314,7 +375,12 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs context.candidateRuleMap .get(prefixedIdentifier) - .push([{ sort: offset, layer: 'utilities', options }, rule]) + .push( + ...partitionRules(rule).map((rule) => [ + { sort: offsets.utilities++, layer: 'utilities', options }, + rule, + ]) + ) } }, matchUtilities: function (utilities, options) { diff --git a/src/processTailwindFeatures.js b/src/processTailwindFeatures.js index 7c622205b623..d9bce8cc20a1 100644 --- a/src/processTailwindFeatures.js +++ b/src/processTailwindFeatures.js @@ -14,6 +14,8 @@ export default function processTailwindFeatures(setupContext) { return function (root, result) { let { tailwindDirectives, applyDirectives } = normalizeTailwindDirectives(root) + detectNesting()(root, result) + let context = setupContext({ tailwindDirectives, applyDirectives, @@ -37,7 +39,6 @@ export default function processTailwindFeatures(setupContext) { issueFlagNotices(context.tailwindConfig) - detectNesting(context)(root, result) expandTailwindAtRules(context)(root, result) expandApplyAtRules(context)(root, result) evaluateTailwindFunctions(context)(root, result) From af5468be1aa2c1d4524ac42447bd34777e1acab8 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 6 Jan 2022 21:04:51 +0100 Subject: [PATCH 2/5] add more `@apply` related tests --- tests/apply.test.js | 190 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 190 insertions(+) diff --git a/tests/apply.test.js b/tests/apply.test.js index 31b19bfa937e..e5f15f36a9a2 100644 --- a/tests/apply.test.js +++ b/tests/apply.test.js @@ -910,6 +910,196 @@ it('should be possible to apply a class from another rule with multiple selector }) }) +describe('multiple instances', () => { + it('should be possible to apply multiple "instances" of the same class', () => { + let config = { + content: [{ raw: html`` }], + plugins: [], + corePlugins: { preflight: false }, + } + + let input = css` + .a { + @apply b; + } + + .b { + @apply uppercase; + } + + .b { + color: red; + } + ` + + return run(input, config).then((result) => { + return expect(result.css).toMatchFormattedCss(css` + .a { + text-transform: uppercase; + color: red; + } + + .b { + text-transform: uppercase; + color: red; + } + `) + }) + }) + + it('should be possible to apply a combination of multiple "instances" of the same class', () => { + let config = { + content: [{ raw: html`` }], + plugins: [], + corePlugins: { preflight: false }, + } + + let input = css` + .a { + @apply b; + } + + .b { + @apply uppercase; + color: red; + } + ` + + return run(input, config).then((result) => { + return expect(result.css).toMatchFormattedCss(css` + .a { + text-transform: uppercase; + color: red; + } + + .b { + text-transform: uppercase; + color: red; + } + `) + }) + }) + + it('should generate the same output, even if it was used in a @layer', () => { + let config = { + content: [{ raw: html`
` }], + plugins: [], + corePlugins: { preflight: false }, + } + + let input = css` + @tailwind components; + + @layer components { + .a { + @apply b; + } + + .b { + @apply uppercase; + color: red; + } + } + ` + + return run(input, config).then((result) => { + return expect(result.css).toMatchFormattedCss(css` + .a { + text-transform: uppercase; + color: red; + } + + .b { + text-transform: uppercase; + color: red; + } + `) + }) + }) + + it('should be possible to apply a combination of multiple "instances" of the same class (defined in a layer)', () => { + let config = { + content: [{ raw: html`
` }], + plugins: [], + corePlugins: { preflight: false }, + } + + let input = css` + @tailwind components; + + @layer components { + .a { + color: red; + @apply b; + color: blue; + } + + .b { + @apply text-green-500; + text-decoration: underline; + } + } + ` + + return run(input, config).then((result) => { + return expect(result.css).toMatchFormattedCss(css` + .a { + color: red; + --tw-text-opacity: 1; + color: rgb(34 197 94 / var(--tw-text-opacity)); + text-decoration: underline; + color: blue; + } + + .b { + --tw-text-opacity: 1; + color: rgb(34 197 94 / var(--tw-text-opacity)); + text-decoration: underline; + } + `) + }) + }) + + it('should properly maintain the order', () => { + let config = { + content: [{ raw: html`` }], + plugins: [], + corePlugins: { preflight: false }, + } + + let input = css` + h2 { + @apply text-xl; + @apply lg:text-3xl; + @apply sm:text-2xl; + } + ` + + return run(input, config).then((result) => { + return expect(result.css).toMatchFormattedCss(css` + h2 { + font-size: 1.25rem; + line-height: 1.75rem; + } + + @media (min-width: 1024px) { + h2 { + font-size: 1.875rem; + line-height: 2.25rem; + } + } + + @media (min-width: 640px) { + h2 { + font-size: 1.5rem; + line-height: 2rem; + } + } + `) + }) + }) +}) + /* it('apply can emit defaults in isolated environments without @tailwind directives', () => { let config = { From 9593d5386cad9db9b4318f451fe76e3f913a4417 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 6 Jan 2022 21:12:43 +0100 Subject: [PATCH 3/5] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 110353936615..b4e51d0c17bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Allow use of falsy values in theme config ([#6917](https://github.com/tailwindlabs/tailwindcss/pull/6917)) - Ensure we can apply classes that are grouped with non-class selectors ([#6922](https://github.com/tailwindlabs/tailwindcss/pull/6922)) - Improve standalone CLI compatibility on Linux by switching to the `linuxstatic` build target ([#6914](https://github.com/tailwindlabs/tailwindcss/pull/6914)) +- Ensure `@apply` works consistently with or without `@layer` ([#6938](https://github.com/tailwindlabs/tailwindcss/pull/6938)) ## [3.0.11] - 2022-01-05 From 6983b89f457f4f35041806c6e1e29f298e2b87b7 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 7 Jan 2022 15:57:30 +0100 Subject: [PATCH 4/5] remove support for basic nesting (leftover) --- tests/apply.test.css | 8 -------- tests/apply.test.js | 5 ----- 2 files changed, 13 deletions(-) diff --git a/tests/apply.test.css b/tests/apply.test.css index e79f201b3fd9..968b43b5f706 100644 --- a/tests/apply.test.css +++ b/tests/apply.test.css @@ -144,14 +144,6 @@ --tw-numeric-fraction: diagonal-fractions; font-variant-numeric: var(--tw-font-variant-numeric); } -.basic-nesting-parent { - .basic-nesting-child { - font-weight: 700; - } - .basic-nesting-child:hover { - font-weight: 400; - } -} .use-base-only-a { font-weight: 700; } diff --git a/tests/apply.test.js b/tests/apply.test.js index e5f15f36a9a2..d281671e624c 100644 --- a/tests/apply.test.js +++ b/tests/apply.test.js @@ -56,11 +56,6 @@ test('@apply', () => { .complex-utilities { @apply ordinal tabular-nums focus:diagonal-fractions shadow-lg hover:shadow-xl; } - .basic-nesting-parent { - .basic-nesting-child { - @apply font-bold hover:font-normal; - } - } .use-base-only-a { @apply font-bold; } From 156052905fc6ea1d9f2c0f1238f355417aa03437 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 7 Jan 2022 16:10:54 +0100 Subject: [PATCH 5/5] remove leftover todo This has been fixed already --- tests/apply.test.css | 1 - tests/apply.test.js | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/apply.test.css b/tests/apply.test.css index 968b43b5f706..4be1b2c78f62 100644 --- a/tests/apply.test.css +++ b/tests/apply.test.css @@ -122,7 +122,6 @@ text-align: left; } } -/* TODO: This works but the generated CSS is unnecessarily verbose. */ .complex-utilities { --tw-ordinal: ordinal; --tw-numeric-spacing: tabular-nums; diff --git a/tests/apply.test.js b/tests/apply.test.js index d281671e624c..da2cf0ba815c 100644 --- a/tests/apply.test.js +++ b/tests/apply.test.js @@ -52,7 +52,6 @@ test('@apply', () => { .selectors-group { @apply group-hover:text-center lg:group-hover:text-left; } - /* TODO: This works but the generated CSS is unnecessarily verbose. */ .complex-utilities { @apply ordinal tabular-nums focus:diagonal-fractions shadow-lg hover:shadow-xl; }