Migrate to PostCSS 8

[hudochenkov]

Steps:

• Update to latest PostCSS dependencies
• Fix failing tests
• Fix failing TypeScript errors
• Migrate to built-in rules to visitor API to remove deprecation warnings
• Investigate impact on executing order

Release notes.
Migration guide.

---

Before we migrate we need all our dependencies be on PostCSS 8.

dependencies are ready:

• @stylelint/postcss-css-in-js — [WIP] Upgrade to PostCSS 8 postcss-css-in-js#74 (removed)
• @stylelint/postcss-markdown (removed)
• autoprefixer
• postcss
• postcss-html — https://github.com/stylelint/postcss-html/pull/1. (removed)
• postcss-less
• postcss-media-query-parser — it doesn't use PostCSS.
• postcss-resolve-nested-selector — doesn't depend on PostCSS.
• postcss-safe-parser
• postcss-sass
• postcss-scss
• postcss-selector-parser — doesn't have PostCSS as peer or regular dependency.
• postcss-syntax — https://github.com/stylelint/postcss-syntax/pull/2. (removed)
• postcss-value-parser — doesn't depend on PostCSS.
• sugarss

devDependencies are ready:

• postcss-import

---

Update won't be easy. We need to update custom syntaxes: @stylelint/postcss-css-in-js, @stylelint/postcss-markdown. And we probably need to move into stylelint organization and update postcss-html and postcss-syntax.

---

After all that we can start updating stylelint itself.

[hudochenkov]

Out of curiosity I just bumped PostCSS to see what happen.

52 errors from TypeScript, mostly about missing imports. Probably because of new TypeScript types structure in PostCSS.

Jest hanged (it was running, but no progress was made) with this message:

(node:93150) UnhandledPromiseRejectionWarning: TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Object'
    |     property 'parent' -> object with constructor 'Object'
    |     property 'nodes' -> object with constructor 'Array'
    --- index 0 closes the circle
    at stringify (<anonymous>)
    at writeChannelMessage (internal/child_process/serialization.js:117:20)
    at process.target._send (internal/child_process.js:779:17)
    at process.target.send (internal/child_process.js:677:19)
    at reportSuccess (/Users/user/projects/stylelint/node_modules/jest-worker/build/workers/processChild.js:67:11)
(node:93150) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 4399)
(node:93150) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

And this was it's status at that moment:

Test Suites: 5 failed, 50 passed, 55 of 339 total

Almost every test has this warning:

    console.warn
      postcss.plugin was deprecated. Migration guide:
      https://evilmartians.com/chronicles/postcss-8-plugin-migration

It's because every of 205 rules we have is a PostCSS plugin.

[hudochenkov]

I've migrated my postcss-sorting to PostCSS 8 and it was easy (didn't use new API, though). I use following syntaxes in tests and no issues:

• @stylelint/postcss-css-in-js
• postcss-html
• postcss-syntax

Maybe it won't be so hard to migrate stylelint. But then we could have strange issues because of different PostCSS versions in syntaxes and stylelint itself.

[hudochenkov]

I forked postcss-html and postcss-syntax to be part of stylelint organization, so we could update them and release under @stylelint namespace.

[stof]

