From aabeeab023c0f33c15e0088cd7c97a8557fddf8a Mon Sep 17 00:00:00 2001 From: Josh Story Date: Mon, 21 Aug 2023 12:17:11 -0700 Subject: [PATCH] Import maps need to be emitted before any scripts or preloads so the browser can properly locate these resources. This change makes React aware of the concept of import maps and emits them before scripts and modules and their preloads. --- .../src/client/ReactFiberConfigDOM.js | 51 +++++++++++++------ .../src/server/ReactFizzConfigDOM.js | 39 ++++++++++++-- .../src/server/ReactFizzConfigDOMLegacy.js | 2 + .../src/__tests__/ReactDOMFizzServer-test.js | 47 ++++++++++++++++- .../src/__tests__/ReactDOMFizzStatic-test.js | 26 +++++++++- .../src/server/ReactDOMFizzServerBrowser.js | 8 ++- .../src/server/ReactDOMFizzServerBun.js | 7 ++- .../src/server/ReactDOMFizzServerEdge.js | 8 ++- .../src/server/ReactDOMFizzServerNode.js | 8 ++- .../src/server/ReactDOMFizzStaticBrowser.js | 7 ++- .../src/server/ReactDOMFizzStaticEdge.js | 7 ++- .../src/server/ReactDOMFizzStaticNode.js | 7 ++- .../react-dom/src/test-utils/FizzTestUtils.js | 6 +-- 13 files changed, 192 insertions(+), 31 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 9e7894cda68ba..7aaac51271f12 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -1968,7 +1968,7 @@ export function clearSingleton(instance: Instance): void { export const supportsResources = true; -type HoistableTagType = 'link' | 'meta' | 'title'; +type HoistableTagType = 'link' | 'meta' | 'title' | 'script'; type TResource< T: 'stylesheet' | 'style' | 'script' | 'void', S: null | {...}, @@ -2759,7 +2759,12 @@ export function getResource( return null; } case 'script': { - if (typeof pendingProps.src === 'string' && pendingProps.async === true) { + if (pendingProps.type === 'importmap') { + return null; + } else if ( + typeof pendingProps.src === 'string' && + pendingProps.async === true + ) { const scriptProps: ScriptProps = pendingProps; const key = getScriptKey(scriptProps.src); const scripts = getResourcesFromRoot(resourceRoot).hoistableScripts; @@ -3110,21 +3115,21 @@ export function hydrateHoistable( case 'title': { instance = ownerDocument.getElementsByTagName('title')[0]; if ( - !instance || - isOwnedInstance(instance) || - instance.namespaceURI === SVG_NAMESPACE || - instance.hasAttribute('itemprop') + instance && + !isOwnedInstance(instance) && + instance.namespaceURI !== SVG_NAMESPACE && + !instance.hasAttribute('itemprop') ) { - instance = ownerDocument.createElement(type); - (ownerDocument.head: any).insertBefore( - instance, - ownerDocument.querySelector('head > title'), - ); + setInitialProperties(instance, type, props); + break; } + instance = ownerDocument.createElement(type); setInitialProperties(instance, type, props); - precacheFiberNode(internalInstanceHandle, instance); - markNodeAsHoistable(instance); - return instance; + (ownerDocument.head: any).insertBefore( + instance, + ownerDocument.querySelector('head > title'), + ); + break; } case 'link': { const cache = getHydratableHoistableCache('link', 'href', ownerDocument); @@ -3201,6 +3206,17 @@ export function hydrateHoistable( (ownerDocument.head: any).appendChild(instance); break; } + case 'script': { + // the only hoistable script is type="importmap" and there should only be 1 + instance = ownerDocument.querySelector('script[type="importmap"]'); + if (instance && !isOwnedInstance(instance)) { + break; + } + instance = ownerDocument.createElement(type); + setInitialProperties(instance, type, props); + (ownerDocument.head: any).appendChild(instance); + break; + } default: throw new Error( `getNodesForType encountered a type it did not expect: "${type}". This is a bug in React.`, @@ -3406,7 +3422,9 @@ export function isHostHoistableType( } } case 'script': { - if ( + if (props.type === 'importmap') { + return true; + } else if ( props.async !== true || props.onLoad || props.onError || @@ -3436,8 +3454,9 @@ export function isHostHoistableType( } } return false; + } else { + return true; } - return true; } case 'noscript': case 'template': { diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 4a3c2d157dc01..04576168447f4 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -139,6 +139,7 @@ export type RenderState = { // Hoistable chunks charsetChunks: Array, preconnectChunks: Array, + importMapChunks: Array, preloadChunks: Array, hoistableChunks: Array, @@ -205,7 +206,7 @@ const scriptCrossOrigin = stringToPrecomputedChunk('" crossorigin="'); const endAsyncScript = stringToPrecomputedChunk('" async="">'); /** - * This escaping function is designed to work with bootstrapScriptContent only. + * This escaping function is designed to work with bootstrapScriptContent and importMap only. * because we know we are escaping the entire script. We can avoid for instance * escaping html comment string sequences that are valid javascript as well because * if there are no sebsequent '); * While untrusted script content should be made safe before using this api it will * ensure that the script cannot be early terminated or never terminated state */ -function escapeBootstrapScriptContent(scriptText: string) { +function escapeBootstrapAndImportMapScriptContent(scriptText: string) { if (__DEV__) { checkHtmlStringCoercion(scriptText); } @@ -237,12 +238,19 @@ export type ExternalRuntimeScript = { src: string, chunks: Array, }; + +const importMapScriptStart = stringToPrecomputedChunk( + ''); + // Allows us to keep track of what we've already written so we can refer back to it. // if passed externalRuntimeConfig and the enableFizzExternalRuntime feature flag // is set, the server will send instructions via data attributes (instead of inline scripts) export function createRenderState( resumableState: ResumableState, nonce: string | void, + importMap: {[string]: string} | void, ): RenderState { const inlineScriptWithNonce = nonce === undefined @@ -251,6 +259,17 @@ export function createRenderState( '', + ); + }); + describe('error escaping', () => { it('escapes error hash, message, and component stack values in directly flushed errors (html escaping)', async () => { window.__outlet = {}; @@ -3949,7 +3976,7 @@ describe('ReactDOMFizzServer', () => { ]); }); - describe('bootstrapScriptContent escaping', () => { + describe('bootstrapScriptContent and importMap escaping', () => { it('the "S" in " { window.__test_outlet = ''; const stringWithScriptsInIt = @@ -4005,6 +4032,24 @@ describe('ReactDOMFizzServer', () => { }); expect(window.__test_outlet).toBe(1); }); + + it('escapes in importMaps', async () => { + window.__test_outlet_key = ''; + window.__test_outlet_value = ''; + const jsonWithScriptsInIt = { + "keypos, + 'hello world', + ]); + }); }); diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js index 1ce1150423e4e..383778beff157 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js @@ -38,6 +38,7 @@ type Options = { onError?: (error: mixed) => ?string, onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, + importMap?: {[string]: string}, }; type ResumeOptions = { @@ -101,7 +102,11 @@ function renderToReadableStream( const request = createRequest( children, resumableState, - createRenderState(resumableState, options ? options.nonce : undefined), + createRenderState( + resumableState, + options ? options.nonce : undefined, + options ? options.importMap : undefined, + ), createRootFormatContext(options ? options.namespaceURI : undefined), options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, @@ -171,6 +176,7 @@ function resume( createRenderState( postponedState.resumableState, options ? options.nonce : undefined, + options ? options.importMap : undefined, ), postponedState.rootFormatContext, postponedState.progressiveChunkSize, diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBun.js b/packages/react-dom/src/server/ReactDOMFizzServerBun.js index e3800b7debc59..83dc11a2a8c30 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBun.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBun.js @@ -37,6 +37,7 @@ type Options = { onError?: (error: mixed) => ?string, onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, + importMap?: {[string]: string}, }; // TODO: Move to sub-classing ReadableStream. @@ -93,7 +94,11 @@ function renderToReadableStream( const request = createRequest( children, resumableState, - createRenderState(resumableState, options ? options.nonce : undefined), + createRenderState( + resumableState, + options ? options.nonce : undefined, + options ? options.importMap : undefined, + ), createRootFormatContext(options ? options.namespaceURI : undefined), options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, diff --git a/packages/react-dom/src/server/ReactDOMFizzServerEdge.js b/packages/react-dom/src/server/ReactDOMFizzServerEdge.js index 1ce1150423e4e..383778beff157 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerEdge.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerEdge.js @@ -38,6 +38,7 @@ type Options = { onError?: (error: mixed) => ?string, onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, + importMap?: {[string]: string}, }; type ResumeOptions = { @@ -101,7 +102,11 @@ function renderToReadableStream( const request = createRequest( children, resumableState, - createRenderState(resumableState, options ? options.nonce : undefined), + createRenderState( + resumableState, + options ? options.nonce : undefined, + options ? options.importMap : undefined, + ), createRootFormatContext(options ? options.namespaceURI : undefined), options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, @@ -171,6 +176,7 @@ function resume( createRenderState( postponedState.resumableState, options ? options.nonce : undefined, + options ? options.importMap : undefined, ), postponedState.rootFormatContext, postponedState.progressiveChunkSize, diff --git a/packages/react-dom/src/server/ReactDOMFizzServerNode.js b/packages/react-dom/src/server/ReactDOMFizzServerNode.js index ca4853a1533f7..ac1b4aef7f6ae 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerNode.js @@ -51,6 +51,7 @@ type Options = { onError?: (error: mixed) => ?string, onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, + importMap?: {[string]: string}, }; type ResumeOptions = { @@ -81,7 +82,11 @@ function createRequestImpl(children: ReactNodeList, options: void | Options) { return createRequest( children, resumableState, - createRenderState(resumableState, options ? options.nonce : undefined), + createRenderState( + resumableState, + options ? options.nonce : undefined, + options ? options.importMap : undefined, + ), createRootFormatContext(options ? options.namespaceURI : undefined), options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, @@ -140,6 +145,7 @@ function resumeRequestImpl( createRenderState( postponedState.resumableState, options ? options.nonce : undefined, + undefined, // importMap ), postponedState.rootFormatContext, postponedState.progressiveChunkSize, diff --git a/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js b/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js index de603b20ac8e9..ecd8f54077a3b 100644 --- a/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js @@ -38,6 +38,7 @@ type Options = { onError?: (error: mixed) => ?string, onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, + importMap?: {[string]: string}, }; type StaticResult = { @@ -81,7 +82,11 @@ function prerender( const request = createRequest( children, resources, - createRenderState(resources, undefined), + createRenderState( + resources, + undefined, // nonce + options ? options.importMap : undefined, + ), createRootFormatContext(options ? options.namespaceURI : undefined), options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, diff --git a/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js b/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js index de603b20ac8e9..ecd8f54077a3b 100644 --- a/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js +++ b/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js @@ -38,6 +38,7 @@ type Options = { onError?: (error: mixed) => ?string, onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, + importMap?: {[string]: string}, }; type StaticResult = { @@ -81,7 +82,11 @@ function prerender( const request = createRequest( children, resources, - createRenderState(resources, undefined), + createRenderState( + resources, + undefined, // nonce + options ? options.importMap : undefined, + ), createRootFormatContext(options ? options.namespaceURI : undefined), options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, diff --git a/packages/react-dom/src/server/ReactDOMFizzStaticNode.js b/packages/react-dom/src/server/ReactDOMFizzStaticNode.js index d22e1ea2d06f2..d9cf813a733f0 100644 --- a/packages/react-dom/src/server/ReactDOMFizzStaticNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzStaticNode.js @@ -40,6 +40,7 @@ type Options = { onError?: (error: mixed) => ?string, onPostpone?: (reason: string) => void, unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor, + importMap?: {[string]: string}, }; type StaticResult = { @@ -95,7 +96,11 @@ function prerenderToNodeStream( const request = createRequest( children, resumableState, - createRenderState(resumableState, undefined), + createRenderState( + resumableState, + undefined, // nonce + options ? options.importMap : undefined, + ), createRootFormatContext(options ? options.namespaceURI : undefined), options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, diff --git a/packages/react-dom/src/test-utils/FizzTestUtils.js b/packages/react-dom/src/test-utils/FizzTestUtils.js index a10bec7fa0dee..545743a6445b7 100644 --- a/packages/react-dom/src/test-utils/FizzTestUtils.js +++ b/packages/react-dom/src/test-utils/FizzTestUtils.js @@ -104,9 +104,9 @@ async function executeScript(script: Element) { const newScript = ownerDocument.createElement('script'); newScript.textContent = script.textContent; // make sure to add nonce back to script if it exists - const scriptNonce = script.getAttribute('nonce'); - if (scriptNonce) { - newScript.setAttribute('nonce', scriptNonce); + for (let i = 0; i < script.attributes.length; i++) { + const attribute = script.attributes[i]; + newScript.setAttribute(attribute.name, attribute.value); } parent.insertBefore(newScript, script);