-
Notifications
You must be signed in to change notification settings - Fork 77
feat(core): warn in dev when SSR styles are rehydrated more than once #996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "type": "patch", | ||
| "comment": "Warn in development when duplicate CSS rules are found during rehydration — a signal that server-rendered styles were flushed into the HTML more than once (e.g. the Next.js App Router calling renderToStyleElements on every flush without clearing the renderer)", | ||
| "packageName": "@griffel/core", | ||
| "email": "vaclav.dvorak@ysoft.com", | ||
| "dependentChangeType": "patch" | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -29,6 +29,29 @@ export function rehydrateRendererCache( | |||||||||||||||||||||||||||||||||||||||||||||
| if (target) { | ||||||||||||||||||||||||||||||||||||||||||||||
| const styleElements = target.querySelectorAll<HTMLStyleElement>('[data-make-styles-bucket]'); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| // Griffel emits each CSS rule into the document exactly once. In development we track | ||||||||||||||||||||||||||||||||||||||||||||||
| // the rules seen while rehydrating, so we can warn if the same rule appears in more than | ||||||||||||||||||||||||||||||||||||||||||||||
| // one server-rendered <style> element — a strong signal that styles were flushed into the | ||||||||||||||||||||||||||||||||||||||||||||||
| // HTML multiple times (see the warning emitted after the loop). | ||||||||||||||||||||||||||||||||||||||||||||||
| const seenRules: Set<string> | undefined = process.env.NODE_ENV !== 'production' ? new Set() : undefined; | ||||||||||||||||||||||||||||||||||||||||||||||
| let duplicateRuleCount = 0; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const cacheRule = (cssRule: string, bucketName: StyleBucketName) => { | ||||||||||||||||||||||||||||||||||||||||||||||
| if (seenRules) { | ||||||||||||||||||||||||||||||||||||||||||||||
| if (seenRules.has(cssRule)) { | ||||||||||||||||||||||||||||||||||||||||||||||
| duplicateRuleCount++; | ||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||
| seenRules.add(cssRule); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+36
to
+46
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
nit: let's write it this way. While Terser is able to DCE the original code, I would prefer to keep instructions gated in more solid way. Thx! |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| renderer.insertionCache[cssRule] = bucketName; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if (process.env.NODE_ENV !== 'production' && isDevToolsEnabled) { | ||||||||||||||||||||||||||||||||||||||||||||||
| debugData.addCSSRule(cssRule); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| styleElements.forEach(styleElement => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const bucketName = styleElement.dataset['makeStylesBucket'] as StyleBucketName; | ||||||||||||||||||||||||||||||||||||||||||||||
| const stylesheetKey = getStyleSheetKeyFromElement(styleElement); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -47,11 +70,7 @@ export function rehydrateRendererCache( | |||||||||||||||||||||||||||||||||||||||||||||
| while ((match = regex.exec(textContent))) { | ||||||||||||||||||||||||||||||||||||||||||||||
| const [cssRule] = match; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| renderer.insertionCache[cssRule] = bucketName; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if (process.env.NODE_ENV !== 'production' && isDevToolsEnabled) { | ||||||||||||||||||||||||||||||||||||||||||||||
| debugData.addCSSRule(cssRule); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| cacheRule(cssRule, bucketName); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||
| // @scope rules can appear in any bucket, so extract them first to prevent | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -60,25 +79,40 @@ export function rehydrateRendererCache( | |||||||||||||||||||||||||||||||||||||||||||||
| while ((match = SCOPE_HYDRATOR.exec(textContent))) { | ||||||||||||||||||||||||||||||||||||||||||||||
| const [cssRule] = match; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| renderer.insertionCache[cssRule] = bucketName; | ||||||||||||||||||||||||||||||||||||||||||||||
| cacheRule(cssRule, bucketName); | ||||||||||||||||||||||||||||||||||||||||||||||
| textContent = textContent.replace(cssRule, ''); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if (process.env.NODE_ENV !== 'production' && isDevToolsEnabled) { | ||||||||||||||||||||||||||||||||||||||||||||||
| debugData.addCSSRule(cssRule); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| // eslint-disable-next-line no-cond-assign | ||||||||||||||||||||||||||||||||||||||||||||||
| while ((match = STYLES_HYDRATOR.exec(textContent))) { | ||||||||||||||||||||||||||||||||||||||||||||||
| const [cssRule] = match; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| renderer.insertionCache[cssRule] = bucketName; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if (process.env.NODE_ENV !== 'production' && isDevToolsEnabled) { | ||||||||||||||||||||||||||||||||||||||||||||||
| debugData.addCSSRule(cssRule); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| cacheRule(cssRule, bucketName); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if (process.env.NODE_ENV !== 'production' && duplicateRuleCount > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||
| // eslint-disable-next-line no-console | ||||||||||||||||||||||||||||||||||||||||||||||
| console.error( | ||||||||||||||||||||||||||||||||||||||||||||||
| [ | ||||||||||||||||||||||||||||||||||||||||||||||
| '@griffel/core:', | ||||||||||||||||||||||||||||||||||||||||||||||
| `Found ${duplicateRuleCount} duplicate CSS rule(s) while rehydrating server-rendered styles.`, | ||||||||||||||||||||||||||||||||||||||||||||||
| 'The same styles were flushed into the HTML more than once.', | ||||||||||||||||||||||||||||||||||||||||||||||
| '\n\n', | ||||||||||||||||||||||||||||||||||||||||||||||
| 'In the Next.js App Router this happens when renderToStyleElements() is called on every', | ||||||||||||||||||||||||||||||||||||||||||||||
| 'useServerInsertedHTML flush without clearing the renderer in between: each flush re-emits', | ||||||||||||||||||||||||||||||||||||||||||||||
| 'the full stylesheet and the extra copies are streamed into <body>. Those stale copies can', | ||||||||||||||||||||||||||||||||||||||||||||||
| 'override styles inserted after a client-side navigation, making makeStyles() overrides lose', | ||||||||||||||||||||||||||||||||||||||||||||||
| 'to their makeResetStyles() base.', | ||||||||||||||||||||||||||||||||||||||||||||||
| '\n\n', | ||||||||||||||||||||||||||||||||||||||||||||||
| 'Clear the renderer after each flush:', | ||||||||||||||||||||||||||||||||||||||||||||||
| '\n\n', | ||||||||||||||||||||||||||||||||||||||||||||||
| 'useServerInsertedHTML(() => {\n const styles = renderToStyleElements(renderer);\n renderer.stylesheets = {};\n return styles;\n});', | ||||||||||||||||||||||||||||||||||||||||||||||
| '\n\n', | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+109
to
+112
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Let's remove the actual recommendation from code. |
||||||||||||||||||||||||||||||||||||||||||||||
| 'See https://griffel.js.org/react/guides/ssr-usage for details.', | ||||||||||||||||||||||||||||||||||||||||||||||
| ].join(' '), | ||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.