From 6bcf0d20dae349bba428d8f73dbcf0284b0acb10 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 3 Dec 2024 13:54:08 -0500 Subject: [PATCH 1/2] [compiler] Empty dep arrays for globals/module-scoped values/imports (#31666) Any LoadGlobal in the "infer deps" position can safely use an empty dep array. Globals have no reactive deps! I just keep messing up sapling. This is the revised version of #31662 --- .../src/Inference/InferEffectDependencies.ts | 72 +++++++++++-------- .../compiler/outlined-function.expect.md | 2 +- 2 files changed, 42 insertions(+), 32 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts index 5936b1bc95bc4..8c9823765bbb6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -55,6 +55,8 @@ export function inferEffectDependencies(fn: HIRFunction): void { {pruned: boolean; deps: ReactiveScopeDependencies; hasSingleInstr: boolean} >(); + const loadGlobals = new Set(); + /** * When inserting LoadLocals, we need to retain the reactivity of the base * identifier, as later passes e.g. PruneNonReactiveDeps take the reactivity of @@ -87,26 +89,23 @@ export function inferEffectDependencies(fn: HIRFunction): void { lvalue.identifier.id, instr as TInstruction, ); - } else if ( - value.kind === 'LoadGlobal' && - value.binding.kind === 'ImportSpecifier' - ) { - const moduleTargets = autodepFnConfigs.get(value.binding.module); - if (moduleTargets != null) { - const numRequiredArgs = moduleTargets.get(value.binding.imported); - if (numRequiredArgs != null) { - autodepFnLoads.set(lvalue.identifier.id, numRequiredArgs); - } - } - } else if ( - value.kind === 'LoadGlobal' && - value.binding.kind === 'ImportDefault' - ) { - const moduleTargets = autodepFnConfigs.get(value.binding.module); - if (moduleTargets != null) { - const numRequiredArgs = moduleTargets.get(DEFAULT_EXPORT); - if (numRequiredArgs != null) { - autodepFnLoads.set(lvalue.identifier.id, numRequiredArgs); + } else if (value.kind === 'LoadGlobal') { + loadGlobals.add(lvalue.identifier.id); + + if ( + value.binding.kind === 'ImportSpecifier' || + value.binding.kind === 'ImportDefault' + ) { + const moduleTargets = autodepFnConfigs.get(value.binding.module); + if (moduleTargets != null) { + const importSpecifierName = + value.binding.kind === 'ImportSpecifier' + ? value.binding.imported + : DEFAULT_EXPORT; + const numRequiredArgs = moduleTargets.get(importSpecifierName); + if (numRequiredArgs != null) { + autodepFnLoads.set(lvalue.identifier.id, numRequiredArgs); + } } } } else if ( @@ -117,8 +116,19 @@ export function inferEffectDependencies(fn: HIRFunction): void { autodepFnLoads.get(value.callee.identifier.id) === value.args.length && value.args[0].kind === 'Identifier' ) { + const effectDeps: Array = []; + const newInstructions: Array = []; + const deps: ArrayExpression = { + kind: 'ArrayExpression', + elements: effectDeps, + loc: GeneratedSource, + }; + const depsPlace = createTemporaryPlace(fn.env, GeneratedSource); + depsPlace.effect = Effect.Read; + const fnExpr = fnExpressions.get(value.args[0].identifier.id); if (fnExpr != null) { + // We have a function expression, so we can infer its dependencies const scopeInfo = fnExpr.lvalue.identifier.scope != null ? scopeInfos.get(fnExpr.lvalue.identifier.scope.id) @@ -140,14 +150,12 @@ export function inferEffectDependencies(fn: HIRFunction): void { } /** - * Step 1: write new instructions to insert a dependency array + * Step 1: push dependencies to the effect deps array * * Note that it's invalid to prune non-reactive deps in this pass, see * the `infer-effect-deps/pruned-nonreactive-obj` fixture for an * explanation. */ - const effectDeps: Array = []; - const newInstructions: Array = []; for (const dep of scopeInfo.deps) { const {place, instructions} = writeDependencyToInstructions( dep, @@ -158,14 +166,6 @@ export function inferEffectDependencies(fn: HIRFunction): void { newInstructions.push(...instructions); effectDeps.push(place); } - const deps: ArrayExpression = { - kind: 'ArrayExpression', - elements: effectDeps, - loc: GeneratedSource, - }; - - const depsPlace = createTemporaryPlace(fn.env, GeneratedSource); - depsPlace.effect = Effect.Read; newInstructions.push({ id: makeInstructionId(0), @@ -177,6 +177,16 @@ export function inferEffectDependencies(fn: HIRFunction): void { // Step 2: push the inferred deps array as an argument of the useEffect value.args.push({...depsPlace, effect: Effect.Freeze}); rewriteInstrs.set(instr.id, newInstructions); + } else if (loadGlobals.has(value.args[0].identifier.id)) { + // Global functions have no reactive dependencies, so we can insert an empty array + newInstructions.push({ + id: makeInstructionId(0), + loc: GeneratedSource, + lvalue: {...depsPlace, effect: Effect.Mutate}, + value: deps, + }); + value.args.push({...depsPlace, effect: Effect.Freeze}); + rewriteInstrs.set(instr.id, newInstructions); } } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlined-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlined-function.expect.md index 8272d4793fb8a..e941ac1462bbe 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlined-function.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlined-function.expect.md @@ -34,7 +34,7 @@ import { print } from "shared-runtime"; * before OutlineFunctions */ function OutlinedFunctionInEffect() { - useEffect(_temp); + useEffect(_temp, []); } function _temp() { return print("hello world!"); From 16d2bbbd1f1617d636ea0fd271b902a12a763c27 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Tue, 3 Dec 2024 13:13:35 -0800 Subject: [PATCH 2/2] Client render dehydrated Suspense boundaries on document load (#31620) When streaming SSR while hydrating React will wait for Suspense boundaries to be revealed by the SSR stream before attempting to hydrate them. The rationale here is that the Server render is likely further ahead of whatever the client would produce so waiting to let the server stream in the UI is preferable to retrying on the client and possibly delaying how quickly the primary content becomes available. However If the connection closes early (user hits stop for instance) or there is a server error which prevents additional HTML from being delivered to the client this can put React into a broken state where the boundary never resolves nor errors and the hydration never retries that boundary freezing it in it's fallback state. Once the document has fully loaded we know there is not way any additional Suspense boundaries can arrive. This update changes react-dom on the client to schedule client renders for any unfinished Suspense boundaries upon document loading. The technique for client rendering a fallback is pretty straight forward. When hydrating a Suspense boundary if the Document is in 'complete' readyState we interpret pending boundaries as fallback boundaries. If the readyState is not 'complete' we register an event to retry the boundary when the DOMContentLoaded event fires. To test this I needed JSDOM to model readyState. We previously had a temporary implementation of readyState for SSR streaming but I ended up implementing this as a mock of JSDOM that implements a fake readyState that is mutable. It starts off in 'loading' readyState and you can advance it by mutating document.readyState. You can also reset it to 'loading'. It fires events when changing states. This seems like the least invasive way to get closer-to-real-browser behavior in a way that won't require remembering this subtle detail every time you create a test that asserts Suspense resolution order. --- packages/internal-test-utils/ReactJSDOM.js | 20 + .../internal-test-utils/ReactJSDOMUtils.js | 33 ++ .../src/client/ReactFiberConfigDOM.js | 22 +- .../src/__tests__/ReactDOMFizzServer-test.js | 349 +++++++++++++----- .../src/__tests__/ReactDOMFloat-test.js | 179 +++++---- .../react-dom/src/test-utils/FizzTestUtils.js | 34 -- .../ReactDOMServerIntegrationEnvironment.js | 4 +- scripts/jest/ReactJSDOMEnvironment.js | 19 + scripts/jest/config.base.js | 2 +- scripts/jest/setupTests.js | 7 + 10 files changed, 445 insertions(+), 224 deletions(-) create mode 100644 packages/internal-test-utils/ReactJSDOM.js create mode 100644 packages/internal-test-utils/ReactJSDOMUtils.js create mode 100644 scripts/jest/ReactJSDOMEnvironment.js diff --git a/packages/internal-test-utils/ReactJSDOM.js b/packages/internal-test-utils/ReactJSDOM.js new file mode 100644 index 0000000000000..12ce623718227 --- /dev/null +++ b/packages/internal-test-utils/ReactJSDOM.js @@ -0,0 +1,20 @@ +const JSDOMModule = jest.requireActual('jsdom'); + +const OriginalJSDOM = JSDOMModule.JSDOM; + +module.exports = JSDOMModule; +module.exports.JSDOM = function JSDOM() { + let result; + if (new.target) { + result = Reflect.construct(OriginalJSDOM, arguments); + } else { + result = JSDOM.apply(undefined, arguments); + } + + require('./ReactJSDOMUtils').setupDocumentReadyState( + result.window.document, + result.window.Event, + ); + + return result; +}; diff --git a/packages/internal-test-utils/ReactJSDOMUtils.js b/packages/internal-test-utils/ReactJSDOMUtils.js new file mode 100644 index 0000000000000..04a0d18fc4221 --- /dev/null +++ b/packages/internal-test-utils/ReactJSDOMUtils.js @@ -0,0 +1,33 @@ +export function setupDocumentReadyState( + document: Document, + Event: typeof Event, +) { + let readyState: 0 | 1 | 2 = 0; + Object.defineProperty(document, 'readyState', { + get() { + switch (readyState) { + case 0: + return 'loading'; + case 1: + return 'interactive'; + case 2: + return 'complete'; + } + }, + set(value) { + if (value === 'interactive' && readyState < 1) { + readyState = 1; + document.dispatchEvent(new Event('readystatechange')); + } else if (value === 'complete' && readyState < 2) { + readyState = 2; + document.dispatchEvent(new Event('readystatechange')); + document.dispatchEvent(new Event('DOMContentLoaded')); + } else if (value === 'loading') { + // We allow resetting the readyState to loading mostly for pragamtism. + // tests that use this environment don't reset the document between tests. + readyState = 0; + } + }, + configurable: true, + }); +} diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index bfa9adb48d737..bac5ec8fb601b 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -194,6 +194,8 @@ const SUSPENSE_FALLBACK_START_DATA = '$!'; const FORM_STATE_IS_MATCHING = 'F!'; const FORM_STATE_IS_NOT_MATCHING = 'F'; +const DOCUMENT_READY_STATE_COMPLETE = 'complete'; + const STYLE = 'style'; opaque type HostContextNamespace = 0 | 1 | 2; @@ -1262,7 +1264,11 @@ export function isSuspenseInstancePending(instance: SuspenseInstance): boolean { export function isSuspenseInstanceFallback( instance: SuspenseInstance, ): boolean { - return instance.data === SUSPENSE_FALLBACK_START_DATA; + return ( + instance.data === SUSPENSE_FALLBACK_START_DATA || + (instance.data === SUSPENSE_PENDING_START_DATA && + instance.ownerDocument.readyState === DOCUMENT_READY_STATE_COMPLETE) + ); } export function getSuspenseInstanceFallbackErrorDetails( @@ -1303,6 +1309,20 @@ export function registerSuspenseInstanceRetry( instance: SuspenseInstance, callback: () => void, ) { + const ownerDocument = instance.ownerDocument; + if (ownerDocument.readyState !== DOCUMENT_READY_STATE_COMPLETE) { + ownerDocument.addEventListener( + 'DOMContentLoaded', + () => { + if (instance.data === SUSPENSE_PENDING_START_DATA) { + callback(); + } + }, + { + once: true, + }, + ); + } instance._reactRetry = callback; } diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index a2ca222daf04b..8a8619ff87e7a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -13,7 +13,6 @@ import { insertNodesAndExecuteScripts, mergeOptions, stripExternalRuntimeInNodes, - withLoadingReadyState, getVisibleChildren, } from '../test-utils/FizzTestUtils'; @@ -210,117 +209,111 @@ describe('ReactDOMFizzServer', () => { return; } - await withLoadingReadyState(async () => { - const bodyMatch = bufferedContent.match(bodyStartMatch); - const headMatch = bufferedContent.match(headStartMatch); - - if (streamingContainer === null) { - // This is the first streamed content. We decide here where to insert it. If we get , , or - // we abandon the pre-built document and start from scratch. If we get anything else we assume it goes into the - // container. This is not really production behavior because you can't correctly stream into a deep div effectively - // but it's pragmatic for tests. - - if ( - bufferedContent.startsWith('') || - bufferedContent.startsWith('') || - bufferedContent.startsWith('') || - bufferedContent.startsWith(' without a which is almost certainly a bug in React', - ); - } - - if (bufferedContent.startsWith('')) { - // we can just use the whole document - const tempDom = new JSDOM(bufferedContent); - - // Wipe existing head and body content - document.head.innerHTML = ''; - document.body.innerHTML = ''; + const bodyMatch = bufferedContent.match(bodyStartMatch); + const headMatch = bufferedContent.match(headStartMatch); + + if (streamingContainer === null) { + // This is the first streamed content. We decide here where to insert it. If we get , , or + // we abandon the pre-built document and start from scratch. If we get anything else we assume it goes into the + // container. This is not really production behavior because you can't correctly stream into a deep div effectively + // but it's pragmatic for tests. + + if ( + bufferedContent.startsWith('') || + bufferedContent.startsWith('') || + bufferedContent.startsWith('') || + bufferedContent.startsWith(' without a which is almost certainly a bug in React', + ); + } - // Copy the attributes over - const tempHtmlNode = tempDom.window.document.documentElement; - for (let i = 0; i < tempHtmlNode.attributes.length; i++) { - const attr = tempHtmlNode.attributes[i]; - document.documentElement.setAttribute(attr.name, attr.value); - } + if (bufferedContent.startsWith('')) { + // we can just use the whole document + const tempDom = new JSDOM(bufferedContent); - if (headMatch) { - // We parsed a head open tag. we need to copy head attributes and insert future - // content into - streamingContainer = document.head; - const tempHeadNode = tempDom.window.document.head; - for (let i = 0; i < tempHeadNode.attributes.length; i++) { - const attr = tempHeadNode.attributes[i]; - document.head.setAttribute(attr.name, attr.value); - } - const source = document.createElement('head'); - source.innerHTML = tempHeadNode.innerHTML; - await insertNodesAndExecuteScripts(source, document.head, CSPnonce); - } + // Wipe existing head and body content + document.head.innerHTML = ''; + document.body.innerHTML = ''; - if (bodyMatch) { - // We parsed a body open tag. we need to copy head attributes and insert future - // content into - streamingContainer = document.body; - const tempBodyNode = tempDom.window.document.body; - for (let i = 0; i < tempBodyNode.attributes.length; i++) { - const attr = tempBodyNode.attributes[i]; - document.body.setAttribute(attr.name, attr.value); - } - const source = document.createElement('body'); - source.innerHTML = tempBodyNode.innerHTML; - await insertNodesAndExecuteScripts(source, document.body, CSPnonce); - } + // Copy the attributes over + const tempHtmlNode = tempDom.window.document.documentElement; + for (let i = 0; i < tempHtmlNode.attributes.length; i++) { + const attr = tempHtmlNode.attributes[i]; + document.documentElement.setAttribute(attr.name, attr.value); + } - if (!headMatch && !bodyMatch) { - throw new Error('expected or after '); + if (headMatch) { + // We parsed a head open tag. we need to copy head attributes and insert future + // content into + streamingContainer = document.head; + const tempHeadNode = tempDom.window.document.head; + for (let i = 0; i < tempHeadNode.attributes.length; i++) { + const attr = tempHeadNode.attributes[i]; + document.head.setAttribute(attr.name, attr.value); } - } else { - // we assume we are streaming into the default container' - streamingContainer = container; - const div = document.createElement('div'); - div.innerHTML = bufferedContent; - await insertNodesAndExecuteScripts(div, container, CSPnonce); + const source = document.createElement('head'); + source.innerHTML = tempHeadNode.innerHTML; + await insertNodesAndExecuteScripts(source, document.head, CSPnonce); } - } else if (streamingContainer === document.head) { - bufferedContent = '' + bufferedContent; - const tempDom = new JSDOM(bufferedContent); - - const tempHeadNode = tempDom.window.document.head; - const source = document.createElement('head'); - source.innerHTML = tempHeadNode.innerHTML; - await insertNodesAndExecuteScripts(source, document.head, CSPnonce); if (bodyMatch) { + // We parsed a body open tag. we need to copy head attributes and insert future + // content into streamingContainer = document.body; - const tempBodyNode = tempDom.window.document.body; for (let i = 0; i < tempBodyNode.attributes.length; i++) { const attr = tempBodyNode.attributes[i]; document.body.setAttribute(attr.name, attr.value); } - const bodySource = document.createElement('body'); - bodySource.innerHTML = tempBodyNode.innerHTML; - await insertNodesAndExecuteScripts( - bodySource, - document.body, - CSPnonce, - ); + const source = document.createElement('body'); + source.innerHTML = tempBodyNode.innerHTML; + await insertNodesAndExecuteScripts(source, document.body, CSPnonce); + } + + if (!headMatch && !bodyMatch) { + throw new Error('expected or after '); } } else { + // we assume we are streaming into the default container' + streamingContainer = container; const div = document.createElement('div'); div.innerHTML = bufferedContent; - await insertNodesAndExecuteScripts(div, streamingContainer, CSPnonce); + await insertNodesAndExecuteScripts(div, container, CSPnonce); } - }, document); + } else if (streamingContainer === document.head) { + bufferedContent = '' + bufferedContent; + const tempDom = new JSDOM(bufferedContent); + + const tempHeadNode = tempDom.window.document.head; + const source = document.createElement('head'); + source.innerHTML = tempHeadNode.innerHTML; + await insertNodesAndExecuteScripts(source, document.head, CSPnonce); + + if (bodyMatch) { + streamingContainer = document.body; + + const tempBodyNode = tempDom.window.document.body; + for (let i = 0; i < tempBodyNode.attributes.length; i++) { + const attr = tempBodyNode.attributes[i]; + document.body.setAttribute(attr.name, attr.value); + } + const bodySource = document.createElement('body'); + bodySource.innerHTML = tempBodyNode.innerHTML; + await insertNodesAndExecuteScripts(bodySource, document.body, CSPnonce); + } + } else { + const div = document.createElement('div'); + div.innerHTML = bufferedContent; + await insertNodesAndExecuteScripts(div, streamingContainer, CSPnonce); + } } function resolveText(text) { @@ -8738,4 +8731,174 @@ describe('ReactDOMFizzServer', () => { expect(caughtError.message).toBe('Maximum call stack size exceeded'); }); + + it('client renders incomplete Suspense boundaries when the document is no longer loading when hydration begins', async () => { + let resolve; + const promise = new Promise(r => { + resolve = r; + }); + + function Blocking() { + React.use(promise); + return null; + } + + function App() { + return ( +
+

outside

+ loading...

}> + +

