Skip to content

Allow nonce to be used on hoistable styles #32461

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.

Sign up for GitHub

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

Merged
gnoff merged 20 commits into from
May 29, 2025
+398 −30
Merged

Allow nonce to be used on hoistable styles #32461

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
cf5d7dd
Allow `nonce` to be used on hoistable styles
Andarist Feb 23, 2025
9c044b7
Merge remote-tracking branch 'origin/main' into fix/style-precendence…
Andarist Apr 5, 2025
947e2f1
remove `@gate`
Andarist Apr 6, 2025
57b4c9e
tweak logic
Andarist Apr 25, 2025
cbf18c5
tweak logic
Andarist Apr 25, 2025
574374a
Merge remote-tracking branch 'origin/main' into fix/style-precendence…
Andarist Apr 25, 2025
7906af5
fmt
Andarist Apr 25, 2025
3c152a9
fix type
Andarist Apr 25, 2025
9967076
fix types
Andarist Apr 26, 2025
a3259a0
Merge remote-tracking branch 'origin/main' into fix/style-precendence…
Andarist May 25, 2025
6936609
Refactor
Andarist May 26, 2025
a89a50d
fixed types
Andarist May 27, 2025
63e0282
propagate nonce type through resume options etc
Andarist May 27, 2025
44d19b1
fmt
Andarist May 27, 2025
0c0eaf8
render styles without nonce when render call didn't receive nonce
Andarist May 27, 2025
3a6a817
add tests
Andarist May 27, 2025
7ede4ed
loosen up expect
Andarist May 27, 2025
aaebd2f
tweak console.error messaging
Andarist May 29, 2025
68d592a
get rid of `styleQueue.render`
Andarist May 29, 2025
acc9f05
fmt
Andarist May 29, 2025
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
96 changes: 76 additions & 20 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,13 @@ const SentMarkShellTime /* */ = 0b001000000;
const NeedUpgradeToViewTransitions /* */ = 0b010000000;
const SentUpgradeToViewTransitions /* */ = 0b100000000;

type NonceOption =
| string
| {
script?: string,
style?: string,
};

