fix(core): fix hash collisions with at rules (#560)

* fix(core): fix hash collisions with at rules * Update packages/core/src/runtime/utils/hashPropertyKey.ts Co-authored-by: ling1726 <lingfangao@hotmail.com> --------- Co-authored-by: ling1726 <lingfangao@hotmail.com>
microsoft · May 21, 2024 · 80ca330 · 80ca330
1 parent f545363
commit 80ca330
Show file tree

Hide file tree

Showing 11 changed files with 83 additions and 47 deletions.
diff --git a/change/@griffel-core-3f1edf4d-9560-425a-bd42-e112f2ced10f.json b/change/@griffel-core-3f1edf4d-9560-425a-bd42-e112f2ced10f.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "fix(core): fix hash collisions with at rules",
+  "packageName": "@griffel/core",
+  "email": "olfedias@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/e2e/rspack/src/snapshots/output.css b/e2e/rspack/src/snapshots/output.css
@@ -25,7 +25,7 @@
   outline-offset: 5px;
 }
 @media (min-width: 968px) and (orientation: landscape) {
-  .fnj14hp {
+  .f1tnsuik {
     width: 400px;
   }
 }
diff --git a/packages/babel-preset/__fixtures__/at-rules/output.ts b/packages/babel-preset/__fixtures__/at-rules/output.ts
@@ -2,21 +2,21 @@ import { __styles } from '@griffel/react';
 export const useStyles = __styles(
   {
     root: {
-      cvzfql: ['f13wceqr', 'fmtq1hn'],
-      g42fl4: ['fmtq1hn', 'f13wceqr'],
-      B6rh290: ['f16pi78z', 'fem5dos'],
-      rth2jb: ['fem5dos', 'f16pi78z'],
+      wbkd2a: ['fgefjy', 'f1h4p6a1'],
+      Dt0rau: ['f1h4p6a1', 'fgefjy'],
+      k2kg1z: ['f1dyc5nu', 'f1kq3x14'],
+      Blnc60a: ['f1kq3x14', 'f1dyc5nu'],
     },
   },
   {
     m: [
       [
-        '@media (min-width: 600px){.f13wceqr{padding-left:4px;}.fmtq1hn{padding-right:4px;}}',
+        '@media (min-width: 600px){.f1h4p6a1{padding-right:4px;}.fgefjy{padding-left:4px;}}',
         {
           m: '(min-width: 600px)',
         },
       ],
     ],
-    t: ['@supports (display: flex){.f16pi78z{padding-left:4px;}.fem5dos{padding-right:4px;}}'],
+    t: ['@supports (display: flex){.f1dyc5nu{padding-left:4px;}.f1kq3x14{padding-right:4px;}}'],
   },
 );
diff --git a/packages/babel-preset/__fixtures__/rules-with-metadata/output.ts b/packages/babel-preset/__fixtures__/rules-with-metadata/output.ts
@@ -2,13 +2,13 @@ import { __styles } from '@griffel/react';
 export const useStyles = __styles(
   {
     media: {
-      Bulngiv: 'fr5o61b',
+      Bn3jx5e: 'ful25cn',
     },
   },
   {
     m: [
       [
-        '@media screen and (max-width: 100px){.fr5o61b{color:red;}}',
+        '@media screen and (max-width: 100px){.ful25cn{color:red;}}',
         {
           m: 'screen and (max-width: 100px)',
         },

diff --git a/packages/core/src/runtime/resolveStyleRules.test.ts b/packages/core/src/runtime/resolveStyleRules.test.ts
@@ -319,10 +319,10 @@ describe('resolveStyleRules', () => {
         }),
       ).toMatchInlineSnapshot(`
         @media screen and (max-width: 992px) {
-          .f133ge8t {
+          .f1avj2xq {
             text-align: right;
           }
-          .f1f6067w {
+          .f1bxmk0a {
             text-align: left;
           }
         }
@@ -496,14 +496,14 @@ describe('resolveStyleRules', () => {
 
       expect(result).toMatchInlineSnapshot(`
         @container foo (max-width: 1px) {
-          .f16pkaez {
+          .fbmp7kx {
             color: red;
           }
         }
       `);
       expect(result[0]).toMatchInlineSnapshot(`
         Object {
-          "Blqc43h": "f16pkaez",
+          "b7rg5g": "fbmp7kx",
         }
       `);
     });
@@ -513,14 +513,14 @@ describe('resolveStyleRules', () => {
 
       expect(result).toMatchInlineSnapshot(`
         @container (max-width: 1px) {
-          .f15adh02 {
+          .f4ivup9 {
             color: red;
           }
         }
       `);
       expect(result[0]).toMatchInlineSnapshot(`
         Object {
-          "qc8hou": "f15adh02",
+          "pmeytk": "f4ivup9",
         }
       `);
     });
@@ -536,14 +536,14 @@ describe('resolveStyleRules', () => {
           color: red;
         }
         @container foo (max-width: 1px) {
-          .f16pkaez {
+          .fbmp7kx {
             color: red;
           }
         }
       `);
       expect(result[0]).toMatchInlineSnapshot(`
         Object {
-          "Blqc43h": "f16pkaez",
+          "b7rg5g": "fbmp7kx",
           "sj55zd": "fe3e8s9",
         }
       `);
@@ -560,7 +560,7 @@ describe('resolveStyleRules', () => {
           color: green;
         }
         @media screen and (max-width: 992px) {
-          .f1ojdyje {
+          .f15as2e {
             color: red;
           }
         }
@@ -582,7 +582,7 @@ describe('resolveStyleRules', () => {
           color: green;
         }
         @media screen and (max-width: 992px) {
-          .f7wpa5l:hover {
+          .f1951wvx:hover {
             color: red;
           }
         }
@@ -603,12 +603,12 @@ describe('resolveStyleRules', () => {
           color: red;
         }
         @media screen and (max-width: 992px) {
-          .f1ojdyje {
+          .f15as2e {
             color: red;
           }
         }
         @media screen and (max-width: 992px) and (min-width: 100px) {
-          .f19a6424 {
+          .fnnlhvt {
             color: red;
           }
         }
@@ -626,7 +626,7 @@ describe('resolveStyleRules', () => {
           color: green;
         }
         @layer color {
-          .f1hjcal7 {
+          .f1al6es7 {
             color: red;
           }
         }
@@ -640,7 +640,7 @@ describe('resolveStyleRules', () => {
         }),
       ).toMatchInlineSnapshot(`
         @layer framework.utilities {
-          .faxdetk {
+          .f12ei13l {
             color: red;
           }
         }
@@ -661,7 +661,7 @@ describe('resolveStyleRules', () => {
           color: green;
         }
         @layer color {
-          .f1jv0g3z:hover {
+          .f1hrbkey:hover {
             color: red;
           }
         }
@@ -682,12 +682,12 @@ describe('resolveStyleRules', () => {
           color: red;
         }
         @layer color {
-          .f1hjcal7 {
+          .f1al6es7 {
             color: red;
           }
         }
         @layer color.theme {
-          .f132c7g3 {
+          .f1mzin3h {
             color: red;
           }
         }
@@ -701,7 +701,7 @@ describe('resolveStyleRules', () => {
         }),
       ).toMatchInlineSnapshot(`
         @supports (display: block) {
-          .f1yofsfp {
+          .fp97nsu {
             color: green;
           }
         }
@@ -985,8 +985,8 @@ describe('resolveStyleRules', () => {
 
       expect(result[1]).toEqual({
         m: [
-          ['@media screen{.f101iwbs{color:red;}}', { m: 'screen' }],
-          ['@media screen{.fqfls7b{padding:10px;}}', { m: 'screen', p: -1 }],
+          ['@media screen{.f12pk7j4{color:red;}}', { m: 'screen' }],
+          ['@media screen{.f1eoyco2{padding:10px;}}', { m: 'screen', p: -1 }],
         ],
       });
     });

diff --git a/packages/core/src/runtime/utils/hashClassName.test.ts b/packages/core/src/runtime/utils/hashClassName.test.ts
@@ -19,6 +19,15 @@ describe('hashClassName', () => {
     expect(hashClassName(defaultOptions, defaultAtRules)).toMatchInlineSnapshot(`"fe3e8s9"`);
   });
 
+  it('should generate non-colliding hashes', () => {
+    const hashA = hashClassName(defaultOptions, { ...defaultAtRules, container: '(min-width: 500px)' });
+    const hashB = hashClassName(defaultOptions, { ...defaultAtRules, media: '(min-width: 500px)' });
+
+    expect(hashA).toMatchInlineSnapshot(`"f18ymuke"`);
+    expect(hashB).toMatchInlineSnapshot(`"f1hoxic9"`);
+    expect(hashA).not.toBe(hashB);
+  });
+
   it('should use salt for hash', () => {
     const withoutSalt = hashClassName(defaultOptions, defaultAtRules);
     const withSalt = hashClassName({ ...defaultOptions, salt: 'HASH_SALT' }, defaultAtRules);

diff --git a/packages/core/src/runtime/utils/hashClassName.ts b/packages/core/src/runtime/utils/hashClassName.ts
@@ -1,6 +1,7 @@
 import hashString from '@emotion/hash';
 
 import { HASH_PREFIX } from '../../constants';
+import { atRulesToString } from './hashPropertyKey';
 import type { AtRules } from './types';
 
 interface HashedClassNameParts {
@@ -16,10 +17,7 @@ export function hashClassName({ property, selector, salt, value }: HashedClassNa
     hashString(
       salt +
         selector +
-        atRules.container +
-        atRules.media +
-        atRules.layer +
-        atRules.supports +
+        atRulesToString(atRules) +
         property +
         // Trimming of value is required to generate consistent hashes
         value.trim(),

diff --git a/packages/core/src/runtime/utils/hashPropertyKey.test.ts b/packages/core/src/runtime/utils/hashPropertyKey.test.ts
@@ -1,13 +1,22 @@
 import { hashPropertyKey } from './hashPropertyKey';
 
+const defaultAtRules = { container: '', media: '', supports: '', layer: '' };
+
 describe('hashPropertyKey', () => {
   it('generates hashes that always start with letters', () => {
-    const atRules = { container: '', media: '', supports: '', layer: '' };
+    expect(hashPropertyKey('', 'color', defaultAtRules)).toBe('sj55zd');
+    expect(hashPropertyKey('', 'display', defaultAtRules)).toBe('mc9l5x');
+
+    expect(hashPropertyKey('', 'backgroundColor', defaultAtRules)).toBe('De3pzq');
+    expect(hashPropertyKey(':hover', 'color', defaultAtRules)).toBe('Bi91k9c');
+  });
 
-    expect(hashPropertyKey('', 'color', atRules)).toBe('sj55zd');
-    expect(hashPropertyKey('', 'display', atRules)).toBe('mc9l5x');
+  it('generates non-colliding hashes', () => {
+    const hashA = hashPropertyKey('', 'color', { ...defaultAtRules, container: '(min-width: 500px)' });
+    const hashB = hashPropertyKey('', 'color', { ...defaultAtRules, media: '(min-width: 500px)' });
 
-    expect(hashPropertyKey('', 'backgroundColor', atRules)).toBe('De3pzq');
-    expect(hashPropertyKey(':hover', 'color', atRules)).toBe('Bi91k9c');
+    expect(hashA).toMatchInlineSnapshot(`"Bhkzl7a"`);
+    expect(hashB).toMatchInlineSnapshot(`"B3v6anr"`);
+    expect(hashA).not.toBe(hashB);
   });
 });
diff --git a/packages/core/src/runtime/utils/hashPropertyKey.ts b/packages/core/src/runtime/utils/hashPropertyKey.ts
@@ -3,9 +3,22 @@ import hash from '@emotion/hash';
 import type { PropertyHash } from '../../types';
 import type { AtRules } from './types';
 
+function addAtRulePrefix(atRule: string, prefix: string): string {
+  return atRule ? prefix + atRule : atRule;
+}
+
+export function atRulesToString(atRules: AtRules): string {
+  return (
+    addAtRulePrefix(atRules.container, 'c') +
+    addAtRulePrefix(atRules.media, 'm') +
+    addAtRulePrefix(atRules.layer, 'l') +
+    addAtRulePrefix(atRules.supports, 's')
+  );
+}
+
 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;
 
   // "key" can be really long as it includes selectors, we use hashes to reduce sizes of keys
   // ".foo :hover" => "abcd"

diff --git a/packages/react/src/createDOMRenderer.test.tsx b/packages/react/src/createDOMRenderer.test.tsx
@@ -75,7 +75,7 @@ describe('createDOMRenderer', () => {
         ".rp2atum:hover{color:blue;}": "r",
         ".rp2atum{color:red;}": "r",
         "@keyframes f1kgwxhb{from{height:10px;}to{height:20px;}}": "k",
-        "@media screen and (max-width: 992px){.fnao3vb:hover{color:blue;}}": "m",
+        "@media screen and (max-width: 992px){.fzd6x39:hover{color:blue;}}": "m",
       }
     `);
     // There is no DOM on a server, style nodes should not be present

diff --git a/packages/react/src/renderToStyleElements-node.test.tsx b/packages/react/src/renderToStyleElements-node.test.tsx
@@ -112,7 +112,7 @@ describe('renderToStyleElements (node)', () => {
           data-make-styles-rehydration="true"
         >
           @supports (display: grid) {
-            .f1ofq0jl {
+            .fo1gfrc {
               color: red;
             }
           }</style
@@ -123,7 +123,7 @@ describe('renderToStyleElements (node)', () => {
           data-make-styles-rehydration="true"
         >
           @media screen and (max-width: 992px) {
-            .fnao3vb:hover {
+            .fzd6x39:hover {
               color: blue;
             }
           }
@@ -186,7 +186,7 @@ describe('renderToStyleElements (node)', () => {
           data-make-styles-rehydration="true"
         >
           @supports (display: grid) {
-            .f1vq01kz {
+            .fui0tgz {
               color: green;
             }
           }</style
@@ -197,7 +197,7 @@ describe('renderToStyleElements (node)', () => {
           data-make-styles-rehydration="true"
         >
           @media (max-width: 1px) {
-            .f1f7njb2:hover {
+            .f13d6lhy:hover {
               color: blue;
             }
           }</style
@@ -208,7 +208,7 @@ describe('renderToStyleElements (node)', () => {
           data-make-styles-rehydration="true"
         >
           @media (max-width: 2px) {
-            .f1c6999y:hover {
+            .f1b07yzi:hover {
               color: blue;
             }
           }</style
@@ -219,7 +219,7 @@ describe('renderToStyleElements (node)', () => {
           data-make-styles-rehydration="true"
         >
           @media (max-width: 3px) {
-            .f1qdcc3n:hover {
+            .f1cy3850:hover {
               color: blue;
             }
           }</style
@@ -230,7 +230,7 @@ describe('renderToStyleElements (node)', () => {
           data-make-styles-rehydration="true"
         >
           @media (max-width: 4px) {
-            .f1b4up97:hover {
+            .fyvg8w:hover {
               color: blue;
             }
           }