Running postcss inside a Once visitor can cause other plugins to stop working.

[thecrypticace]

Hi, I'm looking into an issue where the tailwindcss/nesting plugin breaks other PostCSS plugins and would like some information on what the expected behavior is and what we should be doing / can do to fix this issue.

Summary of the problem

Our nesting plugin wraps existing an nesting plugin and runs it synchronously to completion so Tailwind doesn't have to deal with nested CSS. However, running postcss inside of a postcss Once visitor breaks visitors for plugins that use anything other than Once / OnceExit.

Why the plugin exists

Tailwind can't reliably handle nested CSS internally. As such, we rely on users using a nesting plugin to prevent this from being an issue. However, since Tailwind is effectively a Once plugin (though it doesn't use the visitor API), the visitors for Rule, AtRule, etc… are all run after Tailwind runs — even for plugins that are listed before Tailwind in the postcss plugin list. Because of this we register a plugin that wraps a nesting plugin and runs it synchronously to completion before anything else (like Tailwind) sees them.

Our implementation

First, we perform a few transformations on the tree to ensure that nesting plugins handle hoisting @screen and @apply appropriately (e.g. @screen is the same as a media query so should be hoisted but @apply produces a list of declarations so it shouldn't be).

Next, we run the plugin using the following:

postcss([nestingPlugin]).process(root, result.opts).sync()

Lastly, we do some final cleanup on the @apply fixups so Tailwind can look for @apply like it usually does.

The problem (in some more detail)

Running postcss().process().sync() on a root will mark every node in it as "clean" once it's done. This is an internal marker that postcss uses to determine if a node has been processed. If a node is marked as "clean" then no further transformations on it are run on those nodes. However, doing this breaks plugins that use the visitor API (for example css-has-pseudo registers a Rule visitor) because once the node is clean it doesn't have to be processed any further. Which means that css-has-pseudo's Rule visitor never runs.

The question

I discovered we can work around this by importing LazyResult from the postcss/lib/lazy-result and instantiating it with the root node. However, this feels very hacky and I worry that referencing internals and using the cleaning side effect could break at any time. What would be the best solution to this problem? Is there something we should be doing differently?

You can see the core implementation of our plugin here:
https://github.com/tailwindlabs/tailwindcss/blob/50802e1aed1a8ed9b0eead1d45722793a86d3069/src/postcss-plugins/nesting/plugin.js#L5

Reproduction

This is a minimal reproduction that doesn't use our plugin but still runs postcss in the exact same manner which shows the issue. There are only three plugins involved and all are inlined the the postcss config. I can also prep one using the actual nesting plugins themselves if necessary.

Link: https://github.com/thecrypticace/postcss-visitor-question

1. npm install
2. npm run dev
3. Look at the output in the CLI versus the console.log calls in the postcss config file.

I'm more than happy to provide further details, reproductions, etc… as needed. I appreciate you taking your time to look at and think through this!

[ai]

I will look deeply tomorrow.

The general policy is that Once is some sort or legacy-hack. This plugin-by-plugin sequence always create a problem with plugin order conflict. We added visitor events (Rule, Declaration, etc) to solve the problem with plugin order. This is why Once-way plugins is treated as legacy where the problem can’t be solved. And the only good way is to migrate to full event system.

But I will try to find a better solution in the next few days.

[adamwathan]

Maybe to step back to our bigger picture problem in case it's helpful, the main thing we need to solve is we need to make sure that Tailwind itself never has to work with nested CSS. We need anyone using nesting to flatten their CSS before Tailwind sees it, because Tailwind just doesn't understand the semantics of nested CSS, and can't realistically be expected to in my opinion (since it's not valid CSS, and every nesting plugin could transform the nested structure in different ways).

So our biggest problem here is just how do we guarantee that if someone is using nesting, that the CSS is flattened before the tree hits Tailwind.

[ai]

The simplest idea with current API is to use result.processor.plugins to get the current plugins list. Then you can find a plugin with conflict and ask to replace it.

For instance, you can ask to use Once based tailwind/nested instead of postcss-nested.