// Per request, global state that is not contextual to the rendering subtree.
// This cannot be resumed and therefore should only contain things that are
// temporary working state or are never used in the prerender pass.
Expand All @@ -147,6 +154,8 @@ export type RenderState = {
// inline script streaming format, unused if using external runtime / data
startInlineScript: PrecomputedChunk,

startInlineStyle: PrecomputedChunk,

// the preamble must always flush before resuming, so all these chunks must
// be null or empty when resuming.

Expand Down Expand Up @@ -209,6 +218,11 @@ export type RenderState = {
moduleScripts: Map<string, Resource>,
},

nonce: {
script: string | void,
style: string | void,
},

// Module-global-like reference for flushing/hoisting state of style resources
// We need to track whether the current request has flushed any style resources
// without sending an instruction to hoist them. we do that here
Expand Down Expand Up @@ -307,6 +321,8 @@ const scriptIntegirty = stringToPrecomputedChunk(' integrity="');
const scriptCrossOrigin = stringToPrecomputedChunk(' crossorigin="');
const endAsyncScript = stringToPrecomputedChunk(' async=""></script>');

const startInlineStyle = stringToPrecomputedChunk('<style');

/**
* This escaping function is designed to work with with inline scripts where the entire
* contents are escaped. Because we know we are escaping the entire script we can avoid for instance
Expand Down Expand Up @@ -365,17 +381,32 @@ if (__DEV__) {
// is set, the server will send instructions via data attributes (instead of inline scripts)
export function createRenderState(
resumableState: ResumableState,
nonce: string | void,
nonce:
| string
| {
script?: string,
style?: string,
}
| void,
externalRuntimeConfig: string | BootstrapScriptDescriptor | void,
importMap: ImportMap | void,
onHeaders: void | ((headers: HeadersDescriptor) => void),
maxHeadersLength: void | number,
): RenderState {
const nonceScript = typeof nonce === 'string' ? nonce : nonce && nonce.script;
const inlineScriptWithNonce =
nonce === undefined
nonceScript === undefined
? startInlineScript
: stringToPrecomputedChunk(
'<script nonce="' + escapeTextForBrowser(nonce) + '"',
'<script nonce="' + escapeTextForBrowser(nonceScript) + '"',
);
const nonceStyle =
typeof nonce === 'string' ? undefined : nonce && nonce.style;
const inlineStyleWithNonce =
nonceStyle === undefined
? startInlineStyle
: stringToPrecomputedChunk(
'<style nonce="' + escapeTextForBrowser(nonceStyle) + '"',
);
const idPrefix = resumableState.idPrefix;

Expand Down Expand Up @@ -403,7 +434,7 @@ export function createRenderState(
src: externalRuntimeConfig,
async: true,
integrity: undefined,
nonce: nonce,
nonce: nonceScript,
});
} else {
externalRuntimeScript = {
Expand All @@ -414,7 +445,7 @@ export function createRenderState(
src: externalRuntimeConfig.src,
async: true,
integrity: externalRuntimeConfig.integrity,
nonce: nonce,
nonce: nonceScript,
});
}
}
Expand Down Expand Up @@ -459,6 +490,7 @@ export function createRenderState(
segmentPrefix: stringToPrecomputedChunk(idPrefix + 'S:'),
boundaryPrefix: stringToPrecomputedChunk(idPrefix + 'B:'),
startInlineScript: inlineScriptWithNonce,
startInlineStyle: inlineStyleWithNonce,
preamble: createPreambleState(),

externalRuntimeScript: externalRuntimeScript,
Expand Down Expand Up @@ -500,7 +532,10 @@ export function createRenderState(
moduleScripts: new Map(),
},

nonce,
nonce: {
script: nonceScript,
style: nonceStyle,
},
// like a module global for currently rendering boundary
hoistableState: null,
stylesToHoist: false,
Expand Down Expand Up @@ -539,10 +574,10 @@ export function createRenderState(
stringToChunk(escapeTextForBrowser(src)),
attributeEnd,
);
if (nonce) {
if (nonceScript) {
bootstrapChunks.push(
scriptNonce,
stringToChunk(escapeTextForBrowser(nonce)),
stringToChunk(escapeTextForBrowser(nonceScript)),
attributeEnd,
);
}
Expand Down Expand Up @@ -571,7 +606,7 @@ export function createRenderState(
const props: PreloadModuleProps = ({
rel: 'modulepreload',
fetchPriority: 'low',
nonce,
nonce: nonceScript,
}: any);
if (typeof scriptConfig === 'string') {
props.href = src = scriptConfig;
Expand All @@ -596,10 +631,10 @@ export function createRenderState(
stringToChunk(escapeTextForBrowser(src)),
attributeEnd,
);
if (nonce) {
if (nonceScript) {
bootstrapChunks.push(
scriptNonce,
stringToChunk(escapeTextForBrowser(nonce)),
stringToChunk(escapeTextForBrowser(nonceScript)),
attributeEnd,
);
}
Expand Down Expand Up @@ -627,7 +662,7 @@ export function createRenderState(

export function resumeRenderState(
resumableState: ResumableState,
nonce: string | void,
nonce: NonceOption | void,
): RenderState {
return createRenderState(
resumableState,
Expand Down Expand Up @@ -2882,6 +2917,7 @@ function pushLink(
// to create a StyleQueue
if (!styleQueue) {
styleQueue = {
start: renderState.startInlineStyle,
precedence: stringToChunk(escapeTextForBrowser(precedence)),
rules: ([]: Array<Chunk | PrecomputedChunk>),
hrefs: ([]: Array<Chunk | PrecomputedChunk>),
Expand Down Expand Up @@ -3046,6 +3082,7 @@ function pushStyle(
}
const precedence = props.precedence;
const href = props.href;
const nonce = props.nonce;

if (
formatContext.insertionMode === SVG_MODE ||
Expand Down Expand Up @@ -3089,17 +3126,34 @@ function pushStyle(
// This is the first time we've encountered this precedence we need
// to create a StyleQueue.
styleQueue = {
start: renderState.startInlineStyle,
precedence: stringToChunk(escapeTextForBrowser(precedence)),
rules: ([]: Array<Chunk | PrecomputedChunk>),
hrefs: [stringToChunk(escapeTextForBrowser(href))],
hrefs: ([]: Array<Chunk | PrecomputedChunk>),
sheets: (new Map(): Map<string, StylesheetResource>),
};
renderState.styles.set(precedence, styleQueue);
} else {
// We have seen this precedence before and need to track this href
}

const nonceStyle = renderState.nonce.style;
if (!nonceStyle || nonceStyle === nonce) {
if (__DEV__) {
if (!nonceStyle && nonce) {
console.error(
'React encountered a hoistable style tag with "%s" nonce. The nonce was not passed to the render call though.',
nonce,
);
}
}
styleQueue.hrefs.push(stringToChunk(escapeTextForBrowser(href)));
pushStyleContents(styleQueue.rules, props);
} else if (__DEV__) {
console.error(
'React encountered a hoistable style tag with "%s" nonce. It doesn\'t match the "%s" nonce passed to the render call. They have to be the same.',
nonce,
nonceStyle,
);
}
pushStyleContents(styleQueue.rules, props);
}
if (styleQueue) {
// We need to track whether this boundary should wait on this resource or not.
Expand Down Expand Up @@ -5148,7 +5202,7 @@ function escapeJSObjectForInstructionScripts(input: Object): string {
}

const lateStyleTagResourceOpen1 = stringToPrecomputedChunk(
'<style media="not all" data-precedence="',
' media="not all" data-precedence="',
);
const lateStyleTagResourceOpen2 = stringToPrecomputedChunk('" data-href="');
const lateStyleTagResourceOpen3 = stringToPrecomputedChunk('">');
Expand Down Expand Up @@ -5176,6 +5230,7 @@ function flushStyleTagsLateForBoundary(
}
let i = 0;
if (hrefs.length) {
writeChunk(this, styleQueue.start);
Copy link
Collaborator

@gnoff gnoff May 28, 2025

Choose a reason for hiding this comment

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

any reason we don't just use renderState.startInlineStyle? doesn't seem necessary to have this on every style queue when it must by necessity be the same for each queue

Copy link
Contributor Author

@Andarist Andarist May 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

renderState isn't available here. So I'd have to introduce extra inline functions at those 2 call sites to pass the extra argument:

// ...
hoistableState.styles.forEach(flushStyleTagsLateForBoundary, destination);

// ...
renderState.styles.forEach(flushStylesInPreamble, destination);

I could use a module-level variable instead though. I'll push this change in a second.

Copy link
Contributor Author

@Andarist Andarist May 29, 2025

Choose a reason for hiding this comment

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

done in 68d592a but I imagine you might want to have it done slightly differently - if that's the case I'd appreciate concrete tips as to the preferred solution that would match codebase's style and preferences

Copy link

@ajonathan007 ajonathan007 Oct 9, 2025

Choose a reason for hiding this comment

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

W

Copy link

@ajonathan007 ajonathan007 Oct 9, 2025

Choose a reason for hiding this comment

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

workonly

writeChunk(this, lateStyleTagResourceOpen1);
writeChunk(this, styleQueue.precedence);
writeChunk(this, lateStyleTagResourceOpen2);
Expand Down Expand Up @@ -5268,9 +5323,7 @@ function flushStyleInPreamble(
stylesheet.state = PREAMBLE;
}

const styleTagResourceOpen1 = stringToPrecomputedChunk(
'<style data-precedence="',
);
const styleTagResourceOpen1 = stringToPrecomputedChunk(' data-precedence="');
const styleTagResourceOpen2 = stringToPrecomputedChunk('" data-href="');
const spaceSeparator = stringToPrecomputedChunk(' ');
const styleTagResourceOpen3 = stringToPrecomputedChunk('">');
Expand All @@ -5292,6 +5345,7 @@ function flushStylesInPreamble(
// order so even if there are no rules for style tags at this precedence we emit an empty style
// tag with the data-precedence attribute
if (!hasStylesheets || hrefs.length) {
writeChunk(this, styleQueue.start);
writeChunk(this, styleTagResourceOpen1);
writeChunk(this, styleQueue.precedence);
let i = 0;
Expand Down Expand Up @@ -6057,6 +6111,7 @@ export type HoistableState = {
};

export type StyleQueue = {
start: PrecomputedChunk,
precedence: Chunk | PrecomputedChunk,
rules: Array<Chunk | PrecomputedChunk>,
hrefs: Array<Chunk | PrecomputedChunk>,
Expand Down Expand Up @@ -6508,6 +6563,7 @@ function preinitStyle(
// to create a StyleQueue
if (!styleQueue) {
styleQueue = {
start: renderState.startInlineStyle,
precedence: stringToChunk(escapeTextForBrowser(precedence)),
rules: ([]: Array<Chunk | PrecomputedChunk>),
hrefs: ([]: Array<Chunk | PrecomputedChunk>),
Expand Down
7 changes: 7 additions & 0 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export type RenderState = {
segmentPrefix: PrecomputedChunk,
boundaryPrefix: PrecomputedChunk,
startInlineScript: PrecomputedChunk,
startInlineStyle: PrecomputedChunk,
preamble: PreambleState,
externalRuntimeScript: null | any,
bootstrapChunks: Array<Chunk | PrecomputedChunk>,
Expand Down Expand Up @@ -76,6 +77,10 @@ export type RenderState = {
scripts: Map<string, Resource>,
moduleScripts: Map<string, Resource>,
},
nonce: {
script: string | void,
style: string | void,
},
stylesToHoist: boolean,
// This is an extra field for the legacy renderer
generateStaticMarkup: boolean,
Expand All @@ -99,6 +104,7 @@ export function createRenderState(
segmentPrefix: renderState.segmentPrefix,
boundaryPrefix: renderState.boundaryPrefix,
startInlineScript: renderState.startInlineScript,
startInlineStyle: renderState.startInlineStyle,
preamble: renderState.preamble,
externalRuntimeScript: renderState.externalRuntimeScript,
bootstrapChunks: renderState.bootstrapChunks,
Expand All @@ -118,6 +124,7 @@ export function createRenderState(
scripts: renderState.scripts,
bulkPreloads: renderState.bulkPreloads,
preloads: renderState.preloads,
nonce: renderState.nonce,
stylesToHoist: renderState.stylesToHoist,

// This is an extra field for the legacy renderer
Expand Down
Loading
Loading