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 0000000000..6afb179c66 --- /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 0000000000..019ab307b5 --- /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/packages/core/src/__resetCSS.ts b/packages/core/src/__resetCSS.ts index 8bec57015b..35fea303ff 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): 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 e65b846f62..cd4c86e798 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 77f71ea3c5..87f6e4af0e 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 = {}; + /** @internal */ export const DEFINITION_LOOKUP_TABLE: Record = {}; diff --git a/packages/core/src/makeResetStyles.ts b/packages/core/src/makeResetStyles.ts index 717b28f35e..0b64094356 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 = {}; @@ -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.test.ts b/packages/core/src/mergeClasses.test.ts index 47d5aff474..77e44df4c0 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/core/src/mergeClasses.ts b/packages/core/src/mergeClasses.ts index d780977750..3da1c7b7fe 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.test.tsx b/packages/jest-serializer/src/index.test.tsx index 20040ebe8d..4d0d0ec63e 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
; }; +const TestResetComponent: React.FC<{ id?: string }> = ({ id }) => { + const classes = useStyles1(); + const baseClassName = useBaseStyles(); + + const className = mergeClasses('static-class', classes.paddingLeft, baseClassName); + + return
; +}; + const RtlWrapper: React.FC = ({ children }) => {children}; describe('serializer', () => { @@ -37,11 +51,24 @@ describe('serializer', () => { paddingLeft: '10px', paddingRight: '20px', }); + expect(render().getByTestId('reset-test')).toHaveStyle({ + color: 'red', + paddingLeft: '10px', + marginLeft: '20px', + }); + expect(render(, { wrapper: RtlWrapper }).getByTestId('rtl-test')).toHaveStyle({ display: 'none', paddingLeft: '20px', paddingRight: '10px', }); + expect( + render(, { 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().container.firstChild).toMatchInlineSnapshot(` +
+ `); + expect(render(, { wrapper: RtlWrapper }).container.firstChild).toMatchInlineSnapshot(`
`); + expect(render(, { wrapper: RtlWrapper }).container.firstChild).toMatchInlineSnapshot(` +
+ `); }); }); diff --git a/packages/jest-serializer/src/index.ts b/packages/jest-serializer/src/index.ts index ffb2448b8c..adf0f0bc4b 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');