From 910b6553885a28b55e061b12e617fab8988117a4 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Fri, 25 Feb 2022 08:35:22 -0500 Subject: [PATCH] Use local user css cache for apply (#7524) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix context reuse test * Don't update files with at-apply when content changes * Prevent at-apply directives from creating new contexts * Rework apply to use local postcss root We were storing user CSS in the context so we could use it with apply. The problem is that this CSS does not get updated on save unless it has a tailwind directive in it resulting in stale apply caches. This could result in either stale generation or errors about missing classes. * Don’t build local cache unless `@apply` is used * Update changelog --- CHANGELOG.md | 1 + src/lib/expandApplyAtRules.js | 175 ++++++++++++++++++++++++++++++- src/lib/expandTailwindAtRules.js | 30 +++++- src/lib/setupContextUtils.js | 20 ---- src/lib/setupTrackingContext.js | 6 +- src/util/cloneNodes.js | 9 +- tests/apply.test.js | 51 +++++++++ tests/context-reuse.test.js | 92 ++++++++++++++-- tests/context-reuse.worker.js | 47 --------- 9 files changed, 340 insertions(+), 91 deletions(-) delete mode 100644 tests/context-reuse.worker.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 788c25b68113..1b70fe9fc6c0 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 default ring color to be a function ([#7587](https://github.com/tailwindlabs/tailwindcss/pull/7587)) - Preserve source maps for generated CSS ([#7588](https://github.com/tailwindlabs/tailwindcss/pull/7588)) - Split box shadows on top-level commas only ([#7479](https://github.com/tailwindlabs/tailwindcss/pull/7479)) +- Use local user css cache for `@apply` ([#7524](https://github.com/tailwindlabs/tailwindcss/pull/7524)) ### Changed diff --git a/src/lib/expandApplyAtRules.js b/src/lib/expandApplyAtRules.js index 52ee1f96a5d7..aeaa79985a89 100644 --- a/src/lib/expandApplyAtRules.js +++ b/src/lib/expandApplyAtRules.js @@ -5,6 +5,8 @@ import { resolveMatches } from './generateRules' import bigSign from '../util/bigSign' import escapeClassName from '../util/escapeClassName' +/** @typedef {Map} ApplyCache */ + function extractClasses(node) { let classes = new Set() let container = postcss.root({ nodes: [node.clone()] }) @@ -35,6 +37,131 @@ function prefix(context, selector) { return typeof prefix === 'function' ? prefix(selector) : prefix + selector } +function* pathToRoot(node) { + yield node + while (node.parent) { + yield node.parent + node = node.parent + } +} + +/** + * Only clone the node itself and not its children + * + * @param {*} node + * @param {*} overrides + * @returns + */ +function shallowClone(node, overrides = {}) { + let children = node.nodes + node.nodes = [] + + let tmp = node.clone(overrides) + + node.nodes = children + + return tmp +} + +/** + * Clone just the nodes all the way to the top that are required to represent + * this singular rule in the tree. + * + * For example, if we have CSS like this: + * ```css + * @media (min-width: 768px) { + * @supports (display: grid) { + * .foo { + * display: grid; + * grid-template-columns: repeat(auto-fit, minmax(200px, 1fr)); + * } + * } + * + * @supports (backdrop-filter: blur(1px)) { + * .bar { + * backdrop-filter: blur(1px); + * } + * } + * + * .baz { + * color: orange; + * } + * } + * ``` + * + * And we're cloning `.bar` it'll return a cloned version of what's required for just that single node: + * + * ```css + * @media (min-width: 768px) { + * @supports (backdrop-filter: blur(1px)) { + * .bar { + * backdrop-filter: blur(1px); + * } + * } + * } + * ``` + * + * @param {import('postcss').Node} node + */ +function nestedClone(node) { + for (let parent of pathToRoot(node)) { + if (node === parent) { + continue + } + + if (parent.type === 'root') { + break + } + + node = shallowClone(parent, { + nodes: [node], + }) + } + + return node +} + +/** + * @param {import('postcss').Root} root + */ +function buildLocalApplyCache(root, context) { + /** @type {ApplyCache} */ + let cache = new Map() + + let highestOffset = context.layerOrder.user >> 4n + + root.walkRules((rule, idx) => { + // Ignore rules generated by Tailwind + for (let node of pathToRoot(rule)) { + if (node.raws.tailwind?.layer !== undefined) { + return + } + } + + // Clone what's required to represent this singular rule in the tree + let container = nestedClone(rule) + + for (let className of extractClasses(rule)) { + let list = cache.get(className) || [] + cache.set(className, list) + + list.push([ + { + layer: 'user', + sort: BigInt(idx) + highestOffset, + important: false, + }, + container, + ]) + } + }) + + return cache +} + +/** + * @returns {ApplyCache} + */ function buildApplyCache(applyCandidates, context) { for (let candidate of applyCandidates) { if (context.notClassCache.has(candidate) || context.applyClassCache.has(candidate)) { @@ -62,6 +189,43 @@ function buildApplyCache(applyCandidates, context) { return context.applyClassCache } +/** + * Build a cache only when it's first used + * + * @param {() => ApplyCache} buildCacheFn + * @returns {ApplyCache} + */ +function lazyCache(buildCacheFn) { + let cache = null + + return { + get: (name) => { + cache = cache || buildCacheFn() + + return cache.get(name) + }, + has: (name) => { + cache = cache || buildCacheFn() + + return cache.has(name) + }, + } +} + +/** + * Take a series of multiple caches and merge + * them so they act like one large cache + * + * @param {ApplyCache[]} caches + * @returns {ApplyCache} + */ +function combineCaches(caches) { + return { + get: (name) => caches.flatMap((cache) => cache.get(name) || []), + has: (name) => caches.some((cache) => cache.has(name)), + } +} + function extractApplyCandidates(params) { let candidates = params.split(/[\s\t\n]+/g) @@ -72,7 +236,7 @@ function extractApplyCandidates(params) { return [candidates, false] } -function processApply(root, context) { +function processApply(root, context, localCache) { let applyCandidates = new Set() // Collect all @apply rules and candidates @@ -90,7 +254,7 @@ function processApply(root, context) { // Start the @apply process if we have rules with @apply in them if (applies.length > 0) { // Fill up some caches! - let applyClassCache = buildApplyCache(applyCandidates, context) + let applyClassCache = combineCaches([localCache, buildApplyCache(applyCandidates, context)]) /** * When we have an apply like this: @@ -302,12 +466,15 @@ function processApply(root, context) { } // Do it again, in case we have other `@apply` rules - processApply(root, context) + processApply(root, context, localCache) } } export default function expandApplyAtRules(context) { return (root) => { - processApply(root, context) + // Build a cache of the user's CSS so we can use it to resolve classes used by @apply + let localCache = lazyCache(() => buildLocalApplyCache(root, context)) + + processApply(root, context, localCache) } } diff --git a/src/lib/expandTailwindAtRules.js b/src/lib/expandTailwindAtRules.js index 0c48e97202a0..b04cf90de6f0 100644 --- a/src/lib/expandTailwindAtRules.js +++ b/src/lib/expandTailwindAtRules.js @@ -204,17 +204,29 @@ export default function expandTailwindAtRules(context) { // Replace any Tailwind directives with generated CSS if (layerNodes.base) { - layerNodes.base.before(cloneNodes([...baseNodes, ...defaultNodes], layerNodes.base.source)) + layerNodes.base.before( + cloneNodes([...baseNodes, ...defaultNodes], layerNodes.base.source, { + layer: 'base', + }) + ) layerNodes.base.remove() } if (layerNodes.components) { - layerNodes.components.before(cloneNodes([...componentNodes], layerNodes.components.source)) + layerNodes.components.before( + cloneNodes([...componentNodes], layerNodes.components.source, { + layer: 'components', + }) + ) layerNodes.components.remove() } if (layerNodes.utilities) { - layerNodes.utilities.before(cloneNodes([...utilityNodes], layerNodes.utilities.source)) + layerNodes.utilities.before( + cloneNodes([...utilityNodes], layerNodes.utilities.source, { + layer: 'utilities', + }) + ) layerNodes.utilities.remove() } @@ -234,10 +246,18 @@ export default function expandTailwindAtRules(context) { }) if (layerNodes.variants) { - layerNodes.variants.before(cloneNodes(variantNodes, layerNodes.variants.source)) + layerNodes.variants.before( + cloneNodes(variantNodes, layerNodes.variants.source, { + layer: 'variants', + }) + ) layerNodes.variants.remove() } else if (variantNodes.length > 0) { - root.append(cloneNodes(variantNodes, root.source)) + root.append( + cloneNodes(variantNodes, root.source, { + layer: 'variants', + }) + ) } // If we've got a utility layer and no utilities are generated there's likely something wrong diff --git a/src/lib/setupContextUtils.js b/src/lib/setupContextUtils.js index c4740c46e113..e35edcc14145 100644 --- a/src/lib/setupContextUtils.js +++ b/src/lib/setupContextUtils.js @@ -230,17 +230,6 @@ function buildPluginApi(tailwindConfig, context, { variantList, variantMap, offs // Preserved for backwards compatibility but not used in v3.0+ return [] }, - addUserCss(userCss) { - for (let [identifier, rule] of withIdentifiers(userCss)) { - let offset = offsets.user++ - - if (!context.candidateRuleMap.has(identifier)) { - context.candidateRuleMap.set(identifier, []) - } - - context.candidateRuleMap.get(identifier).push([{ sort: offset, layer: 'user' }, rule]) - } - }, addBase(base) { for (let [identifier, rule] of withIdentifiers(base)) { let prefixedIdentifier = prefixIdentifier(identifier, {}) @@ -521,15 +510,6 @@ function collectLayerPlugins(root) { } }) - root.walkRules((rule) => { - // At this point it is safe to include all the left-over css from the - // user's css file. This is because the `@tailwind` and `@layer` directives - // will already be handled and will be removed from the css tree. - layerPlugins.push(function ({ addUserCss }) { - addUserCss(rule, { respectPrefix: false }) - }) - }) - return layerPlugins } diff --git a/src/lib/setupTrackingContext.js b/src/lib/setupTrackingContext.js index 49050fd3bdca..3716b0ff6235 100644 --- a/src/lib/setupTrackingContext.js +++ b/src/lib/setupTrackingContext.js @@ -112,7 +112,7 @@ function resolveChangedFiles(candidateFiles, fileModifiedMap) { // source path), or set up a new one (including setting up watchers and registering // plugins) then return it export default function setupTrackingContext(configOrPath) { - return ({ tailwindDirectives, registerDependency, applyDirectives }) => { + return ({ tailwindDirectives, registerDependency }) => { return (root, result) => { let [tailwindConfig, userConfigPath, tailwindConfigHash, configDependencies] = getTailwindConfig(configOrPath) @@ -125,7 +125,7 @@ export default function setupTrackingContext(configOrPath) { // being part of this trigger too, but it's tough because it's impossible // for a layer in one file to end up in the actual @tailwind rule in // another file since independent sources are effectively isolated. - if (tailwindDirectives.size > 0 || applyDirectives.size > 0) { + if (tailwindDirectives.size > 0) { // Add current css file as a context dependencies. contextDependencies.add(result.opts.from) @@ -153,7 +153,7 @@ export default function setupTrackingContext(configOrPath) { // We may want to think about `@layer` being part of this trigger too, but it's tough // because it's impossible for a layer in one file to end up in the actual @tailwind rule // in another file since independent sources are effectively isolated. - if (tailwindDirectives.size > 0 || applyDirectives.size > 0) { + if (tailwindDirectives.size > 0) { let fileModifiedMap = getFileModifiedMap(context) // Add template paths as postcss dependencies. diff --git a/src/util/cloneNodes.js b/src/util/cloneNodes.js index 5a51bbf374b2..05f9ae534f4a 100644 --- a/src/util/cloneNodes.js +++ b/src/util/cloneNodes.js @@ -1,4 +1,4 @@ -export default function cloneNodes(nodes, source) { +export default function cloneNodes(nodes, source = undefined, raws = undefined) { return nodes.map((node) => { let cloned = node.clone() @@ -12,6 +12,13 @@ export default function cloneNodes(nodes, source) { } } + if (raws !== undefined) { + cloned.raws.tailwind = { + ...cloned.raws.tailwind, + ...raws, + } + } + return cloned }) } diff --git a/tests/apply.test.js b/tests/apply.test.js index 99b620804654..7f149617e53a 100644 --- a/tests/apply.test.js +++ b/tests/apply.test.js @@ -1328,3 +1328,54 @@ it('should be possible to use apply in plugins', async () => { `) }) }) + +it('should apply using the updated user CSS when the source has changed', async () => { + let config = { + content: [{ raw: html`
` }], + plugins: [], + } + + let inputBefore = css` + .foo { + color: green; + } + + .bar { + @apply foo; + } + ` + + let inputAfter = css` + .foo { + color: red; + } + + .bar { + @apply foo; + } + ` + + let result = await run(inputBefore, config) + + expect(result.css).toMatchFormattedCss(css` + .foo { + color: green; + } + + .bar { + color: green; + } + `) + + result = await run(inputAfter, config) + + expect(result.css).toMatchFormattedCss(css` + .foo { + color: red; + } + + .bar { + color: red; + } + `) +}) diff --git a/tests/context-reuse.test.js b/tests/context-reuse.test.js index 8a606a4d4a75..0619c95cea13 100644 --- a/tests/context-reuse.test.js +++ b/tests/context-reuse.test.js @@ -1,17 +1,87 @@ +const fs = require('fs') const path = require('path') -const process = require('child_process') -const { promisify } = require('util') -const exec = promisify(process.exec) +const postcss = require('postcss') +const tailwind = require('../src/index.js') +const sharedState = require('../src/lib/sharedState.js') +const configPath = path.resolve(__dirname, './context-reuse.tailwind.config.js') +const { css } = require('./util/run.js') -test.skip('a build re-uses the context across multiple files with the same config', async () => { - const filepath = path.resolve(__dirname, './context-reuse.worker.js') +function run(input, config = {}, from = null) { + from = from || path.resolve(__filename) - const { stdout } = await exec(`node ${filepath}`) + return postcss(tailwind(config)).process(input, { from }) +} - const results = JSON.parse(stdout.trim()) +beforeEach(async () => { + let config = { + content: [path.resolve(__dirname, './context-reuse.test.html')], + corePlugins: { preflight: false }, + } - expect(results[0].activeContexts).toBe(1) - expect(results[1].activeContexts).toBe(1) - expect(results[2].activeContexts).toBe(1) - expect(results[3].activeContexts).toBe(1) + await fs.promises.writeFile(configPath, `module.exports = ${JSON.stringify(config)};`) +}) + +afterEach(async () => { + await fs.promises.unlink(configPath) +}) + +it('re-uses the context across multiple files with the same config', async () => { + let from = path.resolve(__filename) + + let results = [ + await run(`@tailwind utilities;`, configPath, `${from}?id=1`), + + // Using @apply directives should still re-use the context + // They depend on the config but do not the other way around + await run(`body { @apply bg-blue-400; }`, configPath, `${from}?id=2`), + await run(`body { @apply text-red-400; }`, configPath, `${from}?id=3`), + await run(`body { @apply mb-4; }`, configPath, `${from}?id=4`), + ] + + let dependencies = results.map((result) => { + return result.messages + .filter((message) => message.type === 'dependency') + .map((message) => message.file) + }) + + // The content files don't have any utilities in them so this should be empty + expect(results[0].css).toMatchFormattedCss(css``) + + // However, @apply is being used so we want to verify that they're being inlined into the CSS rules + expect(results[1].css).toMatchFormattedCss(css` + body { + --tw-bg-opacity: 1; + background-color: rgb(96 165 250 / var(--tw-bg-opacity)); + } + `) + + expect(results[2].css).toMatchFormattedCss(css` + body { + --tw-text-opacity: 1; + color: rgb(248 113 113 / var(--tw-text-opacity)); + } + `) + + expect(results[3].css).toMatchFormattedCss(css` + body { + margin-bottom: 1rem; + } + `) + + // Files with @tailwind directives depends on the PostCSS tree, config, AND any content files + expect(dependencies[0]).toEqual([ + path.resolve(__dirname, 'context-reuse.test.html'), + path.resolve(__dirname, 'context-reuse.tailwind.config.js'), + ]) + + // @apply depends only on the containing PostCSS tree *and* the config file but no content files + // as they cannot affect the outcome of the @apply directives + expect(dependencies[1]).toEqual([path.resolve(__dirname, 'context-reuse.tailwind.config.js')]) + + expect(dependencies[2]).toEqual([path.resolve(__dirname, 'context-reuse.tailwind.config.js')]) + + expect(dependencies[3]).toEqual([path.resolve(__dirname, 'context-reuse.tailwind.config.js')]) + + // And none of this should have resulted in multiple contexts being created + expect(sharedState.contextSourcesMap.size).toBe(1) }) diff --git a/tests/context-reuse.worker.js b/tests/context-reuse.worker.js deleted file mode 100644 index edaac7bce609..000000000000 --- a/tests/context-reuse.worker.js +++ /dev/null @@ -1,47 +0,0 @@ -const fs = require('fs') -const path = require('path') -const postcss = require('postcss') -const tailwind = require('../index.js') -const sharedState = require('../lib/sharedState.js') -const configPath = path.resolve(__dirname, './context-reuse.tailwind.config.js') - -function run(input, config = {}, from = null) { - from = from || path.resolve(__filename) - - return postcss(tailwind(config)).process(input, { from }) -} - -async function runTest() { - let messages = [] - - let config = { - darkMode: 'class', - content: [path.resolve(__dirname, './context-reuse.test.html')], - corePlugins: { preflight: false }, - theme: {}, - plugins: [], - } - - let from = path.resolve(__filename) - - fs.writeFileSync(configPath, `module.exports = ${JSON.stringify(config)};`) - - let results = [ - await run(`@tailwind utilities;`, configPath, `${from}?id=1`), - await run(`body { @apply bg-blue-400; }`, configPath, `${from}?id=2`), - await run(`body { @apply text-red-400; }`, configPath, `${from}?id=3`), - await run(`body { @apply mb-4; }`, configPath, `${from}?id=4`), - ] - - results.forEach(() => { - messages.push({ - activeContexts: sharedState.contextSourcesMap.size, - }) - }) - - process.stdout.write(JSON.stringify(messages)) -} - -runTest().finally(() => { - fs.unlinkSync(configPath) -})