Skip to content

optimize(postcss-cascade-layers): shorter output #789

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

Closed
wants to merge 4 commits into from

Conversation

CGQAQ
Copy link
Contributor

@CGQAQ CGQAQ commented Jan 16, 2023

I found that in the middle you don't have to be id

image

~ npm run test 
> @csstools/postcss-cascade-layers@1.1.1 test
> node .tape.mjs && npm run test:exports

pass postcss-cascade-layers

> @csstools/postcss-cascade-layers@1.1.1 test:exports
> node ./test/_import.mjs && node ./test/_require.cjs
~ npm run test:browser

> @csstools/postcss-cascade-layers@1.1.1 test:browser
> node ./test/_browser.mjs

@romainmenke
Copy link
Member

This is incorrect.
These need to be ID selectors.

Is there something specific you are trying to fix by making these shorter?

@CGQAQ
Copy link
Contributor Author

CGQAQ commented Jan 16, 2023

This is incorrect. These need to be ID selectors.

Is there something specific you are trying to fix by making these shorter?

No, just wondering could it be shorter. do you have any test cases that will failing in this PR? Thanks.

@romainmenke
Copy link
Member

npm run test:browser runs test sourced from WPT in Chrome.
These fail with this change :)

@CGQAQ
Copy link
Contributor Author

CGQAQ commented Jan 16, 2023

no, it works

@romainmenke
Copy link
Member

Ah, yes, bad copy paste on my end :)
I will add a test that will fail with this change

@romainmenke
Copy link
Member

romainmenke commented Jan 16, 2023

I've added the test and updated this branch.
fb9d0ce

You should now get this error:

Error: Evaluation failed: Error: 3 ID specificity (processed), target 'first': expected rgb(0, 128, 0), got rgb(255, 0, 0)

@CGQAQ
Copy link
Contributor Author

CGQAQ commented Jan 16, 2023

yeah! you are right

@CGQAQ CGQAQ closed this Jan 16, 2023
@CGQAQ CGQAQ deleted the layer branch January 16, 2023 08:14
@romainmenke
Copy link
Member

We do appreciate you looking into this and taking the time to try and improve the plugin ❤️

I think the suggestion by @yisibl to dynamically switch to :not(<compound-selector>) has the most promise :)

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

Successfully merging this pull request may close these issues.

2 participants