From b45c255cd32450cc66730dc23e557e21d0d6ff36 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Thu, 3 Feb 2022 09:28:33 -0500 Subject: [PATCH 1/2] Prevent nesting plugin from breaking other plugins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This uses a private API but it’s the only solution we have right now. It’s guarded to hopefully be less breaking if the API disappears. --- src/postcss-plugins/nesting/plugin.js | 36 ++++++++++++ tests/postcss-plugins/nesting/index.test.js | 61 +++++++++++++++++++-- tests/postcss-plugins/nesting/plugins.js | 42 ++++++++++++++ 3 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 tests/postcss-plugins/nesting/plugins.js diff --git a/src/postcss-plugins/nesting/plugin.js b/src/postcss-plugins/nesting/plugin.js index 3b1434d82ed1..a13cfd454ec7 100644 --- a/src/postcss-plugins/nesting/plugin.js +++ b/src/postcss-plugins/nesting/plugin.js @@ -39,6 +39,42 @@ export function nesting(opts = postcssNested) { decl.remove() }) + /** + * Use a private PostCSS API to remove the "clean" flag from the entire AST. + * This is done because running process() on the AST will set the "clean" + * flag on all nodes, which we don't want. + * + * This causes downstream plugins using the visitor API to be skipped. + * + * This is guarded because the PostCSS API is not public + * and may change in future versions of PostCSS. + * + * See https://github.com/postcss/postcss/issues/1712 for more details + * + * @param {import('postcss').Node} node + */ + function markDirty(node) { + if (!('markDirty' in node)) { + return + } + + // Traverse the tree down to the leaf nodes + if (node.nodes) { + node.nodes.forEach((n) => markDirty(n)) + } + + // If it's a leaf node mark it as dirty + // We do this here because marking a node as dirty + // will walk up the tree and mark all parents as dirty + // resulting in a lot of unnecessary work if we did this + // for every single node + if (!node.nodes) { + node.markDirty() + } + } + + markDirty(root) + return root } } diff --git a/tests/postcss-plugins/nesting/index.test.js b/tests/postcss-plugins/nesting/index.test.js index 482622ed3343..667ac851f206 100644 --- a/tests/postcss-plugins/nesting/index.test.js +++ b/tests/postcss-plugins/nesting/index.test.js @@ -1,6 +1,7 @@ import postcss from 'postcss' import postcssNested from 'postcss-nested' import plugin from '../../../src/postcss-plugins/nesting' +import { visitorSpyPlugin } from './plugins.js' it('should be possible to load a custom nesting plugin', async () => { let input = css` @@ -166,6 +167,46 @@ test('@screen rules can work with `@apply`', async () => { `) }) +test('nesting does not break downstream plugin visitors', async () => { + let input = css` + .foo { + color: black; + } + @suppoerts (color: blue) { + .foo { + color: blue; + } + } + /* Comment */ + ` + + let spyPlugin = visitorSpyPlugin() + + let plugins = [plugin(postcssNested), spyPlugin.plugin] + + let result = await run(input, plugins) + + expect(result).toMatchCss(css` + .foo { + color: black; + } + @suppoerts (color: blue) { + .foo { + color: blue; + } + } + /* Comment */ + `) + + expect(spyPlugin.spies.Once).toHaveBeenCalled() + expect(spyPlugin.spies.OnceExit).toHaveBeenCalled() + expect(spyPlugin.spies.Root).toHaveBeenCalled() + expect(spyPlugin.spies.Rule).toHaveBeenCalled() + expect(spyPlugin.spies.AtRule).toHaveBeenCalled() + expect(spyPlugin.spies.Comment).toHaveBeenCalled() + expect(spyPlugin.spies.Declaration).toHaveBeenCalled() +}) + // --- function indentRecursive(node, indent = 0) { @@ -187,11 +228,21 @@ function formatNodes(root) { } async function run(input, options) { - return ( - await postcss([options === undefined ? plugin : plugin(options), formatNodes]).process(input, { - from: undefined, - }) - ).toString() + let plugins = [] + + if (Array.isArray(options)) { + plugins = options + } else { + plugins.push(options === undefined ? plugin : plugin(options)) + } + + plugins.push(formatNodes) + + let result = await postcss(plugins).process(input, { + from: undefined, + }) + + return result.toString() } function css(templates) { diff --git a/tests/postcss-plugins/nesting/plugins.js b/tests/postcss-plugins/nesting/plugins.js new file mode 100644 index 000000000000..bc7f9c93e81c --- /dev/null +++ b/tests/postcss-plugins/nesting/plugins.js @@ -0,0 +1,42 @@ +export function visitorSpyPlugin() { + let Once = jest.fn() + let OnceExit = jest.fn() + let Root = jest.fn() + let AtRule = jest.fn() + let Rule = jest.fn() + let Comment = jest.fn() + let Declaration = jest.fn() + + let plugin = Object.assign( + function () { + return { + postcssPlugin: 'visitor-test', + + // These work fine + Once, + OnceExit, + + // These break + Root, + Rule, + AtRule, + Declaration, + Comment, + } + }, + { postcss: true } + ) + + return { + plugin, + spies: { + Once, + OnceExit, + Root, + AtRule, + Rule, + Comment, + Declaration, + }, + } +} From 737f25e7042c06022662d6b785cb2ed310cfa629 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Mon, 21 Feb 2022 10:08:19 -0500 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27bff64ddb99..a7e4f99b017a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- Nothing yet! +- Prevent nesting plugin from breaking other plugins ([#7563](https://github.com/tailwindlabs/tailwindcss/pull/7563)) ## [3.0.23] - 2022-02-16