Address "postcss.plugin was deprecated" warning

#3261163: Update to PostCSS 8 landed so this can be addressed now.

Whilst looking to upgrade our custom plugin, I found out nothing changes if I remove the plugin on my local after running a yarn build.

Let's see if TestBot sees it the same way.

Hmm, looks like the plugin does actually does things on 9.x and doesn't (need to) on 10.x

So let's try an upgrade attempt on 9.5.x where the custom plugin actually does something.

The goal here is to have:

- No CSS changes
- No deprecation message in the build log

Since yarn build doesn't seem to be part of the drupal CI, we need manual testing for this one:

- Checkout dev branch
- Apply patch
- cd core/
- yarn install
- node ./scripts/css/postcss-build.js (This is the acual sub-command in yarn build that triggers the deprecation message.

After the above commands are fully finished, for a successful test, there should be:
- NO changes in any of the CSS files.
- NO occurrence of the deprecation message remove-unwanted-comments-from-variables: postcss.plugin was deprecated. Migration guide: https://evilmartians.com/chronicles/postcss-8-plugin-migration anywhere in the output of the above commands.

1. +++ b/core/package.json
   @@ -97,6 +97,9 @@
   +  "peerDependencies": {
   +    "postcss": "^8"
   +  },
   

   Is this required? We already have an actual dependency on postcss, not sure we need a peer dependency as well.

2. +++ b/core/scripts/css/remove-unwanted-comments-from-variables.js
   @@ -0,0 +1,18 @@
   +      if (css.source.input.file.indexOf('variables.pcss.css') !== -1) {
   

   In 10.x if we replace variables.pcss.css with media-queries.pcss.css then some cruft is removed from the built CSS. Maybe we should also just rename the plugin to remove-unwanted-comments?

https://www.drupal.org/project/drupal/issues/3308872#comment-14814010.1

Is this required? We already have an actual dependency on postcss, not sure we need a peer dependency as well.

The way I understand this: https://evilmartians.com/chronicles/postcss-8-plugin-migration : Step 1: Move postcss to peerDependencies:

This will keep the size of the end-user’s node_modules directory under control: now, all plugins will use the same version of postcss as a dependency.

it isn't (really?) necessary, but will make all postcss-plugins use the same postcss version, which seems like a good thing to me.

https://www.drupal.org/project/drupal/issues/3308872#comment-14814010.2

In 10.x if we replace variables.pcss.css with media-queries.pcss.css then some cruft is removed from the built CSS. Maybe we should also just rename the plugin to remove-unwanted-comments?

Nice!
Booh to cruft!

Re peerDependencies, I might be wrong here but I think that statement only applies when you are distributing the plugin as a separate npm package. Here we are just shipping it along with core so the devDependencies entry is enough.

Well, you're the native speaker...:)

Oh, nice that we can remove the filename check entirely, and the output is still sensible, and even cleaner than before!

This all looks good to me now.

Given that this includes an API addition, it should probably be 10.1.x only. (Also, 9.5.x does not seem to have the unwanted comments anyway, so no need to backport the API there?)

+++ b/core/scripts/css/compile.js
@@ -48,22 +36,22 @@ module.exports = (filePath, callback) => {
-          propList: [
-            '*',
-            '!background-position',
-            '!border',
-            '!border-width',
-            '!box-shadow',
-            '!border-top*',
-            '!border-right*',
-            '!border-bottom*',
-            '!border-left*',
-            '!border-start*',
-            '!border-end*',
-            '!outline*',
-          ],
-          mediaQuery: true,
-          minPixelValue: 3,
+        propList: [
+          '*',
+          '!background-position',
+          '!border',
+          '!border-width',
+          '!box-shadow',
+          '!border-top*',
+          '!border-right*',
+          '!border-bottom*',
+          '!border-left*',
+          '!border-start*',
+          '!border-end*',
+          '!outline*',
+        ],
+        mediaQuery: true,
+        minPixelValue: 3,
       }),

@@ -74,19 +62,19 @@ module.exports = (filePath, callback) => {
-    .process(css, { from: filePath })
-    .then(result => {
+      .process(css, {from: filePath})
+      .then(result => {
         return stylelint.lint({
           code: result.css,
           fix: true
         });
-    })
-    .then(result => {
-      callback(result.output);
-    })
-    .catch(error => {
-      log(error);
-      process.exitCode = 1;
-    });
+      })
+      .then(result => {
+        callback(result.output);
+      })
+      .catch(error => {
+        log(error);
+        process.exitCode = 1;
+      });

As far as I can see, these hunks are just having their whitespace changed. Out of scope? I confirmed locally that these changes don't show up with git diff -w

Aside from that, I confirmed that if I simply apply these three hunks:

diff --git a/core/package.json b/core/package.json
index e06958b4ce..5aa7f6025b 100644
--- a/core/package.json
+++ b/core/package.json
@@ -107,6 +107,5 @@
     "last 1 ChromeAndroid version",
     "last 1 Samsung version",
     "Firefox ESR"
-  ],
-  "dependencies": {}
+  ]
 }
diff --git a/core/scripts/css/compile.js b/core/scripts/css/compile.js
index fe52555a77..1d984f05cd 100644
--- a/core/scripts/css/compile.js
+++ b/core/scripts/css/compile.js
@@ -8,28 +8,16 @@ const postcssPresetEnv = require('postcss-preset-env');
 // cspell:ignore pxtorem
 const postcssPixelsToRem = require('postcss-pxtorem');
 const stylelint = require('stylelint');
+const removeUnwantedComments = require('./remove-unwanted-comments');
 
 module.exports = (filePath, callback) => {
   // Transform the file.
   fs.readFile(filePath, (err, css) => {
     postcss([
       postcssImport({
-       plugins: [
-         // On import, remove the comments from variables.pcss.css so they don't
-         // appear as useless comments at the top files that import these
-         // variables.
-         postcss.plugin('remove-unwanted-comments-from-variables', (options) => {
-           return css => {
-             if (css.source.input.file.indexOf('variables.pcss.css') !== -1) {
-               css.walk(node => {
-                 if (node.type === 'comment') {
-                   node.remove();
-                 }
-               });
-             }
-           };
-         }),
-       ],
+        plugins: [
+          removeUnwantedComments,
+        ],
       }),
       postcssPresetEnv({
         stage: 1,
diff --git a/core/scripts/css/remove-unwanted-comments.js b/core/scripts/css/remove-unwanted-comments.js
new file mode 100644
index 0000000000..bedbb34797
--- /dev/null
+++ b/core/scripts/css/remove-unwanted-comments.js
@@ -0,0 +1,14 @@
+// On import, remove the comments, so they don't appear as useless comments at the top of the autogenerated css files.
+module.exports = opts => {
+  return {
+    postcssPlugin: 'remove-unwanted-comments',
+    Once(css) {
+      css.walk(node => {
+        if (node.type === 'comment') {
+          node.remove();
+        }
+      })
+    }
+  }
+}
+module.exports.postcss = true

...and then rerun yarn build, it results in the comments being removed from the Olivero files.

NW to remove the out-of-scope whitespace changes. Thanks!

Removed whitespaces, now 10.1.x only.

Partial patch for build testing purposes.

#19 looks good to me.

Committed to 10.1.x. Thanks!