Skip to content

Commit 49d09ae

Browse files
claudequantizor
claude
authored and
quantizor
committed
fix: avoid double rebuildGroup on every dynamic createGlobalStyle render (#5730)
In 6.4.0, dynamic createGlobalStyle components paid the cost of removeStyles → rebuildGroup twice per render: once from the render effect's cleanup callback, once from the new render. Because rebuildGroup clears and reinserts every mounted instance's CSS via CSSOM, this dominated CPU on apps with frequent parent re-renders. Split the single useLayoutEffect into two: one keyed on render deps that calls renderStyles (and lets the inline rulesEqual fast-path skip CSSOM work when CSS is unchanged), and a cleanup-only effect keyed on [instance, sheet, globalStyle] whose teardown fires on unmount, sheet swap, or HMR globalStyle swap -- not on every prop change. Side effect: multiple mounted instances of the same createGlobalStyle now emit CSS in mount order, restoring pre-6.4 cascade behavior.
1 parent e975035 commit 49d09ae

3 files changed

Lines changed: 127 additions & 11 deletions

File tree

‎.changeset/fix-global-style-rebuild-thrash.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'styled-components': patch
3+
---
4+
5+
Fix a performance regression in 6.4.0 where dynamic `createGlobalStyle` components caused significant re-render slowdowns. Also restores pre-6.4 cascade ordering when multiple instances of the same `createGlobalStyle` coexist.

‎packages/styled-components/src/constructors/createGlobalStyle.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,21 @@ export default function createGlobalStyle<Props extends object>(
9494
// Client-side lifecycle: render styles in effect and clean up on unmount.
9595
// __SERVER__ and IS_RSC are build/module-level constants, so this doesn't violate rules of hooks.
9696
if (!__SERVER__ && !IS_RSC) {
97-
// globalStyle is included in deps so HMR-induced module re-evaluation
97+
// Split into two effects so cleanup (removeStyles → full rebuildGroup) only
98+
// fires on actual unmount or sheet/globalStyle swap -- NOT on every prop change.
99+
//
100+
// For dynamic globals, `props` is a new reference every render, so the render
101+
// effect re-runs each render. If cleanup ran on every re-run, each render would
102+
// do two full rebuildGroups (delete + reinsert all instances), which dominates
103+
// CPU on apps with frequent parent re-renders (issue #5730). Splitting lets
104+
// renderStyles' rulesEqual fast-path skip rebuildGroup when CSS is unchanged.
105+
//
106+
// globalStyle is included in render deps so HMR-induced module re-evaluation
98107
// (which creates a new GlobalStyle instance) triggers effect re-run.
99108
// For static rules, renderStyles exits early after the first injection
100109
// (via hasNameForId check), so the extra dep is effectively free at runtime.
101110
// eslint-disable-next-line react-hooks/exhaustive-deps
102-
const effectDeps = globalStyle.isStatic
111+
const renderDeps = globalStyle.isStatic
103112
? [instance, ssc.styleSheet, globalStyle]
104113
: [instance, props, ssc.styleSheet, theme, ssc.stylis, globalStyle];
105114

@@ -116,11 +125,18 @@ export default function createGlobalStyle<Props extends object>(
116125

117126
renderStyles(instance, props, ssc.styleSheet, theme, ssc.stylis);
118127
}
128+
}, renderDeps);
119129

130+
// Cleanup-only effect: fires on unmount, sheet swap, or HMR globalStyle swap.
131+
// Closure captures the specific globalStyle/sheet that owned this instance's
132+
// rules so HMR cleanup targets the prior module's state.
133+
React.useLayoutEffect(() => {
120134
return () => {
121-
globalStyle.removeStyles(instance, ssc.styleSheet);
135+
if (!ssc.styleSheet.server) {
136+
globalStyle.removeStyles(instance, ssc.styleSheet);
137+
}
122138
};
123-
}, effectDeps);
139+
}, [instance, ssc.styleSheet, globalStyle]);
124140
}
125141

126142
// RSC mode: output style tag.

‎packages/styled-components/src/constructors/test/createGlobalStyle.test.tsx‎

