Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent nesting plugin from breaking other plugins #7563

Merged
merged 2 commits into from Feb 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -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

Expand Down
36 changes: 36 additions & 0 deletions src/postcss-plugins/nesting/plugin.js
Expand Up @@ -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
}
}
61 changes: 56 additions & 5 deletions 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`
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
42 changes: 42 additions & 0 deletions 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,
},
}
}