Problem/Motivation

Package                   Current Wanted  Latest   
postcss                   8.4.19  8.4.21  8.4.21   
postcss-import            15.0.1  15.1.0  15.1.0   
postcss-preset-env        7.8.3   7.8.3   8.3.0   

Steps to reproduce

Proposed resolution

$ yarn add -D postcss-preset-env
$ yarn upgrade postcss postcss-import

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3351619

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

longwave created an issue. See original summary.

longwave’s picture

Status: Active » Needs review
StatusFileSize
new85.14 KB

postcss-preset-env now adds -webkit-text-decoration everywhere text-decoration is used due to a Safari issue: https://github.com/postcss/autoprefixer/issues/1473

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

No failures.

Not sure the level of concern -webkit-text-decoration being added everywhere. Assuming none just mentioning.

longwave’s picture

There is no concern from me, just a note to reviewers in case they are surprised by the diff like I was.

longwave’s picture

Status: Reviewed & tested by the community » Needs review

Actually Olivero's variables.css has some odd looking changes, would like a second opinion on those from a frontend developer.

mherchel’s picture

Something's looking weird here. I'm seeing lots of additions of -webkit-text-decoration, which makes absolutely no sense since that's a very old and well supported property.

In general this need to be done, as it's the correct resolution for #3313146: PostCSS Logical not transpiling flow relative properties (e.g. float: inline-start) which are not supported by Chrome and Safari, where PostCSS Preset ENV now includes https://www.npmjs.com/package/@csstools/postcss-logical-float-and-clear (see https://github.com/csstools/postcss-plugins/issues/632)

I'm going to pull it down and take a closer look.

mherchel’s picture

Status: Needs review » Needs work

The patch no longer applies to 10.1.x

I ran the following commands:

$ yarn add -D postcss-preset-env
$ yarn upgrade postcss postcss-import

Then I ran yarn build:css.

The CSS that was generated for me was different than the patch above:

In addition, the new version is moving around some style blocks, which could potentially have regressions (although I doubt it actually will).

Setting this to NW based on all of this. I wish I had more time to work on this, but I don't at the moment.

gauravvvv’s picture

Status: Needs work » Needs review
StatusFileSize
new392.03 KB

Patch #2, no longer applies to 10.1.x. I have re-rolled the patch. please review

smustgrave’s picture

Status: Needs review » Needs work

Umm that patch is almost 4 times the size of patch 2.

Also mentioned in the comment before there were some issues. Did you run the same steps mentioned in #7?

gauravvvv’s picture

Status: Needs work » Needs review
StatusFileSize
new86.62 KB

Above patch has some errors (size is too high), I have rebased the branch and provided the latest patch.

gauravvvv’s picture

Also mentioned in the comment before there were some issues. Did you run the same steps mentioned in #7?

Yes I ran the same steps, it might be because of my branch was not up to date. I have provided the patch after rebasing the branch.

spokje’s picture

StatusFileSize
new316.66 KB

So, "interestingly" when I do a yarn install followed by a yarn build:css , so _without_ updating anything, I get the changes in the attached patch, which seem just to have (very) wrong indentations.

EDIT: This looks like somehow #3314523: PostCSS results in CSS that does not comply with our coding standards got reverted, as already said by @mherchell in #7, but it appears unrelated to updating the packages in this issue, since it's already happening without any updates.

spokje’s picture

If I revert #3344087: Update Stylelint and Prettier for Drupal 10.1 and use Prettier for formatting PostCSS and run yarn build:css I get the same CSS as in the HEAD.
So it looks like that issue broke something

Also, we don't seem to be running yarn build:css whenever core/package.json and/or core/package.lock changes in our commit checks, which would have caught the above, me thinks.

longwave’s picture

Status: Needs review » Needs work

I've reverted #3344087: Update Stylelint and Prettier for Drupal 10.1 and use Prettier for formatting PostCSS. Stylelint 15 has deprecated indentation and other opinionated style rules, and they have been removed from stylelint-config-standard 30. This is why the reformatting no longer works. We will have to migrate to Prettier or similar to continue reformatting our CSS.

spokje’s picture

Let's wait until #3344087: Update Stylelint and Prettier for Drupal 10.1 and use Prettier for formatting PostCSS is committed before proceeding here, to prevent endless rerolling because of conflicts in yarn.lock.

gauravvvv’s picture

Status: Needs work » Needs review
mherchel’s picture

Status: Needs review » Needs work

This is still adding the unnecessary -webkit-text-decoration properties to the CSS. Haven't looked at why, but this is not needed. Webkit has support the unprefixed version since 2014.

longwave’s picture

Status: Needs work » Needs review

@mherchel this is to work around a bug in Safari, see https://github.com/postcss/autoprefixer/issues/1473

mherchel’s picture

OMG Safari 🤦‍♂️

Will look at this closely today (if no one beats me to it)

mherchel’s picture

Status: Needs review » Reviewed & tested by the community

This looks good! I pulled down the patch, and verified the CSS (paying special attention to the changes in Olivero's variables.css).

I also verified no changes to the CSS when I do a yarn build:css, and I also verified the yarn watch:css functionality.

I also verified that the new syntax of CSS nesting works (removes @nest, & not always needed, etc). At some point we'll convert to that.

Note that I have a couple questions upstream that don't affect us quite yet, and I opened up an issue at https://github.com/csstools/postcss-plugins/issues/958

  • catch committed 0bfcd0a6 on 10.1.x
    Issue #3351619 by Gauravvvv, longwave, Spokje, mherchel, smustgrave:...

  • catch committed 3a34afe9 on 11.x
    Issue #3351619 by Gauravvvv, longwave, Spokje, mherchel, smustgrave:...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 10.1.x, thanks!

Status: Fixed » Closed (fixed)

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