Problem/Motivation

Drupal currently builds CSS with PostCSS 7, but PostCSS 8 is available and most plugins have been upgraded to support it.

Steps to reproduce

Proposed resolution

Upgrade to PostCSS 8.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

PostCSS has been upgraded from 7.0.39 to 8.4.16. Developers who used custom PostCSS plugins may need to refer to the PostCSS 8 plugin migration guide.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave created an issue. See original summary.

longwave’s picture

Status: Active » Needs review
FileSize
133.05 KB

Most differences are around output order of CSS rules, and postcss-calc now seems to precalculate the result when the units match. Also, the way you define plugins has changed so our custom removeUnwantedCommentsFromVariables plugin is different.

Two issues:

1. postcss-header has not been updated so there is a warning on build; I suggest we just pull this directly into the build script as it's only a few lines of code.

2. postcss-url doesn't seem to inline all SVGs any more, only some of them.

longwave’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
longwave’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
127.53 KB
135.72 KB

postcss-header has been updated, and postcss-url seems to be fixed now, so the output is now better than #2.

In these patches I haven't dealt with the deprecation for the removeUnwantedCommentsFromVariables plugin, I think maybe we should do that in a followup to keep rerolling this simple.

Patches created with

$ yarn upgrade --latest postcss postcss-calc postcss-header postcss-import postcss-preset-env postcss-pxtorem postcss-url
$ yarn build:css

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bnjmnm’s picture

The 95 patch has changes to core.libraries.yml that seem unexpected. These happen after yarn build so I'm wondering if a build step that would have called vendor update was missed in #3306182: Update to latest minor and patch versions of JavaScript dependencies (except nightwatch and cspell).

longwave’s picture

lauriii’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
lauriii’s picture

Spokje’s picture

Assigned: Unassigned » Spokje
Spokje’s picture

Can for the life of me not get a non-empty interdiff between the old and rerolled 10x patches.
Could be because of the Patch Failed to Apply of the old one.

Anyway, attached a raw-diff (and yes, I even remembered to do a yarn build this time... 😈)

Spokje’s picture

Assigned: Spokje » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
578.84 KB
868.94 KB
Spokje’s picture

Status: Needs review » Needs work

No clue what's up with #12, I just re-did a yarn build and that patch and had no changes.
Never mind, there _are_ changes. Somehow on my machine yarn build seems to be doing things in parallel and not be very good at it.
It returns as being ready, but some tasks come in later.

Anyway, both 10x and 95x are getting a lot of:

[11:31:59] '/var/www/html/core/themes/olivero/css/components/fieldset.pcss.css' is being checked.
remove-unwanted-comments-from-variables: postcss.plugin was deprecated. Migration guide:

So I _think_ it needs work anyway?

Spokje’s picture

FileSize
772.18 KB
64.34 KB
Spokje’s picture

#14 passed the Custom commands phase, keeping this on NW because of (at least) 10.0.x and 10.1.x are giving a lot of messages like the one below

yarn run v1.22.19
$ node ./scripts/css/postcss-build.js --check --file /var/www/html/core/themes/olivero/css/components/navigation/nav-primary-no-js.pcss.css
[11:32:18] '/var/www/html/core/themes/olivero/css/components/navigation/nav-primary-no-js.pcss.css' is being checked.
remove-unwanted-comments-from-variables: postcss.plugin was deprecated. Migration guide:
https://evilmartians.com/chronicles/postcss-8-plugin-migration
Done in 0.84s.
longwave’s picture

In #4 I proposed dealing with that deprecation in a followup, but I also don't mind doing it here if we think that is necessary.

Spokje’s picture

@longwave: Ah, thanks for pointing that out, completely overlooked your comment.

lauriii’s picture

Status: Needs work » Needs review

Based on #16 and #17, it sounds like this needs review.

Wim Leers’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
bnjmnm’s picture

Status: Needs work » Needs review
FileSize
333.67 KB
349.16 KB

Reroll

longwave’s picture

Needs followup for "remove-unwanted-comments-from-variables: postcss.plugin was deprecated."

bnjmnm’s picture

nod_’s picture

Status: Needs review » Reviewed & tested by the community

all good

lauriii’s picture

Issue tags: +Needs reroll

It looks like the 10.0.x patch needs another reroll 😅

bnjmnm’s picture

Issue tags: -Needs reroll
FileSize
333.68 KB

Here's another 10x. 9x in #20 seems to still be OK.

longwave’s picture

Diff between #20 and #25 is only context in package.json and an autoprefixer patch release in yarn.lock, back to RTBC.

edit: was already RTBC, but I had the tab open from earlier!

lauriii’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

10.0.x needs another reroll now that #3269082: Remove HTML5 details collapse polyfill has landed. 🤪

nod_’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
333.54 KB

conflict was in olivero details.css file

  • catch committed 01d22dc on 10.0.x
    Issue #3261163 by bnjmnm, Spokje, longwave, nod_: Update to PostCSS 8
    
    (...

  • catch committed dd692fb on 10.1.x
    Issue #3261163 by bnjmnm, Spokje, longwave, nod_: Update to PostCSS 8
    
  • catch committed e63c60e on 9.5.x
    Issue #3261163 by bnjmnm, Spokje, longwave, nod_: Update to PostCSS 8
    
catch’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs reroll

Committed/pushed to 10.1.x, cherry-picked to 10.0.x and the respective patch to 9.5.x, thanks!

phenaproxima’s picture

Status: Fixed » Needs work
Issue tags: +Needs release note

This dependency update (yes, even dev dependencies, apparently...I don't make the rules) should have a release note. :)

longwave’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs release note

Added a short release note with a link to the plugin migration guide.

longwave’s picture

phenaproxima’s picture

Status: Needs review » Fixed

Perfect.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.