inside

+ +
+ ); + } + + const errors = []; + await act(() => { + const {pipe} = renderToPipeableStream(, { + onError(err) { + errors.push(err.message); + }, + }); + pipe(writable); + }); + + expect(getVisibleChildren(container)).toEqual( +
+

outside

+

loading...

+
, + ); + + await act(() => { + // We now end the stream and resolve the promise that was blocking the boundary + // Because the stream is ended it won't actually propagate to the client + writable.end(); + document.readyState = 'complete'; + resolve(); + }); + // ending the stream early will cause it to error on the server + expect(errors).toEqual([ + expect.stringContaining('The destination stream closed early'), + ]); + expect(getVisibleChildren(container)).toEqual( +
+

outside

+

loading...

+
, + ); + + const clientErrors = []; + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error, errorInfo) { + clientErrors.push(error.message); + }, + }); + await waitForAll([]); + // When we hydrate the client the document is already not loading + // so we client render the boundary in fallback + expect(getVisibleChildren(container)).toEqual( +
+

outside

+

inside

+
, + ); + expect(clientErrors).toEqual([ + expect.stringContaining( + 'The server could not finish this Suspense boundar', + ), + ]); + }); + + it('client renders incomplete Suspense boundaries when the document stops loading during hydration', async () => { + let resolve; + const promise = new Promise(r => { + resolve = r; + }); + + function Blocking() { + React.use(promise); + return null; + } + + function App() { + return ( +
+

outside

+ loading...

}> + +

inside

+ +
+ ); + } + + const errors = []; + await act(() => { + const {pipe} = renderToPipeableStream(, { + onError(err) { + errors.push(err.message); + }, + }); + pipe(writable); + }); + + expect(getVisibleChildren(container)).toEqual( +
+

outside

+

loading...

+
, + ); + + await act(() => { + // We now end the stream and resolve the promise that was blocking the boundary + // Because the stream is ended it won't actually propagate to the client + writable.end(); + resolve(); + }); + // ending the stream early will cause it to error on the server + expect(errors).toEqual([ + expect.stringContaining('The destination stream closed early'), + ]); + expect(getVisibleChildren(container)).toEqual( +
+

outside

+

loading...

+
, + ); + + const clientErrors = []; + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error, errorInfo) { + clientErrors.push(error.message); + }, + }); + await waitForAll([]); + // When we hydrate the client is still waiting for the blocked boundary + // and won't client render unless the document is no longer loading + expect(getVisibleChildren(container)).toEqual( +
+