Lines changed: 102 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -645,14 +645,16 @@ describe(`createGlobalStyle`, () => {
645645
}"
646646
`);
647647

648-
// Change only instance A's prop — instance B should be unaffected
648+
// Change only instance A's prop — instance B's CSS is unchanged, and
649+
// rule order must remain mount-order (A then B) so cascade doesn't flip
650+
// based on which instance updated last.
649651
rerender(<Wrapper colorA="green" colorB="blue" />);
650652
expect(getRenderedCSS()).toMatchInlineSnapshot(`
651653
"body {
652-
background: blue;
654+
background: green;
653655
}
654656
body {
655-
background: green;
657+
background: blue;
656658
}"
657659
`);
658660
});
@@ -957,10 +959,9 @@ describe('createGlobalStyle HMR', () => {
957959
describe('GlobalStyle.renderStyles (unit)', () => {
958960
/**
959961
* These tests exercise GlobalStyle.renderStyles() directly to cover
960-
* the rulesEqual fast-path (lines 57-68 in GlobalStyle.ts) which is
961-
* unreachable through the React component lifecycle because
962-
* useLayoutEffect cleanup always deletes instanceRules before the
963-
* next effect re-runs renderStyles.
962+
* the rulesEqual fast-path on the pure model. The React lifecycle
963+
* path also reaches this fast-path now -- see the lifecycle tests
964+
* below (`useLayoutEffect rebuildGroup`).
964965
*/
965966

966967
let sheet: StyleSheet;
@@ -1164,3 +1165,97 @@ describe('GlobalStyle.renderStyles (unit)', () => {
11641165
expect(sheet.toString()).toBe('');
11651166
});
11661167
});
1168+
1169+
describe('createGlobalStyle useLayoutEffect rebuildGroup (#5730)', () => {
1170+
beforeEach(() => {
1171+
resetStyled();
1172+
});
1173+
1174+
it('skips rebuildGroup when parent re-renders with unchanged global style props', () => {
1175+
const GlobalStyle = createGlobalStyle<{ bg: string }>`
1176+
body { background: ${props => props.bg}; }
1177+
`;
1178+
1179+
function Parent({ bg, counter }: { bg: string; counter: number }) {
1180+
// `counter` forces Parent to re-render without changing `bg`.
1181+
return (
1182+
<>
1183+
<span data-testid="counter">{counter}</span>
1184+
<GlobalStyle bg={bg} />
1185+
</>
1186+
);
1187+
}
1188+
1189+
const { rerender } = render(<Parent bg="red" counter={0} />);
1190+
expect(getRenderedCSS()).toMatchInlineSnapshot(`
1191+
"body {
1192+
background: red;
1193+
}"
1194+
`);
1195+
1196+
// Spy on the shared-group rebuild cost. With the fix, unchanged CSS must
1197+
// NOT clearRules on each unrelated parent re-render.
1198+
const clearRulesSpy = jest.spyOn(StyleSheet.prototype, 'clearRules');
1199+
1200+
for (let i = 1; i <= 5; i++) {
1201+
rerender(<Parent bg="red" counter={i} />);
1202+
}
1203+
1204+
expect(clearRulesSpy).not.toHaveBeenCalled();
1205+
expect(getRenderedCSS()).toMatchInlineSnapshot(`
1206+
"body {
1207+
background: red;
1208+
}"
1209+
`);
1210+
1211+
clearRulesSpy.mockRestore();
1212+
});
1213+
1214+
it('still rebuilds when the dynamic global style CSS actually changes', () => {
1215+
const GlobalStyle = createGlobalStyle<{ bg: string }>`
1216+
body { background: ${props => props.bg}; }
1217+
`;
1218+
1219+
const { rerender } = render(<GlobalStyle bg="red" />);
1220+
expect(getRenderedCSS()).toMatchInlineSnapshot(`
1221+
"body {
1222+
background: red;
1223+
}"
1224+
`);
1225+
1226+
rerender(<GlobalStyle bg="blue" />);
1227+
expect(getRenderedCSS()).toMatchInlineSnapshot(`
1228+
"body {
1229+
background: blue;
1230+
}"
1231+
`);
1232+
});
1233+
1234+
it('still cleans up on unmount after many re-renders', async () => {
1235+
const GlobalStyle = createGlobalStyle<{ bg: string }>`
1236+
[data-5730-cleanup] { background: ${props => props.bg}; }
1237+
`;
1238+
1239+
function App({ show, counter }: { show: boolean; counter: number }) {
1240+
return show ? (
1241+
<>
1242+
<span>{counter}</span>
1243+
<GlobalStyle bg="red" />
1244+
</>
1245+
) : null;
1246+
}
1247+
1248+
const { rerender } = render(<App show counter={0} />);
1249+
1250+
for (let i = 1; i <= 5; i++) {
1251+
rerender(<App show counter={i} />);
1252+
}
1253+
1254+
expect(getRenderedCSS()).toContain('[data-5730-cleanup]');
1255+
1256+
rerender(<App show={false} counter={99} />);
1257+
await Promise.resolve();
1258+
1259+
expect(getRenderedCSS()).not.toContain('[data-5730-cleanup]');
1260+
});
1261+
});

0 commit comments

Comments
 (0)