From 1533612389814e437cc6c2513e290cf5ca4a40b5 Mon Sep 17 00:00:00 2001 From: "manuel.carrera" Date: Mon, 26 Aug 2024 12:19:23 -0600 Subject: [PATCH 1/3] fix: Enable styling compat mode to ensure proper style merging --- modules/styling/lib/cs.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/modules/styling/lib/cs.ts b/modules/styling/lib/cs.ts index 350cdf3f46..ff80877b7b 100644 --- a/modules/styling/lib/cs.ts +++ b/modules/styling/lib/cs.ts @@ -694,8 +694,13 @@ export function handleCsProp< const {cs, style, className, ...restProps} = elemProps; const instance = getInstance(); - // We are going to track if any runtime styles are detected - let shouldRuntimeMergeStyles = false; + // We're seeing style merging issues when using createStyles or createStencil. It only happens when + // every style override of the element uses these utilities and @emotion/react or @emotion/styled is not used on the same element. + // These utilities rely on module execution order and we're having a few reports where modules are possibly executing out of order. + // In order to allow everyone to use createStyles and createStencil without worrying about style merge issues, we're going + // to enable compat mode all the time. We'll look into possible out-of-order execution issues in the future and plan to re-enable + // full static mode (for better performance) once we know why this is happening and have a proper workaround. + let shouldRuntimeMergeStyles = true; // The order is intentional. The `className` should go first, then the `cs` prop. If we don't do // runtime merging, this order doesn't actually matter because the browser doesn't care the order @@ -891,7 +896,7 @@ function combineClassNames(input: (string | undefined)[]): string { .filter((s, i, a) => onlyDefined(s) && onlyUnique(s, i, a)) .join(' '); } - + /** * Creates a reuseable Stencil for styling elements. It takes vars, base styles, modifiers, and * compound modifiers. From 1efa2a0c25d1187cebf3a8b00a587d71c30d8787 Mon Sep 17 00:00:00 2001 From: "manuel.carrera" Date: Mon, 26 Aug 2024 15:24:35 -0600 Subject: [PATCH 2/3] test: Skip test verifying class --- modules/styling/spec/cs.spec.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/styling/spec/cs.spec.tsx b/modules/styling/spec/cs.spec.tsx index 5f5cab85fa..e7d4f419fb 100644 --- a/modules/styling/spec/cs.spec.tsx +++ b/modules/styling/spec/cs.spec.tsx @@ -862,7 +862,9 @@ describe('handleCsProp', () => { expect(screen.getByTestId('base')).toHaveStyle({padding: padding.styleAttribute}); }); - it('should allow the cs prop to override base styles', () => { + // While we have compat mode enabled, we'll skip these tests. The class generated comes from emotion and + //we have no way of validating the correct class. + it.skip('should allow the cs prop to override base styles', () => { const overrideStyles = createStyles({ padding: padding.createStyles, }); From 1f68df8cb8c94ccc5b1008175ae6dd23e6d920db Mon Sep 17 00:00:00 2001 From: "manuel.carrera" Date: Tue, 27 Aug 2024 09:13:30 -0600 Subject: [PATCH 3/3] test: Skip test while in compat mode --- modules/react/layout/spec/mergeStyles.spec.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/react/layout/spec/mergeStyles.spec.tsx b/modules/react/layout/spec/mergeStyles.spec.tsx index 7cadbf4da7..d24392ebbe 100644 --- a/modules/react/layout/spec/mergeStyles.spec.tsx +++ b/modules/react/layout/spec/mergeStyles.spec.tsx @@ -41,8 +41,8 @@ describe('mergeStyles', () => { expect(screen.getByTestId('base')).toHaveStyle({padding: padding.styleAttribute}); }); - - it('should allow the cs prop to override base styles', () => { + // Skipping this for now while we enable compat mode to run all the time + it.skip('should allow the cs prop to override base styles', () => { const overrideStyles = createStyles({ padding: padding.createStyles, });