Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"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)",
"comment": "chore: add development warnings for hydration issues in `rehydrateRendererCache`",

"packageName": "@griffel/core",
"email": "vaclav.dvorak@ysoft.com",
"dependentChangeType": "patch"
}
50 changes: 49 additions & 1 deletion packages/core/src/renderer/rehydrateRendererCache.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { DATA_BUCKET_ATTR, DATA_PRIORITY_ATTR } from '../constants.js';
import type { GriffelRenderer } from '../types.js';
import { createDOMRenderer } from './createDOMRenderer.js';
Expand Down Expand Up @@ -105,4 +105,52 @@ describe('rehydrateRendererCache', () => {
}
`);
});

it('warns in development when the same rule is rehydrated from multiple <style> elements', () => {
process.env.NODE_ENV = 'development';
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined);

// The same rule emitted into two server-rendered <style> elements is the signature of
// styles being flushed into the HTML more than once — e.g. a Next.js App Router setup that
// calls renderToStyleElements() on every flush without clearing the renderer in between.
for (let i = 0; i < 2; i++) {
const styleElement = document.createElement('style');
styleElement.setAttribute(DATA_BUCKET_ATTR, 'd');
styleElement.setAttribute(DATA_PRIORITY_ATTR, '0');
styleElement.textContent = '.foo { color: red; }';
document.head.appendChild(styleElement);
}

rehydrateRendererCache(renderer, document);

expect(consoleError).toHaveBeenCalledTimes(1);
expect(consoleError.mock.calls[0][0]).toContain('duplicate CSS rule');

consoleError.mockRestore();
});

it('does not warn when buckets repeat with different rules (correct streamed delta)', () => {
process.env.NODE_ENV = 'development';
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined);

// Same bucket + priority across two elements, but different rules — this is what correct
// per-flush delta flushing looks like, and it must NOT warn.
const first = document.createElement('style');
first.setAttribute(DATA_BUCKET_ATTR, 'd');
first.setAttribute(DATA_PRIORITY_ATTR, '0');
first.textContent = '.foo { color: red; }';
document.head.appendChild(first);

const second = document.createElement('style');
second.setAttribute(DATA_BUCKET_ATTR, 'd');
second.setAttribute(DATA_PRIORITY_ATTR, '0');
second.textContent = '.bar { color: blue; }';
document.head.appendChild(second);

rehydrateRendererCache(renderer, document);

expect(consoleError).not.toHaveBeenCalled();

consoleError.mockRestore();
});
});
64 changes: 49 additions & 15 deletions packages/core/src/renderer/rehydrateRendererCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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);
}
}
const seenRules: Set<string> | undefined = new Set();
let duplicateRuleCount = 0;
const cacheRule = (cssRule: string, bucketName: StyleBucketName) => {
if (process.env.NODE_ENV !== 'production') {
if (seenRules.has(cssRule)) {
duplicateRuleCount++;
} else {
seenRules.add(cssRule);
}
}

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);
Expand All @@ -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
Expand All @@ -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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'Clear the renderer after each flush:',
'\n\n',
'useServerInsertedHTML(() => {\n const styles = renderToStyleElements(renderer);\n renderer.stylesheets = {};\n return styles;\n});',
'\n\n',

Let's remove the actual recommendation from code.

'See https://griffel.js.org/react/guides/ssr-usage for details.',
].join(' '),
);
}
}
}