outside

+

loading...

+
, + ); + + document.readyState = 'complete'; + await waitForAll([]); + // Now that the document is no longer in loading readyState it will client + // render the boundary in fallback + expect(getVisibleChildren(container)).toEqual( +
+

outside

+

inside

+
, + ); + expect(clientErrors).toEqual([ + expect.stringContaining( + 'The server could not finish this Suspense boundar', + ), + ]); + }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index 5de0cc9e2f49c..360d51973579d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -12,7 +12,6 @@ import { insertNodesAndExecuteScripts, mergeOptions, - withLoadingReadyState, } from '../test-utils/FizzTestUtils'; let JSDOM; @@ -126,117 +125,111 @@ describe('ReactDOMFloat', () => { return; } - await withLoadingReadyState(async () => { - const bodyMatch = bufferedContent.match(bodyStartMatch); - const headMatch = bufferedContent.match(headStartMatch); - - if (streamingContainer === null) { - // This is the first streamed content. We decide here where to insert it. If we get , , or - // we abandon the pre-built document and start from scratch. If we get anything else we assume it goes into the - // container. This is not really production behavior because you can't correctly stream into a deep div effectively - // but it's pragmatic for tests. - - if ( - bufferedContent.startsWith('') || - bufferedContent.startsWith('') || - bufferedContent.startsWith('') || - bufferedContent.startsWith(' without a which is almost certainly a bug in React', - ); - } - - if (bufferedContent.startsWith('')) { - // we can just use the whole document - const tempDom = new JSDOM(bufferedContent); - - // Wipe existing head and body content - document.head.innerHTML = ''; - document.body.innerHTML = ''; + const bodyMatch = bufferedContent.match(bodyStartMatch); + const headMatch = bufferedContent.match(headStartMatch); + + if (streamingContainer === null) { + // This is the first streamed content. We decide here where to insert it. If we get , , or + // we abandon the pre-built document and start from scratch. If we get anything else we assume it goes into the + // container. This is not really production behavior because you can't correctly stream into a deep div effectively + // but it's pragmatic for tests. + + if ( + bufferedContent.startsWith('') || + bufferedContent.startsWith('') || + bufferedContent.startsWith('') || + bufferedContent.startsWith(' without a which is almost certainly a bug in React', + ); + } - // Copy the attributes over - const tempHtmlNode = tempDom.window.document.documentElement; - for (let i = 0; i < tempHtmlNode.attributes.length; i++) { - const attr = tempHtmlNode.attributes[i]; - document.documentElement.setAttribute(attr.name, attr.value); - } + if (bufferedContent.startsWith('')) { + // we can just use the whole document + const tempDom = new JSDOM(bufferedContent); - if (headMatch) { - // We parsed a head open tag. we need to copy head attributes and insert future - // content into - streamingContainer = document.head; - const tempHeadNode = tempDom.window.document.head; - for (let i = 0; i < tempHeadNode.attributes.length; i++) { - const attr = tempHeadNode.attributes[i]; - document.head.setAttribute(attr.name, attr.value); - } - const source = document.createElement('head'); - source.innerHTML = tempHeadNode.innerHTML; - await insertNodesAndExecuteScripts(source, document.head, CSPnonce); - } + // Wipe existing head and body content + document.head.innerHTML = ''; + document.body.innerHTML = ''; - if (bodyMatch) { - // We parsed a body open tag. we need to copy head attributes and insert future - // content into - streamingContainer = document.body; - const tempBodyNode = tempDom.window.document.body; - for (let i = 0; i < tempBodyNode.attributes.length; i++) { - const attr = tempBodyNode.attributes[i]; - document.body.setAttribute(attr.name, attr.value); - } - const source = document.createElement('body'); - source.innerHTML = tempBodyNode.innerHTML; - await insertNodesAndExecuteScripts(source, document.body, CSPnonce); - } + // Copy the attributes over + const tempHtmlNode = tempDom.window.document.documentElement; + for (let i = 0; i < tempHtmlNode.attributes.length; i++) { + const attr = tempHtmlNode.attributes[i]; + document.documentElement.setAttribute(attr.name, attr.value); + } - if (!headMatch && !bodyMatch) { - throw new Error('expected or after '); + if (headMatch) { + // We parsed a head open tag. we need to copy head attributes and insert future + // content into + streamingContainer = document.head; + const tempHeadNode = tempDom.window.document.head; + for (let i = 0; i < tempHeadNode.attributes.length; i++) { + const attr = tempHeadNode.attributes[i]; + document.head.setAttribute(attr.name, attr.value); } - } else { - // we assume we are streaming into the default container' - streamingContainer = container; - const div = document.createElement('div'); - div.innerHTML = bufferedContent; - await insertNodesAndExecuteScripts(div, container, CSPnonce); + const source = document.createElement('head'); + source.innerHTML = tempHeadNode.innerHTML; + await insertNodesAndExecuteScripts(source, document.head, CSPnonce); } - } else if (streamingContainer === document.head) { - bufferedContent = '' + bufferedContent; - const tempDom = new JSDOM(bufferedContent); - - const tempHeadNode = tempDom.window.document.head; - const source = document.createElement('head'); - source.innerHTML = tempHeadNode.innerHTML; - await insertNodesAndExecuteScripts(source, document.head, CSPnonce); if (bodyMatch) { + // We parsed a body open tag. we need to copy head attributes and insert future + // content into streamingContainer = document.body; - const tempBodyNode = tempDom.window.document.body; for (let i = 0; i < tempBodyNode.attributes.length; i++) { const attr = tempBodyNode.attributes[i]; document.body.setAttribute(attr.name, attr.value); } - const bodySource = document.createElement('body'); - bodySource.innerHTML = tempBodyNode.innerHTML; - await insertNodesAndExecuteScripts( - bodySource, - document.body, - CSPnonce, - ); + const source = document.createElement('body'); + source.innerHTML = tempBodyNode.innerHTML; + await insertNodesAndExecuteScripts(source, document.body, CSPnonce); + } + + if (!headMatch && !bodyMatch) { + throw new Error('expected or after '); } } else { + // we assume we are streaming into the default container' + streamingContainer = container; const div = document.createElement('div'); div.innerHTML = bufferedContent; - await insertNodesAndExecuteScripts(div, streamingContainer, CSPnonce); + await insertNodesAndExecuteScripts(div, container, CSPnonce); } - }, document); + } else if (streamingContainer === document.head) { + bufferedContent = '' + bufferedContent; + const tempDom = new JSDOM(bufferedContent); + + const tempHeadNode = tempDom.window.document.head; + const source = document.createElement('head'); + source.innerHTML = tempHeadNode.innerHTML; + await insertNodesAndExecuteScripts(source, document.head, CSPnonce); + + if (bodyMatch) { + streamingContainer = document.body; + + const tempBodyNode = tempDom.window.document.body; + for (let i = 0; i < tempBodyNode.attributes.length; i++) { + const attr = tempBodyNode.attributes[i]; + document.body.setAttribute(attr.name, attr.value); + } + const bodySource = document.createElement('body'); + bodySource.innerHTML = tempBodyNode.innerHTML; + await insertNodesAndExecuteScripts(bodySource, document.body, CSPnonce); + } + } else { + const div = document.createElement('div'); + div.innerHTML = bufferedContent; + await insertNodesAndExecuteScripts(div, streamingContainer, CSPnonce); + } } function getMeaningfulChildren(element) { diff --git a/packages/react-dom/src/test-utils/FizzTestUtils.js b/packages/react-dom/src/test-utils/FizzTestUtils.js index b709cc50d91e1..537c64a889a7d 100644 --- a/packages/react-dom/src/test-utils/FizzTestUtils.js +++ b/packages/react-dom/src/test-utils/FizzTestUtils.js @@ -139,39 +139,6 @@ function stripExternalRuntimeInNodes( ); } -// Since JSDOM doesn't implement a streaming HTML parser, we manually overwrite -// readyState here (currently read by ReactDOMServerExternalRuntime). This does -// not trigger event callbacks, but we do not rely on any right now. -async function withLoadingReadyState( - fn: () => T, - document: Document, -): Promise { - // JSDOM implements readyState in document's direct prototype, but this may - // change in later versions - let prevDescriptor = null; - let proto: Object = document; - while (proto != null) { - prevDescriptor = Object.getOwnPropertyDescriptor(proto, 'readyState'); - if (prevDescriptor != null) { - break; - } - proto = Object.getPrototypeOf(proto); - } - Object.defineProperty(document, 'readyState', { - get() { - return 'loading'; - }, - configurable: true, - }); - const result = await fn(); - // $FlowFixMe[incompatible-type] - delete document.readyState; - if (prevDescriptor) { - Object.defineProperty(proto, 'readyState', prevDescriptor); - } - return result; -} - function getVisibleChildren(element: Element): React$Node { const children = []; let node: any = element.firstChild; @@ -218,6 +185,5 @@ export { insertNodesAndExecuteScripts, mergeOptions, stripExternalRuntimeInNodes, - withLoadingReadyState, getVisibleChildren, }; diff --git a/scripts/jest/ReactDOMServerIntegrationEnvironment.js b/scripts/jest/ReactDOMServerIntegrationEnvironment.js index 1d92807ee8ddd..5a0ab2f665389 100644 --- a/scripts/jest/ReactDOMServerIntegrationEnvironment.js +++ b/scripts/jest/ReactDOMServerIntegrationEnvironment.js @@ -1,6 +1,6 @@ 'use strict'; -const {TestEnvironment: JSDOMEnvironment} = require('jest-environment-jsdom'); +const ReactJSDOMEnvironment = require('./ReactJSDOMEnvironment'); const {TestEnvironment: NodeEnvironment} = require('jest-environment-node'); /** @@ -10,7 +10,7 @@ class ReactDOMServerIntegrationEnvironment extends NodeEnvironment { constructor(config, context) { super(config, context); - this.domEnvironment = new JSDOMEnvironment(config, context); + this.domEnvironment = new ReactJSDOMEnvironment(config, context); this.global.window = this.domEnvironment.dom.window; this.global.document = this.global.window.document; diff --git a/scripts/jest/ReactJSDOMEnvironment.js b/scripts/jest/ReactJSDOMEnvironment.js new file mode 100644 index 0000000000000..eb686d6f4be27 --- /dev/null +++ b/scripts/jest/ReactJSDOMEnvironment.js @@ -0,0 +1,19 @@ +'use strict'; + +const {TestEnvironment: JSDOMEnvironment} = require('jest-environment-jsdom'); +const { + setupDocumentReadyState, +} = require('internal-test-utils/ReactJSDOMUtils'); + +/** + * Test environment for testing integration of react-dom (browser) with react-dom/server (node) + */ +class ReactJSDOMEnvironment extends JSDOMEnvironment { + constructor(config, context) { + super(config, context); + + setupDocumentReadyState(this.global.document, this.global.Event); + } +} + +module.exports = ReactJSDOMEnvironment; diff --git a/scripts/jest/config.base.js b/scripts/jest/config.base.js index 263010a87b451..6fa4a3619429f 100644 --- a/scripts/jest/config.base.js +++ b/scripts/jest/config.base.js @@ -24,7 +24,7 @@ module.exports = { }, snapshotSerializers: [require.resolve('jest-snapshot-serializer-raw')], - testEnvironment: 'jsdom', + testEnvironment: '/scripts/jest/ReactJSDOMEnvironment', testRunner: 'jest-circus/runner', }; diff --git a/scripts/jest/setupTests.js b/scripts/jest/setupTests.js index d339c86433a41..d642e77a2a09e 100644 --- a/scripts/jest/setupTests.js +++ b/scripts/jest/setupTests.js @@ -274,4 +274,11 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { const flags = getTestFlags(); return gateFn(flags); }; + + // We augment JSDOM to produce a document that has a loading readyState by default + // and can be changed. We mock it here globally so we don't have to import our special + // mock in every file. + jest.mock('jsdom', () => { + return require('internal-test-utils/ReactJSDOM.js'); + }); }