Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update dependencies and postcss config #234

Closed
wants to merge 4 commits into from

Conversation

romainmenke
Copy link
Contributor

@romainmenke romainmenke commented May 29, 2022

fixes : #233

But likely also breaks things as postcss-preset-env was updated.
I am biased but postcss-preset-env v6.7.0 is getting old and has too many unresolved bugs :)

The most important thing to verify is how all the nested CSS is transformed with this new version. (6.7.0 was not spec compliant)

'logical-properties-and-values': false,
'prefers-color-scheme-query': false,
'gap-properties': false,
'cascade-layers': false,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only works well for final products, not for libraries

@@ -14,17 +14,16 @@ module.exports = {
postcssPresetEnv({
stage: 0,
autoprefixer: false,
enableClientSidePolyfills: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fairly recent plugin option that disables all plugins that need client side JS to function (:has, :focus-within, ...)

@argyleink
Copy link
Owner

confirmed fix in normalize output! thanks so much ❤️

i did however see a few other changes in the normalize output. I couldn't put a thumb on the cause, plugins like selector merging are involved, but just a straight unminfied comparison of normalize shows just a /:has/ selector being removed cuz of the new flag to remove js plugins (cool bool btw). so i'm a little tentative to merge this pr, but i do believe an update to latest in due 👍🏻 thanks for kickin it off

i also tried just updating cssnano, and that's enough to fix #233. what do you think of a pr for updating cssnano and then this one becomes updating preset-env?

@romainmenke
Copy link
Contributor Author

see : #235

i also tried just updating cssnano, and that's enough to fix #233. what do you think of a pr for updating cssnano and then this one becomes updating preset-env?

That would be better :)

I got a bit carried away with this change, but getting a fix out first for the weird minified result will unblock some people.

@argyleink
Copy link
Owner

think its worth updating the other postcss packages while this is updating preset-env?

@argyleink
Copy link
Owner

it's very clear there are bytes to shave off my files by inspecting and managing the output better.. just spent 45m making builds of normalize with old and new postcss/preset-env/etc.

@romainmenke
Copy link
Contributor Author

think its worth updating the other postcss packages while this is updating preset-env?

npm outdated      
Package             Current  Wanted  Latest  Location                         Depended by
ava                  3.15.0  3.15.0   4.2.0  node_modules/ava                 open-props
concurrently          6.5.1   6.5.1   7.2.1  node_modules/concurrently        open-props
microbundle          0.13.3  0.13.3  0.15.0  node_modules/microbundle         open-props
postcss-cli           8.3.1   8.3.1   9.1.0  node_modules/postcss-cli         open-props
postcss-preset-env    7.6.0   7.7.0   7.7.0  node_modules/postcss-preset-env  open-props

there was a release of postcss-preset-env yesterday with support for sin(), cos(), ...

postcss-cli can be updated safely if I compare how it's used here with the changelog : https://github.com/postcss/postcss-cli/blob/master/CHANGELOG.md#900--2021-09-24


just spent 45m making builds of normalize with old and new postcss/preset-env/etc.

Was there anything unexpected there?
Always interesting to have feedback based on real world CSS :)

@@ -153,6 +153,6 @@
"postcss-cli": "^8.3.1",
"postcss-combine-duplicated-selectors": "^10.0.3",
"postcss-import": "^14.0.2",
"postcss-preset-env": "6.7.x"
"postcss-preset-env": "^7.6.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clamp

https://github.com/polemius/postcss-clamp#readme

I would propose disabling this plugin and leave it up to users to add a fallback for this feature. It only benefits a specific Safari version that has max/min but no clamp

diff --git a/sizes.min.css b/sizes.min.css
-  --size-fluid-1: clamp(.5rem, 1vw, 1rem);
+  --size-fluid-1: max(.5rem, min(1vw, 1rem));

double position gradients

at keyword was incorrectly handled before, breaking gradients.
The removed declaration is a broken "fallback".

This has been fixed.

-  --gradient-8: conic-gradient(from 90deg at 50%, at 0%, #111, 50%, #222, #111);
   --gradient-8: conic-gradient(from 90deg at 50% 0%, #111, 50%, #222, #111);

:has pseudo class

:has() conversion was enabled but this requires a clients side polyfill.
Best left up to users and this is now disabled in PostCSS config.

-:where(html[\:has\(dialog\[open\]\)]) {
-  overflow: hidden;
-}
 :where(html:has(dialog[open])) {
   overflow: hidden;
 }

:is pseudo class

A new plugin was added for :is pseudo class transforms.

I do think there is typo in the source code here as these selectors have specificity [0, 2, 0]

 /* submit, form > button, reset matching hover border color */
-:where([type="submit"], [type="reset"], form button:not([type])):is(:hover, :focus-visible):not([disabled]) {
+:where( form button:not([type]),[type="submit"], [type="reset"]):hover:not([disabled]) {
+  --_border: currentColor;
+}
+:where( form button:not([type]),[type="submit"], [type="reset"]):focus-visible:not([disabled]) {
   --_border: currentColor;
 }

selector order

Plugins like nesting, :is, :has, ... that do radical selector transforms and mutations re-order selectors to prevent issues like .foo:is(a) -> .fooa

This might cause attribute selectors to be moved in front of pseudo classes.
They must all be equivalent, if they are not we have a bug somewhere.

-:where(button,button[type],input[type="button"],input[type="submit"],input[type="reset"])[disabled] {
+[disabled]:where(button,button[type],input[type="button"],input[type="submit"],input[type="reset"]) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a bug rule with selector :where(html) { in normalize.min.css that is moved down a few rules.

I can not think of anything postcss-preset-env related that could cause this.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disable postcss-clamp 👍🏻

I do think there is typo in the source code here..

appreciate the note! i have a refactored version i've been meaning to swap out for the button styles, i wonder if i still have an issue in there too. thanks!

bug with lines moved down a few rules in normalize.min.css..

i chased that one too, not even sure how to work around it yet

@argyleink
Copy link
Owner

will give this another checkout and build pass, get back to you soon. thanks!

@equinusocio
Copy link
Contributor

equinusocio commented Nov 28, 2022

I think this PR is important and should get attention. Providing code from old postcss-preset-env version may result in useless and bloated code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSSNano bug 1402 is affecting normalize.css
3 participants