Repository: microsoft/griffel (ts)

Issue:
core: ensure that there no multiple occupancies of classes produced by makeResetStyles in mergeClasses calls 
See #225 for details.

We should add a runtime validation to `mergeClasses()` to ensure that the following scenario is impossible:

```tsx
const useClassNameA = makeResetStyles()
const useClassNameB = makeResetStyles()

function App() {
  const classNameA = useClassNameA()
  const classNameB = useClassNameB()

  return <div className={mergeClasses(classNameA, classNameB)} />
}
```
jest-serializer: implement makeResetStyles support
Our Jest serializer does not handle `makeResetStyles` calls and keeps its classes (discovered in #25216).

See #225 for more details about `makeResetStyles`.

Pull request description:
feat: add validation into mergeClasses for makeResetStyles calls
Fixes #261.
Fixes #230.

This PR modifies:
- `mergeClasses()` in Griffel core to emit errors if a call contains multiple occupancies of classes from `makeResetStyles`.
- Jest serializer to handle classes from `makeResetStyles`

Both changes uses new _global_ variable `DEBUG_RESET_CLASSES` to store classes. This works similarly to `DEFINITION_LOOKUP_TABLE`, but we need `DEBUG_RESET_CLASSES` only in development/tests so it has zero impact on production bundles.

Patch:
diff --git a/change/@griffel-core-f18356ed-6d74-4218-a44d-df648ef511ac.json b/change/@griffel-core-f18356ed-6d74-4218-a44d-df648ef511ac.json
new file mode 100644
index 0000000..6afb179
--- /dev/null
+++ b/change/@griffel-core-f18356ed-6d74-4218-a44d-df648ef511ac.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "feat: add validation into mergeClasses for makeResetStyles calls",
+  "packageName": "@griffel/core",
+  "email": "olfedias@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@griffel-jest-serializer-d855a4d4-e4c6-4f70-a258-c22caa4992ea.json b/change/@griffel-jest-serializer-d855a4d4-e4c6-4f70-a258-c22caa4992ea.json
new file mode 100644
index 0000000..019ab30
--- /dev/null
+++ b/change/@griffel-jest-serializer-d855a4d4-e4c6-4f70-a258-c22caa4992ea.json
@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "feat: add support for makeResetStyles",
+  "packageName": "@griffel/jest-serializer",
+  "email": "olfedias@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@griffel-react-0c417873-8b81-4a7e-b68a-6f5fc2eb229b.json b/change/@griffel-react-0c417873-8b81-4a7e-b68a-6f5fc2eb229b.json
new file mode 100644
index 0000000..6ca81d0
--- /dev/null
+++ b/change/@griffel-react-0c417873-8b81-4a7e-b68a-6f5fc2eb229b.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "fix: add export for __resetCSS",
+  "packageName": "@griffel/react",
+  "email": "olfedias@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/core/src/__resetCSS.ts b/packages/core/src/__resetCSS.ts
index 8bec570..35fea30 100644
--- a/packages/core/src/__resetCSS.ts
+++ b/packages/core/src/__resetCSS.ts
@@ -1,3 +1,4 @@
+import { DEBUG_RESET_CLASSES } from './constants';
 import type { MakeStylesOptions } from './types';
 
 /**
@@ -6,8 +7,13 @@ import type { MakeStylesOptions } from './types';
 export function __resetCSS(ltrClassName: string, rtlClassName: string | null) {
   function computeClassName(options: Pick<MakeStylesOptions, 'dir'>): string {
     const { dir } = options;
+    const className = dir === 'ltr' ? ltrClassName : rtlClassName || ltrClassName;
 
-    return dir === 'ltr' ? ltrClassName : rtlClassName || ltrClassName;
+    if (process.env.NODE_ENV !== 'production') {
+      DEBUG_RESET_CLASSES[className] = 1;
+    }
+
+    return className;
   }
 
   return computeClassName;
diff --git a/packages/core/src/__resetStyles.ts b/packages/core/src/__resetStyles.ts
index e65b846..cd4c86e 100644
--- a/packages/core/src/__resetStyles.ts
+++ b/packages/core/src/__resetStyles.ts
@@ -1,3 +1,4 @@
+import { DEBUG_RESET_CLASSES } from './constants';
 import type { MakeStylesOptions } from './types';
 
 /**
@@ -18,7 +19,13 @@ export function __resetStyles(ltrClassName: string, rtlClassName: string | null,
       insertionCache[rendererId] = true;
     }
 
-    return isLTR ? ltrClassName : rtlClassName || ltrClassName;
+    const className = isLTR ? ltrClassName : rtlClassName || ltrClassName;
+
+    if (process.env.NODE_ENV !== 'production') {
+      DEBUG_RESET_CLASSES[className] = 1;
+    }
+
+    return className;
   }
 
   return computeClassName;
diff --git a/packages/core/src/constants.ts b/packages/core/src/constants.ts
index 77f71ea..87f6e4a 100644
--- a/packages/core/src/constants.ts
+++ b/packages/core/src/constants.ts
@@ -24,6 +24,9 @@ export const SEQUENCE_SIZE =
     ? SEQUENCE_PREFIX.length + SEQUENCE_HASH_LENGTH
     : SEQUENCE_PREFIX.length + SEQUENCE_HASH_LENGTH + DEBUG_SEQUENCE_SEPARATOR.length + SEQUENCE_HASH_LENGTH;
 
+/** @internal */
+export const DEBUG_RESET_CLASSES: Record<string, 1> = {};
+
 /** @internal */
 export const DEFINITION_LOOKUP_TABLE: Record<SequenceHash, LookupItem> = {};
 
diff --git a/packages/core/src/makeResetStyles.ts b/packages/core/src/makeResetStyles.ts
index 717b28f..0b64094 100644
--- a/packages/core/src/makeResetStyles.ts
+++ b/packages/core/src/makeResetStyles.ts
@@ -1,9 +1,7 @@
+import { DEBUG_RESET_CLASSES } from './constants';
 import { resolveResetStyleRules } from './runtime/resolveResetStyleRules';
 import type { GriffelResetStyle, MakeStylesOptions } from './types';
 
-/**
- * @internal
- */
 export function makeResetStyles(styles: GriffelResetStyle) {
   const insertionCache: Record<string, boolean> = {};
 
@@ -28,7 +26,13 @@ export function makeResetStyles(styles: GriffelResetStyle) {
       insertionCache[rendererId] = true;
     }
 
-    return isLTR ? ltrClassName : rtlClassName || ltrClassName;
+    const className = isLTR ? ltrClassName : rtlClassName || ltrClassName;
+
+    if (process.env.NODE_ENV !== 'production') {
+      DEBUG_RESET_CLASSES[className] = 1;
+    }
+
+    return className;
   }
 
   return computeClassName;
diff --git a/packages/core/src/mergeClasses.ts b/packages/core/src/mergeClasses.ts
index d780977..3da1c7b 100644
--- a/packages/core/src/mergeClasses.ts
+++ b/packages/core/src/mergeClasses.ts
@@ -1,7 +1,9 @@
 import {
+  DEBUG_RESET_CLASSES,
   DEFINITION_LOOKUP_TABLE,
   LOOKUP_DEFINITIONS_INDEX,
   LOOKUP_DIR_INDEX,
+  RESET_HASH_PREFIX,
   SEQUENCE_PREFIX,
   SEQUENCE_SIZE,
 } from './constants';
@@ -43,6 +45,8 @@ export function mergeClasses(): string {
   let sequenceMatch = '';
   const sequencesIds: (SequenceHash | undefined)[] = new Array(arguments.length);
 
+  let containsResetClassName = '';
+
   for (let i = 0; i < arguments.length; i++) {
     const className = arguments[i];
 
@@ -52,6 +56,25 @@ export function mergeClasses(): string {
       const sequenceIndex = className.indexOf(SEQUENCE_PREFIX);
 
       if (sequenceIndex === -1) {
+        if (process.env.NODE_ENV !== 'production') {
+          className.split(' ').forEach(entry => {
+            if (entry.startsWith(RESET_HASH_PREFIX) && DEBUG_RESET_CLASSES[entry]) {
+              if (containsResetClassName) {
+                // eslint-disable-next-line no-console
+                console.error(
+                  'mergeClasses(): a passed string contains multiple classes produced by makeResetStyles (' +
+                    `${className} & ${resultClassName}, this will lead to non-deterministic behavior. Learn more:` +
+                    'https://griffel.js.org/react/api/make-reset-styles#limitations' +
+                    '\n' +
+                    `Source string: ${className}`,
+                );
+              } else {
+                containsResetClassName = entry;
+              }
+            }
+          });
+        }
+
         resultClassName += className + ' ';
       } else {
         const sequenceId = className.substr(sequenceIndex, SEQUENCE_SIZE);
diff --git a/packages/jest-serializer/src/index.ts b/packages/jest-serializer/src/index.ts
index ffb2448..adf0f0b 100644
--- a/packages/jest-serializer/src/index.ts
+++ b/packages/jest-serializer/src/index.ts
@@ -1,4 +1,4 @@
-import { DEFINITION_LOOKUP_TABLE, CSSClasses } from '@griffel/core';
+import { DEBUG_RESET_CLASSES, DEFINITION_LOOKUP_TABLE, CSSClasses } from '@griffel/core';
 
 export function print(val: unknown) {
   /**
@@ -17,6 +17,12 @@ export function print(val: unknown) {
 
   while ((result = regex.exec(_val))) {
     const [name] = result;
+
+    if (DEBUG_RESET_CLASSES[name]) {
+      regexParts.push(name);
+      continue;
+    }
+
     const [definitions] = DEFINITION_LOOKUP_TABLE[name];
 
     /**
@@ -34,7 +40,7 @@ export function print(val: unknown) {
   /**
    * form parts of regular expression and removes collected classNames from string
    * @example
-   * regex = /r?(f16th3vw|frdkuqy0|fat0sn40|fjseox00)/
+   * regex = /f16th3vw|frdkuqy0|fat0sn40|fjseox00/
    */
   const valStrippedClassNames = _val.replace(new RegExp(regexParts.join('|'), 'g'), '').trim();
 
@@ -61,7 +67,7 @@ export function test(val: unknown) {
  * lookupRegex() // /(__1qdh4ig)/g
  */
 function lookupRegex(): RegExp | undefined {
-  const definitionKeys = Object.keys(DEFINITION_LOOKUP_TABLE);
+  const definitionKeys = Object.keys({ ...DEFINITION_LOOKUP_TABLE, ...DEBUG_RESET_CLASSES });
 
   if (definitionKeys.length) {
     return new RegExp(`${definitionKeys.join('|')}`, 'g');
diff --git a/packages/react/src/__resetCSS.ts b/packages/react/src/__resetCSS.ts
index 0614b14..4c33d8c 100644
--- a/packages/react/src/__resetCSS.ts
+++ b/packages/react/src/__resetCSS.ts
@@ -7,7 +7,7 @@ import { useTextDirection } from './TextDirectionContext';
  * @internal
  */
 // eslint-disable-next-line @typescript-eslint/naming-convention
-export function __resetStyles(ltrClassName: string, rtlClassName: string | null) {
+export function __resetCSS(ltrClassName: string, rtlClassName: string | null) {
   const getStyles = vanillaResetCSS(ltrClassName, rtlClassName);
 
   return function useClasses(): string {
diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts
index d3a73c1..1ff8e01 100644
--- a/packages/react/src/index.ts
+++ b/packages/react/src/index.ts
@@ -12,4 +12,5 @@ export { TextDirectionProvider } from './TextDirectionContext';
 // Private exports, are used by build time transforms
 export { __css } from './__css';
 export { __styles } from './__styles';
+export { __resetCSS } from './__resetCSS';
 export { __resetStyles } from './__resetStyles';


Test patch:
diff --git a/packages/core/src/mergeClasses.test.ts b/packages/core/src/mergeClasses.test.ts
index 47d5aff..77e44df 100644
--- a/packages/core/src/mergeClasses.test.ts
+++ b/packages/core/src/mergeClasses.test.ts
@@ -3,6 +3,7 @@ import { makeStyles } from './makeStyles';
 import { createDOMRenderer } from './renderer/createDOMRenderer';
 import { MakeStylesOptions } from './types';
 import { SEQUENCE_PREFIX, SEQUENCE_SIZE } from './constants';
+import { makeResetStyles } from './makeResetStyles';
 
 function removeSequenceHash(classNames: string) {
   const indexOfSequence = classNames.indexOf(SEQUENCE_PREFIX);
@@ -149,6 +150,19 @@ describe('mergeClasses', () => {
     expect(error).toHaveBeenCalledWith(expect.stringMatching(/that has different direction \(dir="rtl"\)/));
   });
 
+  it('warns if contains multiple classes from makeResetStyles', () => {
+    // eslint-disable-next-line @typescript-eslint/no-empty-function
+    const error = jest.spyOn(console, 'error').mockImplementationOnce(() => {});
+
+    const className1 = makeResetStyles({ display: 'block' })(options);
+    const className2 = makeResetStyles({ display: 'grid' })(options);
+
+    expect(mergeClasses(className1, className2)).toMatchInlineSnapshot(`r13o7eu2 rlgj0h8`);
+    expect(error).toHaveBeenCalledWith(
+      expect.stringMatching(/a passed string contains multiple classes produced by makeResetStyles/),
+    );
+  });
+
   describe('"dir" option', () => {
     it('performs deduplication for RTL classes', () => {
       const computeClasses = makeStyles({
diff --git a/packages/jest-serializer/src/index.test.tsx b/packages/jest-serializer/src/index.test.tsx
index 20040eb..4d0d0ec 100644
--- a/packages/jest-serializer/src/index.test.tsx
+++ b/packages/jest-serializer/src/index.test.tsx
@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { makeStyles, mergeClasses, TextDirectionProvider } from '@griffel/react';
+import { makeResetStyles, makeStyles, mergeClasses, TextDirectionProvider } from '@griffel/react';
 import { render } from '@testing-library/react';
 
 import { print, test } from './index';
@@ -19,6 +19,11 @@ const useStyles3 = makeStyles({
   display: { display: 'none' },
 });
 
+const useBaseStyles = makeResetStyles({
+  color: 'red',
+  marginLeft: '20px',
+});
+
 const TestComponent: React.FC<{ id?: string }> = ({ id }) => {
   const styles1 = useStyles1();
   const styles2 = useStyles2();
@@ -28,6 +33,15 @@ const TestComponent: React.FC<{ id?: string }> = ({ id }) => {
   return <div data-testid={id} className={styles} />;
 };
 
+const TestResetComponent: React.FC<{ id?: string }> = ({ id }) => {
+  const classes = useStyles1();
+  const baseClassName = useBaseStyles();
+
+  const className = mergeClasses('static-class', classes.paddingLeft, baseClassName);
+
+  return <div data-testid={id} className={className} />;
+};
+
 const RtlWrapper: React.FC = ({ children }) => <TextDirectionProvider dir="rtl">{children}</TextDirectionProvider>;
 
 describe('serializer', () => {
@@ -37,11 +51,24 @@ describe('serializer', () => {
       paddingLeft: '10px',
       paddingRight: '20px',
     });
+    expect(render(<TestResetComponent id="reset-test" />).getByTestId('reset-test')).toHaveStyle({
+      color: 'red',
+      paddingLeft: '10px',
+      marginLeft: '20px',
+    });
+
     expect(render(<TestComponent id="rtl-test" />, { wrapper: RtlWrapper }).getByTestId('rtl-test')).toHaveStyle({
       display: 'none',
       paddingLeft: '20px',
       paddingRight: '10px',
     });
+    expect(
+      render(<TestResetComponent id="rtl-reset-test" />, { wrapper: RtlWrapper }).getByTestId('rtl-reset-test'),
+    ).toHaveStyle({
+      color: 'red',
+      paddingRight: '10px',
+      marginRight: '20px',
+    });
   });
 
   it('renders without generated classes', () => {
@@ -50,10 +77,21 @@ describe('serializer', () => {
         class="static-class"
       />
     `);
+    expect(render(<TestResetComponent />).container.firstChild).toMatchInlineSnapshot(`
+      <div
+        class="static-class"
+      />
+    `);
+
     expect(render(<TestComponent />, { wrapper: RtlWrapper }).container.firstChild).toMatchInlineSnapshot(`
       <div
         class="static-class"
       />
     `);
+    expect(render(<TestResetComponent />, { wrapper: RtlWrapper }).container.firstChild).toMatchInlineSnapshot(`
+      <div
+        class="static-class"
+      />
+    `);
   });
 });