what is the status of this work ?
The update of my other postcss-based tooling in my project are blocked because I cannot really depend on 2 versions on postcss depending on whether stylelint plugins or gulp are calling postcss (or at least I don't know how that could work in a reliable way regarding dependencies)

[hudochenkov]

I believe you could continue to use stylelint as you would before. Even if you use it as a PostCSS plugin. Because PostCSS 8 support plugins made for PostCSS 7.

[niksy]

Because PostCSS 8 support plugins made for PostCSS 7.

Is this true? It seems to me that most of the plugin updates are following documentation recommendation to use Once hook to make plugin compatible with PostCSS 8, and then later switch to more performant hooks.

[stof]

@niksy postcss 8 still supports the postcss.plugin API. But using it triggers a warning.

[hudochenkov]

I wrote down some issues we have with custom syntaxes and what to do about that in PostCSS repository postcss/postcss#1498

[ludofischer]

Will OnceExit on indentation suffice as a workaround in PostCSS 8? It's the one rule we've stuck at the bottom of the list of rules.

In my experience, it's better to stick with either Once or OnceExit. If you define every plugin with Once or every plugin with OnceExit they run in the order you pass them to postcss. If you run mix Once and OnceExit you have less control, as a third-party Once plugin could jump in between your Once and OnceExit rules.

[ludofischer]

Yep, later down the line, it'd be great to benchmark the performance gains we can bring to some of our heavier rules.

Oh! Great that you've created this tool, I could use it to test cssnano plugins!

I've found that extracting the full performance potential from the visitor API was quite tricky, because in some cases the visitor API calls the same plugin multiple times.
For example, with the traditional API, color minification runs just once on the whole stylesheet, but with the visitor API, if another plugin merges two declarations, PostCSS calls the color minifier again on the resulting new declaration. I suspect the more time you spend inside css-selector-parser and css-value-parser, the less performance benefits you get from the visitor API.

[jeddy3]

We've got the ball rolling on the 14.0.0 release (#5205 (comment)), and this issue is now unblocked.

I've added a checklist to the top post for the update to PostCSS 8. There's a Draft PR, and associated branch, for this work.

There are 6 failing tests. Most seem to be around a change in parser behaviour for custom property sets. I suggest we pick these up in 3 separate pull requests:

• FAIL lib/__tests__/disableRanges.test.js (skipped)
• FAIL lib/rules/string-no-newline/__tests__/index.js
• The rest (which are connected to custom property sets)

Once we've patched up the failing tests and TypeScript errors, we can migrate the built-in rules to the visitor API to remove the deprecation warnings. Lastly, we'll need to investigate the impact of this change on the execution order.

Please shout out if you'd like to pick up one of the failing tests pull requests, or start looking at the TypeScript errors. Remember to branch off update-to-postcss-8.

[hudochenkov]

Before migrating a rule to visitor API we would need to change how we run rules. Currently we going around PostCSS and call rule function directly. It will not work anymore, because now PostCSS plugin returns an object, and not a function.

We need to use PostCSS API to call plugins now. E. g.:

await postcss([plugin(pluginOptions)]).process(root, {
    from: result.from,
});

[jeddy3]

The last failing test we need to fix is:

 FAIL  lib/__tests__/disableRanges.test.js
  ● Less // disable next-line comment (with multi-line description)

    expect(received).toEqual(expected) // deep equality

    - Expected  - 3
    + Received  + 3

      Object {
    -   "description": "Long-winded description",
    -   "end": 5,
    -   "start": 5,
    +   "description": undefined,
    +   "end": 39,
    +   "start": 39,
        "strictEnd": true,
        "strictStart": true,
      }

      1000 | 
      1001 |    delete actualMutable.comment;
    > 1002 |    expect(actualMutable).toEqual(expected);
           |                          ^
      1003 | }
      1004 | 
      1005 | function testDisableRanges(source, options) {

      at expectDisableRange (lib/__tests__/disableRanges.test.js:1002:24)
      at expectDisableRanges (lib/__tests__/disableRanges.test.js:993:4)
      at Object.<anonymous> (lib/__tests__/disableRanges.test.js:968:2)

For this test:

it('Less // disable next-line comment (with multi-line description)', async () => {
	const result = await testDisableRangesLess(
		`a {
			// stylelint-disable-next-line declaration-no-important
			// --
			// Long-winded description
			color: pink !important;
		}`,
	);

	expectDisableRanges(result, {
		all: [],
		'declaration-no-important': [
			{
				start: 5,
				end: 5,
				strictStart: true,
				strictEnd: true,
				description: 'Long-winded description',
			},
		],
	});
});

It's the workaround we added to merge adjacent double slash comments into one.

comment.source.end.line jumps to 69 at some point, which doesn't seem right. It may be an issue upstream in the less parser or one we need to address in stylelint.

@jathak do you have any ideas? I think you worked on this workaround at the time.

[jeddy3]

I think we can, if anyone fancies it, pick up the TypeScript errors while we look into this last failing test?

[kaaax0815]

This would fix: CVE-2021-23368

stylelint@13.13.1 requires postcss@^7.0.32 via a transitive dependency on autoprefixer@9.8.6
stylelint@13.13.1 requires postcss@^7.0.35
stylelint@13.13.1 requires postcss@^7.0.14 via postcss-less@3.1.4
stylelint@13.13.1 requires postcss@^7.0.26 via postcss-safe-parser@4.0.2
stylelint@13.13.1 requires postcss@^7.0.21 via a transitive dependency on postcss-sass@0.4.4
stylelint@13.13.1 requires postcss@^7.0.6 via postcss-scss@2.1.1
stylelint@13.13.1 requires postcss@^7.0.2 via sugarss@2.0.0

[jeddy3]

That's correct. The Draft PR has no PostCSS@7 within its tree.

[jeddy3]

Before migrating a rule to visitor API we would need to change how we run rules. Currently we going around PostCSS and call rule function directly.

As our rules and plugins are rule functions, is there a way we can adapt them Once: {} PostCSS 8 plugins on the fly?

I'm not very familiar with the engine as I've been more focused on the rules since the beginning of the project. So, if anyone fancies picking up this migration to PostCSS 8, please go ahead as I don't think I'll have time myself.

[hudochenkov]

As our rules and plugins are rule functions, is there a way we can adapt them Once: {} PostCSS 8 plugins on the fly?

If it's possible and we do this we need to find a way how to support rules and plugins, which use visitor API.

[jeddy3]

Yep. I figured we could get the existing rules and plugins working first and then look into the visitor API. I think it's OK if we release without support for the visitor API. We can update our rule and plugin docs to say that the stylelint rule function will be placed within the PostCSS Once context, and therefore a stylelint rule/plugin can't (at this time) use the other visitors from the API.

Better to get something secure out-the-door than try to support of a type of stylelint rule or plugin that's yet to be written. Of course, if there's a way of doing both then that would be great. But I think we should prioritise the former over the latter for now.

[hudochenkov]

I'll be fixing type issues and then try to continue migration to PostCSS 8. I've already started doing some work in a new branch, which is branched out from update-to-postcss-8.

[jeddy3]

Closed by #5304

[jeddy3]

For anyone following this issue, there's still lots to do before we can release 14.0.0. Please consider contributing to any of the outstanding issues, if you'd like to help make the release happen.

[Stanzilla]

@jeddy3 any plans for an interim 13.x release just to fix the vulnerabilities and deprecation warnings?

[jeddy3]

A 13.x release isn't possible because we had to make breaking changes, including removing the bundled syntaxes e.g. the incompatible CSS-in-JS syntax, to make the migration to PostCSS@8 possible.

There is also a vulnerability in another dependency that is now ESM-only, so we need to migrate to ESM to fix all vulnerabilities.

The best thing everyone can do is help us work through the remaining issues so that we release 14.0.0 as soon as possible.

[hudochenkov]

PostCSS v7 was released with back-port of ReDoS fix. So stylelint users on current version of stylelint should see warning go away after running npm audit fix (it could take few days until npm audit will know about the fix).

https://twitter.com/PostCSS/status/1403351574110511106