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

fix: ensure unique classNames for pseudo classes & elems #750

Merged
merged 4 commits into from
Oct 16, 2024

Conversation

nmn
Copy link
Contributor

@nmn nmn commented Oct 15, 2024

What changed

We recently fixed an issue with how pseudo selectors are sorted in StyleX to ensure we don't accidentally mix up pseudo classes and pseudo elements incorrectly.

However, while we fixed the generated rules in the previous PR, the string hashed to generate a unique className was still using the old logic. This would mean that, e.g. if you used both ::before:hover and :hover::before selectors in your styles for the exact same property and value, both rules would generate the same className.

(TODO: Create a separate commit to actually demonstrate this edge-case in a test)

This PR updates the logic used when generating the className to match the logic used when generating the CSS rule to fix this issue.

Fixes #749

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 15, 2024
Copy link

github-actions bot commented Oct 15, 2024

workflow: benchmarks/size

Comparison of minified (terser) and compressed (brotli) size results, measured in bytes. Smaller is better.

@stylexjs/scripts@0.8.0 size:compare
./size-compare.js /tmp/tmp.LuB14pXxuu /tmp/tmp.lh3ryT1kwc

Results Base Patch Ratio
stylex/lib/stylex.js
· compressed 729 729 1.00
· minified 2,541 2,541 1.00
stylex/lib/StyleXSheet.js
· compressed 1,266 1,266 1.00
· minified 3,776 3,776 1.00
rollup-example/.build/bundle.js
· compressed 563,025 563,025 1.00
· minified 10,185,368 10,185,368 1.00
rollup-example/.build/stylex.css
· compressed 99,154 99,154 1.00
· minified 745,649 745,649 1.00

@nmn nmn force-pushed the fix/pseudos-class-uniqueness branch from 4d36e77 to fec65e8 Compare October 15, 2024 01:51
@nmn nmn changed the base branch from main to chore/repro-bug October 15, 2024 01:51
Base automatically changed from chore/repro-bug to main October 16, 2024 07:11
@nmn nmn force-pushed the fix/pseudos-class-uniqueness branch from 3419d05 to 3b6a130 Compare October 16, 2024 07:12
@nmn nmn merged commit 1357106 into main Oct 16, 2024
8 checks passed
@nmn nmn deleted the fix/pseudos-class-uniqueness branch October 16, 2024 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential hash conflicts in Classname generator
2 participants