[ai]

Running postcss().process().sync() on a root will mark every node in it as "clean" once it's done. This is an internal marker that postcss uses to determine if a node has been processed.

Hm. Maybe (as a quick fix without API breaking changes) we can use unique clean marks (for instance, by generating new Symbol for every process call).

You can use root.markDirty() after process call, but I think that it is really unexpected that you can’t call two different process on the same AST. Maybe it better to fix the core since other user could be caught by the same problem.

Will it solve the problem for you?

[thecrypticace]

Ah I didn't notice markDirty (it's not in the typescript types as far as I can tell so I missed it when poking around) — I'm assuming it's somewhat of a private API? I would also say it's unexpected that you can't have two different process() calls on the same AST. Using unique marks per call to process() (or even just per Processor instance) would indeed fix the problem for us.

Given that isClean is currently defined on Node Would it make sense to use a WeakMap or WeakSet to track clean vs. dirty nodes instead of attaching new symbols to a node every time process is called? Or would that involve breaking changes? (or even make sense? I'm just throwing out ideas here)

[ai]

Yeap, it is “private” API. Can you try to call it to check it will help?

I will think about the best implementation (I am not sure that we can't avoid a single mark, but we can always call markDirty before processing if root is not clean).

[thecrypticace]

Just checked. A lone call to root.markDirty() will not fix it because the subtree is still clean.

However, this works since it cleans the entire tree (though it's a bit inefficient since it walks back up the ancestor tree for every single node):

root.markDirty()
root.walk(node => node.markDirty())

[ai]

Very strange. PostCSS already has a code to clean isClean https://github.com/postcss/postcss/blob/main/lib/lazy-result.js#L118-L120

How do you call PostCSS? Maybe you use another API which do not clean marks?

[thecrypticace]

It has to do with the order plugins run in and when marks are cleared. They are removed when LazyResult is created not when it is finished. The reason LazyResult isn't clearing the clean flag is because the inner LazyResult is created while the outer one is still running plugins.

So, if you were to do this:

postcss(plugins).process(root).sync()
postcss(otherPlugins).process(root).sync()

This order of calls looks like this:

calls process() with plugins
  1. Clear marks
  2. Run plugins
    2.1 Plugin 1
      sees no clean flag
    2.2 Plugin 2
      sees no clean flag
    2.3 Plugin 3
      sees no clean flag
  3. Mark as clean

calls process() with otherPlugins
  1. Clear marks
  2. Run plugins
    2.1 Plugin 1
      sees no clean flag
    2.2 Plugin 2
      sees no clean flag
    2.3 Plugin 3
      sees no clean flag
  3. Mark as clean

This works as expected because a LazyResult is created after the previous one is finished.

However, if you run process() on the root node inside a plugin the marks get cleared before it runs (correct) but the entire tree gets marked as clean at the end (incorrect only because we're running process() during the plugin run).

So, if you were to do this:

postcss([
  {
    Once(root, { result }) {
      postcss(otherPlugins).process(root, result.opts).sync()
    }
  },
  plugin2,
  plugin3,
]).process(root).sync()

The order of operations looks like this, which exhibits the weird behavior:

calls process() with plugins
  1. Clear marks
  2. Run plugins
    2.1 Plugin 1
      sees no clean flag
      calls process() with otherPlugins
      2.1.1. Clear marks
      2.1.2 Run plugins
        2.1.2.1 Plugin 1
          sees no clean flag
        2.1.2.2 Plugin 2
          sees no clean flag
        2.1.2.3 Plugin 3
          sees no clean flag
      2.1.3. Mark as clean
    2.2 Plugin 2
      sees clean flag
    2.3 Plugin 3
      sees clean flag
  3. Mark as clean

[ai]

Oh, process() inside Once. Got it.

What do you think if we will stick with markDirty? It is a very unexpected (and I hope rare) way to use PostCSS API.

We will not add auto-dirty to PostCSS for this case since it is hard. It is better spend time of rewriting Tailwind to PostCSS 8 event system. If you have any question about migration, I can answer quick in Telegram or by email.