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(core): fix hash collisions with at rules #560

Merged
merged 3 commits into from
May 21, 2024

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented May 21, 2024

See issues for reproductions:

Adds prefixes for at rules in hashes so they will be unique.

Copy link

github-actions bot commented May 21, 2024

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
core
makeStyles (runtime)
25.976 kB
9.279 kB
26.043 kB
9.3 kB
67 B
21 B
react
makeStyles (runtime)
28.481 kB
10.267 kB
28.548 kB
10.298 kB
67 B
31 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
core
__resetStyles (makeResetStyles)
273 B
197 B
core
__styles (makeStyles)
1.727 kB
832 B
core
makeResetStyles (runtime)
17.21 kB
6.588 kB
core
mergeClasses
1.865 kB
900 B
core
shorthands.padding()
4.781 kB
1.537 kB
react
__css
1.723 kB
812 B
react
__styles
4.187 kB
1.832 kB
react
makeResetStyles (runtime)
19.71 kB
7.578 kB
react
makeStaticStyles (runtime)
9.638 kB
4.197 kB
shadow-dom
createShadowDOMRenderer
4.085 kB
1.724 kB
🤖 This report was generated against f545363cc35245722381f1ec1524f60b7175f3d1

@layershifter layershifter marked this pull request as ready for review May 21, 2024 10:29
@layershifter layershifter requested a review from a team as a code owner May 21, 2024 10:29
export function hashPropertyKey(selector: string, property: string, atRules: AtRules): PropertyHash {
// uniq key based on property & selector, used for merging later
const computedKey = selector + atRules.container + atRules.media + atRules.supports + property;
const computedKey = selector + atRulesToString(atRules) + property;
Copy link
Member

Choose a reason for hiding this comment

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

previously layer queries were not supported in hashPropertyKey I guess it was unintentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it's a bug #561.

@layershifter layershifter force-pushed the fix/hash-collisions branch 3 times, most recently from b7d0560 to 6e18794 Compare May 21, 2024 17:17
@layershifter layershifter merged commit 80ca330 into microsoft:main May 21, 2024
4 checks passed
@layershifter layershifter deleted the fix/hash-collisions branch May 21, 2024 17:46
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.

core: @layer is not part of property hash core: hashes for @media & @container can collide
2 participants