From d2767c96e80a6fdc35b002f1518d01d90e2a8528 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 4 Jun 2024 18:10:06 -0400 Subject: [PATCH 01/15] [Flight] Encode fragments properly in DEV (#29762) Normally we take the renderClientElement path but this is an internal fast path. No tests because we don't run tests with console.createTask (which is not easy since we test component stacks). Ideally this would be covered by types but since the types don't consider flags and DEV it doesn't really help. --- .../react-server/src/ReactFlightServer.js | 50 ++++++++++++++----- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 63c7871cfa20b..f1f471d8f6840 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1215,12 +1215,25 @@ function renderFragment( if (task.keyPath !== null) { // We have a Server Component that specifies a key but we're now splitting // the tree using a fragment. - const fragment = [ - REACT_ELEMENT_TYPE, - REACT_FRAGMENT_TYPE, - task.keyPath, - {children}, - ]; + const fragment = __DEV__ + ? enableOwnerStacks + ? [ + REACT_ELEMENT_TYPE, + REACT_FRAGMENT_TYPE, + task.keyPath, + {children}, + null, + null, + 0, + ] + : [ + REACT_ELEMENT_TYPE, + REACT_FRAGMENT_TYPE, + task.keyPath, + {children}, + null, + ] + : [REACT_ELEMENT_TYPE, REACT_FRAGMENT_TYPE, task.keyPath, {children}]; if (!task.implicitSlot) { // If this was keyed inside a set. I.e. the outer Server Component was keyed // then we need to handle reorders of the whole set. To do this we need to wrap @@ -1274,12 +1287,25 @@ function renderAsyncFragment( if (task.keyPath !== null) { // We have a Server Component that specifies a key but we're now splitting // the tree using a fragment. - const fragment = [ - REACT_ELEMENT_TYPE, - REACT_FRAGMENT_TYPE, - task.keyPath, - {children}, - ]; + const fragment = __DEV__ + ? enableOwnerStacks + ? [ + REACT_ELEMENT_TYPE, + REACT_FRAGMENT_TYPE, + task.keyPath, + {children}, + null, + null, + 0, + ] + : [ + REACT_ELEMENT_TYPE, + REACT_FRAGMENT_TYPE, + task.keyPath, + {children}, + null, + ] + : [REACT_ELEMENT_TYPE, REACT_FRAGMENT_TYPE, task.keyPath, {children}]; if (!task.implicitSlot) { // If this was keyed inside a set. I.e. the outer Server Component was keyed // then we need to handle reorders of the whole set. To do this we need to wrap From 1df34bdf626af3e4566364dcdf7f1c387d2f4252 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 5 Jun 2024 03:41:37 -0400 Subject: [PATCH 02/15] [Flight] Override prepareStackTrace when reading stacks (#29740) This lets us ensure that we use the original V8 format and it lets us skip source mapping. Source mapping every call can be expensive since we do it eagerly for server components even if an error doesn't happen. In the case of an error being thrown we don't actually always do this in practice because if a try/catch before us touches it or if something in onError touches it (which the default console.error does), it has already been initialized. So we have to be resilient to thrown errors having other formats. These are not as perf sensitive since something actually threw but if you want better perf in these cases, you can simply do something like `onError(error) { console.error(error.message) }` instead. The server has to be aware whether it's looking up original or compiled output. I currently use the file:// check to determine if it's referring to a source mapped file or compiled file in the fixture. A bundled app can more easily check if it's a bundle or not. --- .eslintrc.js | 1 + fixtures/flight/server/region.js | 26 ++++++----- .../react-server/src/ReactFlightServer.js | 45 +++++++++++++++---- 3 files changed, 52 insertions(+), 20 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index cf5b58587085a..ec20e2196e94b 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -486,6 +486,7 @@ module.exports = { $ReadOnlyArray: 'readonly', $ArrayBufferView: 'readonly', $Shape: 'readonly', + CallSite: 'readonly', ConsoleTask: 'readonly', // TOOD: Figure out what the official name of this will be. ReturnType: 'readonly', AnimationFrameID: 'readonly', diff --git a/fixtures/flight/server/region.js b/fixtures/flight/server/region.js index d2136d8b91a4c..4313f48502da1 100644 --- a/fixtures/flight/server/region.js +++ b/fixtures/flight/server/region.js @@ -187,7 +187,11 @@ if (process.env.NODE_ENV === 'development') { res.set('Content-type', 'application/json'); let requestedFilePath = req.query.name; + let isCompiledOutput = false; if (requestedFilePath.startsWith('file://')) { + // We assume that if it was prefixed with file:// it's referring to the compiled output + // and if it's a direct file path we assume it's source mapped back to original format. + isCompiledOutput = true; requestedFilePath = requestedFilePath.slice(7); } @@ -204,11 +208,11 @@ if (process.env.NODE_ENV === 'development') { let map; // There are two ways to return a source map depending on what we observe in error.stack. // A real app will have a similar choice to make for which strategy to pick. - if (!sourceMap || Error.prepareStackTrace === undefined) { - // When --enable-source-maps is enabled, the error.stack that we use to track - // stacks will have had the source map already applied so it's pointing to the - // original source. We return a blank source map that just maps everything to - // the original source in this case. + if (!sourceMap || !isCompiledOutput) { + // If a file doesn't have a source map, such as this file, then we generate a blank + // source map that just contains the original content and segments pointing to the + // original lines. + // Similarly const sourceContent = await readFile(requestedFilePath, 'utf8'); const lines = sourceContent.split('\n').length; map = { @@ -222,13 +226,11 @@ if (process.env.NODE_ENV === 'development') { sourceRoot: '', }; } else { - // If something has overridden prepareStackTrace it is likely not getting the - // natively applied source mapping to error.stack and so the line will point to - // the compiled output similar to how a browser works. - // E.g. ironically this can happen with the source-map-support library that is - // auto-invoked by @babel/register if external source maps are generated. - // In this case we just use the source map that the native source mapping would - // have used. + // We always set prepareStackTrace before reading the stack so that we get the stack + // without source maps applied. Therefore we have to use the original source map. + // If something read .stack before we did, we might observe the line/column after + // source mapping back to the original file. We use the isCompiledOutput check above + // in that case. map = sourceMap.payload; } res.write(JSON.stringify(map)); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index f1f471d8f6840..790bda2457340 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -137,10 +137,41 @@ function isNotExternal(stackFrame: string): boolean { return !externalRegExp.test(stackFrame); } +function prepareStackTrace( + error: Error, + structuredStackTrace: CallSite[], +): string { + const name = error.name || 'Error'; + const message = error.message || ''; + let stack = name + ': ' + message; + for (let i = 0; i < structuredStackTrace.length; i++) { + stack += '\n at ' + structuredStackTrace[i].toString(); + } + return stack; +} + +function getStack(error: Error): string { + // We override Error.prepareStackTrace with our own version that normalizes + // the stack to V8 formatting even if the server uses other formatting. + // It also ensures that source maps are NOT applied to this since that can + // be slow we're better off doing that lazily from the client instead of + // eagerly on the server. If the stack has already been read, then we might + // not get a normalized stack and it might still have been source mapped. + // So the client still needs to be resilient to this. + const previousPrepare = Error.prepareStackTrace; + Error.prepareStackTrace = prepareStackTrace; + try { + // eslint-disable-next-line react-internal/safe-string-coercion + return String(error.stack); + } finally { + Error.prepareStackTrace = previousPrepare; + } +} + function initCallComponentFrame(): string { // Extract the stack frame of the callComponentInDEV function. const error = callComponentInDEV(Error, 'react-stack-top-frame', {}); - const stack = error.stack; + const stack = getStack(error); const startIdx = stack.startsWith('Error: react-stack-top-frame\n') ? 29 : 0; const endIdx = stack.indexOf('\n', startIdx); if (endIdx === -1) { @@ -155,7 +186,7 @@ function initCallIteratorFrame(): string { (callIteratorInDEV: any)({next: null}); return ''; } catch (error) { - const stack = error.stack; + const stack = getStack(error); const startIdx = stack.startsWith('TypeError: ') ? stack.indexOf('\n') + 1 : 0; @@ -174,7 +205,7 @@ function initCallLazyInitFrame(): string { _init: Error, _payload: 'react-stack-top-frame', }); - const stack = error.stack; + const stack = getStack(error); const startIdx = stack.startsWith('Error: react-stack-top-frame\n') ? 29 : 0; const endIdx = stack.indexOf('\n', startIdx); if (endIdx === -1) { @@ -188,7 +219,7 @@ function filterDebugStack(error: Error): string { // to save bandwidth even in DEV. We'll also replay these stacks on the client so by // stripping them early we avoid that overhead. Otherwise we'd normally just rely on // the DevTools or framework's ignore lists to filter them out. - let stack = error.stack; + let stack = getStack(error); if (stack.startsWith('Error: react-stack-top-frame\n')) { // V8's default formatting prefixes with the error message which we // don't want/need. @@ -2601,8 +2632,7 @@ function emitPostponeChunk( try { // eslint-disable-next-line react-internal/safe-string-coercion reason = String(postponeInstance.message); - // eslint-disable-next-line react-internal/safe-string-coercion - stack = String(postponeInstance.stack); + stack = getStack(postponeInstance); } catch (x) {} row = serializeRowHeader('P', id) + stringify({reason, stack}) + '\n'; } else { @@ -2627,8 +2657,7 @@ function emitErrorChunk( if (error instanceof Error) { // eslint-disable-next-line react-internal/safe-string-coercion message = String(error.message); - // eslint-disable-next-line react-internal/safe-string-coercion - stack = String(error.stack); + stack = getStack(error); } else if (typeof error === 'object' && error !== null) { message = describeObjectForErrorMessage(error); } else { From 8d87e374ac69904012530af702af1cd51d90e07d Mon Sep 17 00:00:00 2001 From: Batuhan Tomo <91488737+Rekl0w@users.noreply.github.com> Date: Wed, 5 Jun 2024 13:17:35 +0300 Subject: [PATCH 03/15] Fix #29724: `ip` dependency update for CVE-2024-29415 (#29725) ## Summary This version update of `ip` dependency solves the CVE-2024-29415 vulnerability. --- packages/react-devtools/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-devtools/package.json b/packages/react-devtools/package.json index a5f2c1fadcc4d..eddba2b3d207d 100644 --- a/packages/react-devtools/package.json +++ b/packages/react-devtools/package.json @@ -25,7 +25,7 @@ "dependencies": { "cross-spawn": "^5.0.1", "electron": "^23.1.2", - "ip": "^1.1.4", + "ip": "^2.0.1", "minimist": "^1.2.3", "react-devtools-core": "5.2.0", "update-notifier": "^2.1.0" From eb259b5d3b20b053dc0444e6ae442774c396c4a7 Mon Sep 17 00:00:00 2001 From: Dmytro Rykun Date: Wed, 5 Jun 2024 15:07:58 +0100 Subject: [PATCH 04/15] Add enableShallowPropDiffing feature flag (#29664) ## Summary We currently do deep diffing for object props, and also use custom differs, if they are defined, for props with custom attribute config. The idea is to simply do a `===` comparison instead of all that work. We will do less computation on the JS side, but send more data to native. The hypothesis is that this change should be neutral in terms of performance. If that's the case, we'll be able to get rid of custom differs, and be one step closer to deleting view configs. This PR adds the `enableShallowPropDiffing` feature flag to support this experiment. ## How did you test this change? With `enableShallowPropDiffing` hardcoded to `true`: ``` yarn test packages/react-native-renderer ``` This fails on the following test cases: - should use the diff attribute - should do deep diffs of Objects by default - should skip deeply-nested changed functions Which makes sense with this change. These test cases should be deleted if the experiment is shipped. --- .../src/ReactNativeAttributePayloadFabric.js | 8 ++++++-- .../ReactNativeAttributePayloadFabric-test.internal.js | 7 +++++-- packages/shared/ReactFeatureFlags.js | 2 ++ .../shared/forks/ReactFeatureFlags.native-fb-dynamic.js | 1 + packages/shared/forks/ReactFeatureFlags.native-fb.js | 1 + packages/shared/forks/ReactFeatureFlags.native-oss.js | 2 +- packages/shared/forks/ReactFeatureFlags.test-renderer.js | 1 + .../forks/ReactFeatureFlags.test-renderer.native-fb.js | 1 + .../shared/forks/ReactFeatureFlags.test-renderer.www.js | 1 + packages/shared/forks/ReactFeatureFlags.www.js | 1 + 10 files changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js b/packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js index eed17b799e7cc..817c01f187202 100644 --- a/packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js +++ b/packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js @@ -14,7 +14,10 @@ import { } from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface'; import isArray from 'shared/isArray'; -import {enableAddPropertiesFastPath} from 'shared/ReactFeatureFlags'; +import { + enableAddPropertiesFastPath, + enableShallowPropDiffing, +} from 'shared/ReactFeatureFlags'; import type {AttributeConfiguration} from './ReactNativeTypes'; @@ -342,7 +345,7 @@ function diffProperties( // Pattern match on: attributeConfig if (typeof attributeConfig !== 'object') { // case: !Object is the default case - if (defaultDiffer(prevProp, nextProp)) { + if (enableShallowPropDiffing || defaultDiffer(prevProp, nextProp)) { // a normal leaf has changed (updatePayload || (updatePayload = ({}: {[string]: $FlowFixMe})))[ propKey @@ -354,6 +357,7 @@ function diffProperties( ) { // case: CustomAttributeConfiguration const shouldUpdate = + enableShallowPropDiffing || prevProp === undefined || (typeof attributeConfig.diff === 'function' ? attributeConfig.diff(prevProp, nextProp) diff --git a/packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js index 4df4507a93d38..68cf318c6f126 100644 --- a/packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js @@ -10,7 +10,7 @@ const {diff, create} = require('../ReactNativeAttributePayloadFabric'); -describe('ReactNativeAttributePayload.create', () => { +describe('ReactNativeAttributePayloadFabric.create', () => { it('should work with simple example', () => { expect(create({b: 2, c: 3}, {a: true, b: true})).toEqual({ b: 2, @@ -171,7 +171,7 @@ describe('ReactNativeAttributePayload.create', () => { }); }); -describe('ReactNativeAttributePayload.diff', () => { +describe('ReactNativeAttributePayloadFabric.diff', () => { it('should work with simple example', () => { expect(diff({a: 1, c: 3}, {b: 2, c: 3}, {a: true, b: true})).toEqual({ a: null, @@ -201,6 +201,7 @@ describe('ReactNativeAttributePayload.diff', () => { expect(diff({a: 1}, {b: 2}, {})).toEqual(null); }); + // @gate !enableShallowPropDiffing it('should use the diff attribute', () => { const diffA = jest.fn((a, b) => true); const diffB = jest.fn((a, b) => false); @@ -225,6 +226,7 @@ describe('ReactNativeAttributePayload.diff', () => { expect(diffB).not.toBeCalled(); }); + // @gate !enableShallowPropDiffing it('should do deep diffs of Objects by default', () => { expect( diff( @@ -422,6 +424,7 @@ describe('ReactNativeAttributePayload.diff', () => { ).toEqual(null); }); + // @gate !enableShallowPropDiffing it('should skip deeply-nested changed functions', () => { expect( diff( diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index adec53c109352..8b2d0800cb933 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -125,6 +125,8 @@ export const enableAddPropertiesFastPath = false; export const enableOwnerStacks = __EXPERIMENTAL__; +export const enableShallowPropDiffing = false; + /** * Enables an expiration time for retry lanes to avoid starvation. */ diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js index bb8b523e6d3bb..ecdb3755691d2 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js @@ -24,4 +24,5 @@ export const enableAddPropertiesFastPath = __VARIANT__; export const enableDeferRootSchedulingToMicrotask = __VARIANT__; export const enableFastJSX = __VARIANT__; export const enableInfiniteRenderLoopDetection = __VARIANT__; +export const enableShallowPropDiffing = __VARIANT__; export const passChildrenWhenCloningPersistedNodes = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index f5387abb03c41..c306b2a6a28ed 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -26,6 +26,7 @@ export const { enableDeferRootSchedulingToMicrotask, enableFastJSX, enableInfiniteRenderLoopDetection, + enableShallowPropDiffing, passChildrenWhenCloningPersistedNodes, } = dynamicFlags; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index f6820d3bf5803..63fe1885c0bda 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -102,7 +102,7 @@ export const enableDO_NOT_USE_disableStrictPassiveEffect = false; export const passChildrenWhenCloningPersistedNodes = false; export const enableAsyncIterableChildren = false; export const enableAddPropertiesFastPath = false; - +export const enableShallowPropDiffing = false; export const renameElementSymbol = true; export const enableOwnerStacks = __EXPERIMENTAL__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 24d94adaf82ec..e40351ae1fcf4 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -79,6 +79,7 @@ export const enableInfiniteRenderLoopDetection = false; export const enableAddPropertiesFastPath = false; export const renameElementSymbol = true; +export const enableShallowPropDiffing = false; // TODO: This must be in sync with the main ReactFeatureFlags file because // the Test Renderer's value must be the same as the one used by the diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 731aa42147579..fda4ec73af8b9 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -92,6 +92,7 @@ export const enableAddPropertiesFastPath = false; export const renameElementSymbol = false; export const enableOwnerStacks = false; +export const enableShallowPropDiffing = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index babd61677d0fe..8bb8df8736c0b 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -92,6 +92,7 @@ export const enableAddPropertiesFastPath = false; export const renameElementSymbol = false; export const enableOwnerStacks = false; +export const enableShallowPropDiffing = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index c5002a0a9ac82..25064d60e9b2c 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -122,6 +122,7 @@ export const disableStringRefs = false; export const disableLegacyMode = __EXPERIMENTAL__; export const enableOwnerStacks = false; +export const enableShallowPropDiffing = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); From 3730b40e9bbacef0279f6d120b344c1544cb38ba Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Wed, 5 Jun 2024 19:58:12 +0100 Subject: [PATCH 05/15] chore[react-devtools]: ip => internal-ip (#29772) ## Summary There was an attempt to upgrade `ip` to 2.0.1 to mitigate CVE in https://github.com/facebook/react/pull/29725#issuecomment-2150389616, but there actually another one CVE in version `2.0.1`. Instead, migrate to `internal-ip`, which similarly small package that we can use Note: not upgrading to version 7+, because they are pure ESM. ## How did you test this change? Validated that standalone version of RDT works and connects to the app. --- packages/react-devtools/package.json | 2 +- packages/react-devtools/preload.js | 4 +- yarn.lock | 121 ++++++++++----------------- 3 files changed, 46 insertions(+), 81 deletions(-) diff --git a/packages/react-devtools/package.json b/packages/react-devtools/package.json index eddba2b3d207d..cc89dfcf67dd3 100644 --- a/packages/react-devtools/package.json +++ b/packages/react-devtools/package.json @@ -25,7 +25,7 @@ "dependencies": { "cross-spawn": "^5.0.1", "electron": "^23.1.2", - "ip": "^2.0.1", + "internal-ip": "^6.2.0", "minimist": "^1.2.3", "react-devtools-core": "5.2.0", "update-notifier": "^2.1.0" diff --git a/packages/react-devtools/preload.js b/packages/react-devtools/preload.js index d9d2dbd3cdebb..634cffc635a40 100644 --- a/packages/react-devtools/preload.js +++ b/packages/react-devtools/preload.js @@ -1,11 +1,11 @@ const {clipboard, shell, contextBridge} = require('electron'); const fs = require('fs'); -const {address} = require('ip'); +const internalIP = require('internal-ip'); // Expose protected methods so that render process does not need unsafe node integration contextBridge.exposeInMainWorld('api', { electron: {clipboard, shell}, - ip: {address}, + ip: {address: internalIP.v4.sync}, getDevTools() { let devtools; try { diff --git a/yarn.lock b/yarn.lock index 70432d06253a8..a72923e5c175e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6498,7 +6498,7 @@ deepmerge@^4.2.2: resolved "https://registry.yarnpkg.com/deepmerge/-/deepmerge-4.3.1.tgz#44b5f2147cd3b00d4b56137685966f26fd25dd4a" integrity sha512-3sUqbMEc77XqpdNO7FRyRog+eW3ph+GYCbj+rK+uYyRMuwsVy0rMiVtPn+QJlKFvWP/1PYpapqYn0Me2knFn+A== -default-gateway@^6.0.3: +default-gateway@^6.0.0, default-gateway@^6.0.3: version "6.0.3" resolved "https://registry.yarnpkg.com/default-gateway/-/default-gateway-6.0.3.tgz#819494c888053bdb743edbf343d6cdf7f2943a71" integrity sha512-fwSOJsbbNzZ/CUFpqFBqYfYNLj1NbMPm8MMCIzHjC83iSJRBEGmDUxU+WP661BaBQImeC2yHwXtz+P/O9o+XEg== @@ -7226,7 +7226,7 @@ eslint-utils@^2.0.0, eslint-utils@^2.1.0: dependencies: eslint-visitor-keys "^1.1.0" -"eslint-v7@npm:eslint@^7.7.0": +"eslint-v7@npm:eslint@^7.7.0", eslint@^7.7.0: version "7.32.0" resolved "https://registry.yarnpkg.com/eslint/-/eslint-7.32.0.tgz#c6d328a14be3fb08c8d1d21e12c02fdb7a2a812d" integrity sha512-VHZ8gX+EDfz+97jGcgyGCyRia/dPOd6Xh9yPv8Bl1+SoaIwD+a/vlrOmGRUyOYu7MwUhc7CxqeaDZU13S4+EpA== @@ -7389,52 +7389,6 @@ eslint@5.16.0: table "^5.2.3" text-table "^0.2.0" -eslint@^7.7.0: - version "7.32.0" - resolved "https://registry.yarnpkg.com/eslint/-/eslint-7.32.0.tgz#c6d328a14be3fb08c8d1d21e12c02fdb7a2a812d" - integrity sha512-VHZ8gX+EDfz+97jGcgyGCyRia/dPOd6Xh9yPv8Bl1+SoaIwD+a/vlrOmGRUyOYu7MwUhc7CxqeaDZU13S4+EpA== - dependencies: - "@babel/code-frame" "7.12.11" - "@eslint/eslintrc" "^0.4.3" - "@humanwhocodes/config-array" "^0.5.0" - ajv "^6.10.0" - chalk "^4.0.0" - cross-spawn "^7.0.2" - debug "^4.0.1" - doctrine "^3.0.0" - enquirer "^2.3.5" - escape-string-regexp "^4.0.0" - eslint-scope "^5.1.1" - eslint-utils "^2.1.0" - eslint-visitor-keys "^2.0.0" - espree "^7.3.1" - esquery "^1.4.0" - esutils "^2.0.2" - fast-deep-equal "^3.1.3" - file-entry-cache "^6.0.1" - functional-red-black-tree "^1.0.1" - glob-parent "^5.1.2" - globals "^13.6.0" - ignore "^4.0.6" - import-fresh "^3.0.0" - imurmurhash "^0.1.4" - is-glob "^4.0.0" - js-yaml "^3.13.1" - json-stable-stringify-without-jsonify "^1.0.1" - levn "^0.4.1" - lodash.merge "^4.6.2" - minimatch "^3.0.4" - natural-compare "^1.4.0" - optionator "^0.9.1" - progress "^2.0.0" - regexpp "^3.1.0" - semver "^7.2.1" - strip-ansi "^6.0.0" - strip-json-comments "^3.1.0" - table "^6.0.9" - text-table "^0.2.0" - v8-compile-cache "^2.0.3" - espree@6.2.1: version "6.2.1" resolved "https://registry.yarnpkg.com/espree/-/espree-6.2.1.tgz#77fc72e1fd744a2052c20f38a5b575832e82734a" @@ -9489,6 +9443,16 @@ inquirer@^6.2.2: strip-ansi "^5.1.0" through "^2.3.6" +internal-ip@^6.2.0: + version "6.2.0" + resolved "https://registry.yarnpkg.com/internal-ip/-/internal-ip-6.2.0.tgz#d5541e79716e406b74ac6b07b856ef18dc1621c1" + integrity sha512-D8WGsR6yDt8uq7vDMu7mjcR+yRMm3dW8yufyChmszWRjcSHuxLBkR3GdS2HZAjodsaGuCvXeEJpueisXJULghg== + dependencies: + default-gateway "^6.0.0" + ipaddr.js "^1.9.1" + is-ip "^3.1.0" + p-event "^4.2.0" + interpret@^1.0.0: version "1.2.0" resolved "https://registry.yarnpkg.com/interpret/-/interpret-1.2.0.tgz#d5061a6224be58e8083985f5014d844359576296" @@ -9519,12 +9483,17 @@ invert-kv@^3.0.0: resolved "https://registry.yarnpkg.com/invert-kv/-/invert-kv-3.0.1.tgz#a93c7a3d4386a1dc8325b97da9bb1620c0282523" integrity sha512-CYdFeFexxhv/Bcny+Q0BfOV+ltRlJcd4BBZBYFX/O0u4npJrgZtIcjokegtiSMAvlMTJ+Koq0GBCc//3bueQxw== -ip@^1.1.4, ip@^1.1.5: +ip-regex@^4.0.0: + version "4.3.0" + resolved "https://registry.yarnpkg.com/ip-regex/-/ip-regex-4.3.0.tgz#687275ab0f57fa76978ff8f4dddc8a23d5990db5" + integrity sha512-B9ZWJxHHOHUhUjCPrMpLD4xEq35bUTClHM1S6CBU5ixQnkZmwipwgc96vAd7AAGM9TGHvJR+Uss+/Ak6UphK+Q== + +ip@^1.1.5: version "1.1.5" resolved "https://registry.yarnpkg.com/ip/-/ip-1.1.5.tgz#bdded70114290828c0a039e72ef25f5aaec4354a" integrity sha1-vd7XARQpCCjAoDnnLvJfWq7ENUo= -ipaddr.js@1.9.1: +ipaddr.js@1.9.1, ipaddr.js@^1.9.1: version "1.9.1" resolved "https://registry.yarnpkg.com/ipaddr.js/-/ipaddr.js-1.9.1.tgz#bff38543eeb8984825079ff3a2a8e6cbd46781b3" integrity sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g== @@ -9778,6 +9747,13 @@ is-installed-globally@^0.3.1: global-dirs "^2.0.1" is-path-inside "^3.0.1" +is-ip@^3.1.0: + version "3.1.0" + resolved "https://registry.yarnpkg.com/is-ip/-/is-ip-3.1.0.tgz#2ae5ddfafaf05cb8008a62093cf29734f657c5d8" + integrity sha512-35vd5necO7IitFPjd/YBeqwWnyDWbuLH9ZXQdMfDA8TEo7pv5X8yfrvVO3xbJbLUlERCMvf6X0hTUamQxCYJ9Q== + dependencies: + ip-regex "^4.0.0" + is-jpg@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/is-jpg/-/is-jpg-2.0.0.tgz#2e1997fa6e9166eaac0242daae443403e4ef1d97" @@ -12389,6 +12365,13 @@ p-event@^2.1.0: dependencies: p-timeout "^2.0.1" +p-event@^4.2.0: + version "4.2.0" + resolved "https://registry.yarnpkg.com/p-event/-/p-event-4.2.0.tgz#af4b049c8acd91ae81083ebd1e6f5cae2044c1b5" + integrity sha512-KXatOjCRXXkSePPb1Nbi0p0m+gQAwdlbhi4wQKJPI1HsMQS9g+Sqp2o+QHziPr7eYJyOZet836KoHEVM1mwOrQ== + dependencies: + p-timeout "^3.1.0" + p-finally@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/p-finally/-/p-finally-1.0.0.tgz#3fbcfb15b899a44123b34b6dcc18b724336a2cae" @@ -12485,6 +12468,13 @@ p-timeout@^2.0.1: dependencies: p-finally "^1.0.0" +p-timeout@^3.1.0: + version "3.2.0" + resolved "https://registry.yarnpkg.com/p-timeout/-/p-timeout-3.2.0.tgz#c7e17abc971d2a7962ef83626b35d635acf23dfe" + integrity sha512-rhIwUycgwwKcP9yTOOFK/AKsAopjjCakVqLHePO3CC6Mir1Z99xT+R63jZxAT5lFZLa2inS5h+ZS2GvR99/FBg== + dependencies: + p-finally "^1.0.0" + p-try@^2.0.0: version "2.2.0" resolved "https://registry.yarnpkg.com/p-try/-/p-try-2.2.0.tgz#cb2868540e313d61de58fafbe35ce9004d5540e6" @@ -14987,7 +14977,7 @@ string-natural-compare@^3.0.1: resolved "https://registry.yarnpkg.com/string-natural-compare/-/string-natural-compare-3.0.1.tgz#7a42d58474454963759e8e8b7ae63d71c1e7fdf4" integrity sha512-n3sPwynL1nwKi3WJ6AIsClwBMa0zTi54fn2oLU6ndfTSIO05xaznjSf15PcBZU6FNWbmN5Q6cxT4V5hGvB4taw== -"string-width-cjs@npm:string-width@^4.2.0": +"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -15022,15 +15012,6 @@ string-width@^4.0.0: is-fullwidth-code-point "^3.0.0" strip-ansi "^6.0.0" -string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - string-width@^5.0.1, string-width@^5.1.2: version "5.1.2" resolved "https://registry.yarnpkg.com/string-width/-/string-width-5.1.2.tgz#14f8daec6d81e7221d2a357e668cab73bdbca794" @@ -15091,7 +15072,7 @@ string_decoder@~1.1.1: dependencies: safe-buffer "~5.1.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -15119,13 +15100,6 @@ strip-ansi@^5.1.0: dependencies: ansi-regex "^4.1.0" -strip-ansi@^6.0.0, strip-ansi@^6.0.1: - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - strip-ansi@^7.0.1: version "7.1.0" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.0.tgz#d5b6568ca689d8561370b0707685d22434faff45" @@ -16573,7 +16547,7 @@ workerize-loader@^2.0.2: dependencies: loader-utils "^2.0.0" -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -16591,15 +16565,6 @@ wrap-ansi@^6.2.0: string-width "^4.1.0" strip-ansi "^6.0.0" -wrap-ansi@^7.0.0: - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214" From 704aeed022f4277cd5604bf6d76199a6cfe4707f Mon Sep 17 00:00:00 2001 From: XiaoPi <530257315@qq.com> Date: Thu, 6 Jun 2024 07:51:09 +0800 Subject: [PATCH 06/15] feat: consider that the dispatch function from `useReducer` is non-reactive (#29705) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary The dispatch function from useReducer is stable, so it is also non-reactive. the related PR: #29665 the related comment: #29674 (comment) I am not sure if the location of the new test file is appropriate😅. How did you test this change? Added the specific test compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md. --- .../src/HIR/Globals.ts | 13 +++++ .../src/HIR/HIR.ts | 13 +++++ .../src/HIR/ObjectShape.ts | 22 +++++++ .../src/Inference/InferReactivePlaces.ts | 6 +- .../src/Inference/InferReferenceEffects.ts | 2 + .../PruneNonReactiveDependencies.ts | 6 +- .../error.modify-useReducer-state.expect.md | 28 +++++++++ .../compiler/error.modify-useReducer-state.js | 7 +++ ...urned-dispatcher-is-non-reactive.expect.md | 57 +++++++++++++++++++ ...cer-returned-dispatcher-is-non-reactive.js | 17 ++++++ 10 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts index 931d315d308e0..041d2fbf00911 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts @@ -13,6 +13,7 @@ import { BuiltInUseInsertionEffectHookId, BuiltInUseLayoutEffectHookId, BuiltInUseOperatorId, + BuiltInUseReducerId, BuiltInUseRefId, BuiltInUseStateId, ShapeRegistry, @@ -265,6 +266,18 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ returnValueReason: ValueReason.State, }), ], + [ + "useReducer", + addHook(DEFAULT_SHAPES, { + positionalParams: [], + restParam: Effect.Freeze, + returnType: { kind: "Object", shapeId: BuiltInUseReducerId }, + calleeEffect: Effect.Read, + hookKind: "useReducer", + returnValueKind: ValueKind.Frozen, + returnValueReason: ValueReason.ReducerState, + }), + ], [ "useRef", addHook(DEFAULT_SHAPES, { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index f9dfea52f363e..afa0799b40d26 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -1254,6 +1254,11 @@ export enum ValueReason { */ State = "state", + /** + * A value returned from `useReducer` + */ + ReducerState = "reducer-state", + /** * Props of a component or arguments of a hook. */ @@ -1493,6 +1498,14 @@ export function isSetStateType(id: Identifier): boolean { return id.type.kind === "Function" && id.type.shapeId === "BuiltInSetState"; } +export function isUseReducerType(id: Identifier): boolean { + return id.type.kind === "Function" && id.type.shapeId === "BuiltInUseReducer"; +} + +export function isDispatcherType(id: Identifier): boolean { + return id.type.kind === "Function" && id.type.shapeId === "BuiltInDispatch"; +} + export function isUseEffectHookType(id: Identifier): boolean { return ( id.type.kind === "Function" && id.type.shapeId === "BuiltInUseEffectHook" diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts index fd04bf43c2950..8997ad086f5a2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -118,6 +118,7 @@ function addShape( export type HookKind = | "useContext" | "useState" + | "useReducer" | "useRef" | "useEffect" | "useLayoutEffect" @@ -200,6 +201,8 @@ export const BuiltInUseEffectHookId = "BuiltInUseEffectHook"; export const BuiltInUseLayoutEffectHookId = "BuiltInUseLayoutEffectHook"; export const BuiltInUseInsertionEffectHookId = "BuiltInUseInsertionEffectHook"; export const BuiltInUseOperatorId = "BuiltInUseOperator"; +export const BuiltInUseReducerId = "BuiltInUseReducer"; +export const BuiltInDispatchId = "BuiltInDispatch"; // ShapeRegistry with default definitions for built-ins. export const BUILTIN_SHAPES: ShapeRegistry = new Map(); @@ -387,6 +390,25 @@ addObject(BUILTIN_SHAPES, BuiltInUseStateId, [ ], ]); +addObject(BUILTIN_SHAPES, BuiltInUseReducerId, [ + ["0", { kind: "Poly" }], + [ + "1", + addFunction( + BUILTIN_SHAPES, + [], + { + positionalParams: [], + restParam: Effect.Freeze, + returnType: PRIMITIVE_TYPE, + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Primitive, + }, + BuiltInDispatchId + ), + ], +]); + addObject(BUILTIN_SHAPES, BuiltInUseRefId, [ ["current", { kind: "Object", shapeId: BuiltInRefValueId }], ]); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts index ad2f666ac16d1..e6a7bb49ce132 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts @@ -15,6 +15,7 @@ import { Place, computePostDominatorTree, getHookKind, + isDispatcherType, isSetStateType, isUseOperator, } from "../HIR"; @@ -219,7 +220,10 @@ export function inferReactivePlaces(fn: HIRFunction): void { if (hasReactiveInput) { for (const lvalue of eachInstructionLValue(instruction)) { - if (isSetStateType(lvalue.identifier)) { + if ( + isSetStateType(lvalue.identifier) || + isDispatcherType(lvalue.identifier) + ) { continue; } reactiveIdentifiers.markReactive(lvalue); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts index 520684c026bda..387dafb6e5a1f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts @@ -2117,6 +2117,8 @@ function getWriteErrorReason(abstractValue: AbstractValue): string { return "Mutating component props or hook arguments is not allowed. Consider using a local variable instead"; } else if (abstractValue.reason.has(ValueReason.State)) { return "Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead"; + } else if (abstractValue.reason.has(ValueReason.ReducerState)) { + return "Mutating a value returned from 'useReducer()', which should not be mutated. Use the dispatch function to update instead"; } else { return "This mutates a variable that React considers immutable"; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts index 0c82cefc59f06..aef5d50ee3a06 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts @@ -10,6 +10,7 @@ import { ReactiveFunction, ReactiveInstruction, ReactiveScopeBlock, + isDispatcherType, isSetStateType, } from "../HIR"; import { eachPatternOperand } from "../HIR/visitors"; @@ -56,7 +57,10 @@ class Visitor extends ReactiveFunctionVisitor { case "Destructure": { if (state.has(value.value.identifier.id)) { for (const lvalue of eachPatternOperand(value.lvalue.pattern)) { - if (isSetStateType(lvalue.identifier)) { + if ( + isSetStateType(lvalue.identifier) || + isDispatcherType(lvalue.identifier) + ) { continue; } state.add(lvalue.identifier.id); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.expect.md new file mode 100644 index 0000000000000..22bdff08d8731 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.expect.md @@ -0,0 +1,28 @@ + +## Input + +```javascript +import { useReducer } from "react"; + +function Foo() { + let [state, setState] = useReducer({ foo: 1 }); + state.foo = 1; + return state; +} + +``` + + +## Error + +``` + 3 | function Foo() { + 4 | let [state, setState] = useReducer({ foo: 1 }); +> 5 | state.foo = 1; + | ^^^^^ InvalidReact: Mutating a value returned from 'useReducer()', which should not be mutated. Use the dispatch function to update instead (5:5) + 6 | return state; + 7 | } + 8 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.js new file mode 100644 index 0000000000000..42a04fc8da3d2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.modify-useReducer-state.js @@ -0,0 +1,7 @@ +import { useReducer } from "react"; + +function Foo() { + let [state, setState] = useReducer({ foo: 1 }); + state.foo = 1; + return state; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md new file mode 100644 index 0000000000000..32c0836647cbf --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md @@ -0,0 +1,57 @@ + +## Input + +```javascript +import { useReducer } from "react"; + +function f() { + const [state, dispatch] = useReducer(); + + const onClick = () => { + dispatch(); + }; + + return
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: f, + params: [], + isComponent: true, +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useReducer } from "react"; + +function f() { + const $ = _c(1); + const [state, dispatch] = useReducer(); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const onClick = () => { + dispatch(); + }; + + t0 =
; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: f, + params: [], + isComponent: true, +}; + +``` + +### Eval output +(kind: ok)
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.js new file mode 100644 index 0000000000000..c1dec4e5a7f00 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.js @@ -0,0 +1,17 @@ +import { useReducer } from "react"; + +function f() { + const [state, dispatch] = useReducer(); + + const onClick = () => { + dispatch(); + }; + + return
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: f, + params: [], + isComponent: true, +}; From 99da76f23ac85d279457470f8fb19a9b2f173ed0 Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Thu, 6 Jun 2024 17:10:40 +0100 Subject: [PATCH 07/15] fix[react-devtools] remove native inspection button when it can't be used (#29779) ## Summary There's no native inspection available in any of the React-Native devtools: * **React DevTools in Fusebox** * **React DevTools standalone** Besides, **React DevTools Inline** can't really open the devtools and point to the native inspector because of lack of an API to do that. Only **React DevTools extension** can actually do that. That's why I've disabled it for the first 3 flavours of React DevTools mentioned above. ## How did you test this change? Still enabled on **React DevTools extension** Screenshot 2024-06-06 at 16 09 21 Disabled on **React DevTools in Fusebox** Screenshot 2024-06-06 at 16 04 28 Disabled on **React DevTools standalone** Screenshot 2024-06-06 at 16 15 08 Disabled on **React DevTools Inline** Screenshot 2024-06-06 at 16 09 26 --- packages/react-devtools-core/src/standalone.js | 2 +- packages/react-devtools-fusebox/src/frontend.js | 2 +- packages/react-devtools-inline/src/frontend.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-devtools-core/src/standalone.js b/packages/react-devtools-core/src/standalone.js index 6829c27895d93..7eb246a28a316 100644 --- a/packages/react-devtools-core/src/standalone.js +++ b/packages/react-devtools-core/src/standalone.js @@ -279,7 +279,7 @@ function initialize(socket: WebSocket) { // $FlowFixMe[incompatible-call] found when upgrading Flow store = new Store(bridge, { checkBridgeProtocolCompatibility: true, - supportsNativeInspection: true, + supportsNativeInspection: false, supportsTraceUpdates: true, }); diff --git a/packages/react-devtools-fusebox/src/frontend.js b/packages/react-devtools-fusebox/src/frontend.js index ca236031ddf41..68f5560bd9f98 100644 --- a/packages/react-devtools-fusebox/src/frontend.js +++ b/packages/react-devtools-fusebox/src/frontend.js @@ -37,7 +37,7 @@ export function createStore(bridge: FrontendBridge, config?: Config): Store { return new Store(bridge, { checkBridgeProtocolCompatibility: true, supportsTraceUpdates: true, - supportsNativeInspection: true, + supportsNativeInspection: false, ...config, }); } diff --git a/packages/react-devtools-inline/src/frontend.js b/packages/react-devtools-inline/src/frontend.js index d0e0fbfcccd8a..35897b9407e59 100644 --- a/packages/react-devtools-inline/src/frontend.js +++ b/packages/react-devtools-inline/src/frontend.js @@ -23,7 +23,7 @@ export function createStore(bridge: FrontendBridge, config?: Config): Store { checkBridgeProtocolCompatibility: true, supportsTraceUpdates: true, supportsTimeline: true, - supportsNativeInspection: true, + supportsNativeInspection: false, ...config, }); } From fd6e130b00d4d1fe211c75e981160131669c4412 Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Thu, 6 Jun 2024 17:48:44 +0100 Subject: [PATCH 08/15] Default native inspections config false (#29784) ## Summary To make the config `supportsNativeInspection` explicit, set it to default to `false` and only allow it in the extension. ## How did you test this change? When disabled on **React DevTools extension** Screenshot 2024-06-06 at 17 34 02 When enabled on **React DevTools extension** (the chosen config) Screenshot 2024-06-06 at 17 34 53 When enabled on **React DevTools in Fusebox** Screenshot 2024-06-06 at 17 29 24 When disabled on **React DevTools in Fusebox** (the chosen config) Screenshot 2024-06-06 at 17 30 31 When enabled on **React DevTools Inline** Screenshot 2024-06-06 at 17 24 20 When disabled on **React DevTools Inline** (the chosen config) Screenshot 2024-06-06 at 17 19 39 When enabled on **React DevTools standalone** Screenshot 2024-06-06 at 17 23 16 When disabled on **React DevTools standalone** (the chosen config) Screenshot 2024-06-06 at 17 19 39 --- packages/react-devtools-core/src/standalone.js | 1 - packages/react-devtools-extensions/src/main/index.js | 1 + packages/react-devtools-fusebox/src/frontend.js | 1 - packages/react-devtools-inline/src/frontend.js | 1 - packages/react-devtools-shared/src/devtools/store.js | 6 ++++-- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react-devtools-core/src/standalone.js b/packages/react-devtools-core/src/standalone.js index 7eb246a28a316..e4e4ada1c31a9 100644 --- a/packages/react-devtools-core/src/standalone.js +++ b/packages/react-devtools-core/src/standalone.js @@ -279,7 +279,6 @@ function initialize(socket: WebSocket) { // $FlowFixMe[incompatible-call] found when upgrading Flow store = new Store(bridge, { checkBridgeProtocolCompatibility: true, - supportsNativeInspection: false, supportsTraceUpdates: true, }); diff --git a/packages/react-devtools-extensions/src/main/index.js b/packages/react-devtools-extensions/src/main/index.js index 224e4cd4b4a8a..e1db3d505577b 100644 --- a/packages/react-devtools-extensions/src/main/index.js +++ b/packages/react-devtools-extensions/src/main/index.js @@ -97,6 +97,7 @@ function createBridgeAndStore() { // At this time, the timeline can only parse Chrome performance profiles. supportsTimeline: __IS_CHROME__, supportsTraceUpdates: true, + supportsNativeInspection: true, }); if (!isProfiling) { diff --git a/packages/react-devtools-fusebox/src/frontend.js b/packages/react-devtools-fusebox/src/frontend.js index 68f5560bd9f98..976b8693d373e 100644 --- a/packages/react-devtools-fusebox/src/frontend.js +++ b/packages/react-devtools-fusebox/src/frontend.js @@ -37,7 +37,6 @@ export function createStore(bridge: FrontendBridge, config?: Config): Store { return new Store(bridge, { checkBridgeProtocolCompatibility: true, supportsTraceUpdates: true, - supportsNativeInspection: false, ...config, }); } diff --git a/packages/react-devtools-inline/src/frontend.js b/packages/react-devtools-inline/src/frontend.js index 35897b9407e59..9031f6ffc7bd7 100644 --- a/packages/react-devtools-inline/src/frontend.js +++ b/packages/react-devtools-inline/src/frontend.js @@ -23,7 +23,6 @@ export function createStore(bridge: FrontendBridge, config?: Config): Store { checkBridgeProtocolCompatibility: true, supportsTraceUpdates: true, supportsTimeline: true, - supportsNativeInspection: false, ...config, }); } diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 3eb589b903dac..408151dcdbbaf 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -172,7 +172,7 @@ export default class Store extends EventEmitter<{ _rootIDToRendererID: Map = new Map(); // These options may be initially set by a configuration option when constructing the Store. - _supportsNativeInspection: boolean = true; + _supportsNativeInspection: boolean = false; _supportsReloadAndProfile: boolean = false; _supportsTimeline: boolean = false; _supportsTraceUpdates: boolean = false; @@ -216,7 +216,9 @@ export default class Store extends EventEmitter<{ supportsTimeline, supportsTraceUpdates, } = config; - this._supportsNativeInspection = supportsNativeInspection !== false; + if (supportsNativeInspection) { + this._supportsNativeInspection = true; + } if (supportsReloadAndProfile) { this._supportsReloadAndProfile = true; } From b526a0a419029eea31f4d967951b6feca123012d Mon Sep 17 00:00:00 2001 From: Josh Story Date: Thu, 6 Jun 2024 10:07:24 -0700 Subject: [PATCH 09/15] [Flight][Fizz] schedule work async (#29551) While most builds of Flight and Fizz schedule work in new tasks some do execute work synchronously. While this is necessary for legacy APIs like renderToString for modern APIs there really isn't a great reason to do this synchronously. We could schedule works as microtasks but we actually want to yield so the runtime can run events and other things that will unblock additional work before starting the next work loop. This change updates all non-legacy uses to be async using the best availalble macrotask scheduler. Browser now uses postMessage Bun uses setTimeout because while it also supports setImmediate the scheduling is not as eager as the same API in node the FB build also uses setTimeout This change required a number of changes to tests which were utilizing the sync nature of work in the Browser builds to avoid having to manage timers and tasks. I added a patch to install MessageChannel which is required by the browser builds and made this patched version integrate with the Scheduler mock. This way we can effectively use `act` to flush flight and fizz work similar to how we do this on the client. --- ...ctClassComponentPropResolutionFizz-test.js | 26 +- .../ReactDOMFizzDeferredValue-test.js | 30 +- .../src/__tests__/ReactDOMFizzForm-test.js | 72 ++- .../ReactDOMFizzServerBrowser-test.js | 339 +++++++----- .../ReactDOMFizzStaticBrowser-test.js | 513 ++++++++++------- .../__tests__/ReactDOMFizzStaticFloat-test.js | 59 +- .../__tests__/ReactFlightTurbopackDOM-test.js | 41 +- .../ReactFlightTurbopackDOMBrowser-test.js | 23 +- .../ReactFlightTurbopackDOMNode-test.js | 30 +- .../ReactFlightTurbopackDOMReply-test.js | 7 + .../src/__tests__/ReactFlightDOM-test.js | 364 +++++++----- .../__tests__/ReactFlightDOMBrowser-test.js | 521 +++++++++++------- .../src/__tests__/ReactFlightDOMNode-test.js | 87 +-- .../src/__tests__/ReactFlightDOMReply-test.js | 44 +- .../react-server/src/ReactFlightServer.js | 9 +- .../src/ReactServerStreamConfigBrowser.js | 12 +- .../src/ReactServerStreamConfigBun.js | 2 +- ...tServerStreamConfig.dom-fb-experimental.js | 16 +- .../__tests__/ReactMismatchedVersions-test.js | 5 + scripts/jest/patchMessageChannel.js | 30 + scripts/jest/patchSetImmediate.js | 13 + scripts/jest/setupEnvironment.js | 13 - 22 files changed, 1419 insertions(+), 837 deletions(-) create mode 100644 scripts/jest/patchMessageChannel.js create mode 100644 scripts/jest/patchSetImmediate.js diff --git a/packages/react-dom/src/__tests__/ReactClassComponentPropResolutionFizz-test.js b/packages/react-dom/src/__tests__/ReactClassComponentPropResolutionFizz-test.js index 653797ec44b00..67e7fff249855 100644 --- a/packages/react-dom/src/__tests__/ReactClassComponentPropResolutionFizz-test.js +++ b/packages/react-dom/src/__tests__/ReactClassComponentPropResolutionFizz-test.js @@ -10,6 +10,7 @@ 'use strict'; import {insertNodesAndExecuteScripts} from '../test-utils/FizzTestUtils'; +import {patchMessageChannel} from '../../../../scripts/jest/patchMessageChannel'; // Polyfills for test environment global.ReadableStream = @@ -21,12 +22,16 @@ let ReactDOMServer; let Scheduler; let assertLog; let container; +let act; describe('ReactClassComponentPropResolutionFizz', () => { beforeEach(() => { jest.resetModules(); - React = require('react'); Scheduler = require('scheduler'); + patchMessageChannel(Scheduler); + act = require('internal-test-utils').act; + + React = require('react'); ReactDOMServer = require('react-dom/server.browser'); assertLog = require('internal-test-utils').assertLog; container = document.createElement('div'); @@ -37,6 +42,17 @@ describe('ReactClassComponentPropResolutionFizz', () => { document.body.removeChild(container); }); + async function serverAct(callback) { + let maybePromise; + await act(() => { + maybePromise = callback(); + if (maybePromise && typeof maybePromise.catch === 'function') { + maybePromise.catch(() => {}); + } + }); + return maybePromise; + } + async function readIntoContainer(stream) { const reader = stream.getReader(); let result = ''; @@ -57,7 +73,7 @@ describe('ReactClassComponentPropResolutionFizz', () => { return text; } - test('resolves ref and default props before calling lifecycle methods', async () => { + it('resolves ref and default props before calling lifecycle methods', async () => { function getPropKeys(props) { return Object.keys(props).join(', '); } @@ -80,11 +96,13 @@ describe('ReactClassComponentPropResolutionFizz', () => { }; // `ref` should never appear as a prop. `default` always should. + const ref = React.createRef(); - const stream = await ReactDOMServer.renderToReadableStream( - , + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), ); await readIntoContainer(stream); + assertLog([ 'constructor: text, default', 'componentWillMount: text, default', diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzDeferredValue-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzDeferredValue-test.js index fbfb00df87a1d..04e60648fb2e6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzDeferredValue-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzDeferredValue-test.js @@ -13,6 +13,7 @@ import { insertNodesAndExecuteScripts, getVisibleChildren, } from '../test-utils/FizzTestUtils'; +import {patchMessageChannel} from '../../../../scripts/jest/patchMessageChannel'; // Polyfills for test environment global.ReadableStream = @@ -33,13 +34,14 @@ let Suspense; describe('ReactDOMFizzForm', () => { beforeEach(() => { jest.resetModules(); - React = require('react'); Scheduler = require('scheduler'); + patchMessageChannel(Scheduler); + act = require('internal-test-utils').act; + React = require('react'); ReactDOMServer = require('react-dom/server.browser'); ReactDOMClient = require('react-dom/client'); useDeferredValue = React.useDeferredValue; Suspense = React.Suspense; - act = require('internal-test-utils').act; assertLog = require('internal-test-utils').assertLog; waitForPaint = require('internal-test-utils').waitForPaint; container = document.createElement('div'); @@ -50,6 +52,17 @@ describe('ReactDOMFizzForm', () => { document.body.removeChild(container); }); + async function serverAct(callback) { + let maybePromise; + await act(() => { + maybePromise = callback(); + if (maybePromise && typeof maybePromise.catch === 'function') { + maybePromise.catch(() => {}); + } + }); + return maybePromise; + } + async function readIntoContainer(stream) { const reader = stream.getReader(); let result = ''; @@ -76,7 +89,9 @@ describe('ReactDOMFizzForm', () => { return useDeferredValue('Final', 'Initial'); } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); expect(container.textContent).toEqual('Initial'); @@ -107,7 +122,9 @@ describe('ReactDOMFizzForm', () => { ); } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); expect(container.textContent).toEqual('Loading...'); @@ -153,8 +170,9 @@ describe('ReactDOMFizzForm', () => { const cRef = React.createRef(); - // The server renders using the "initial" value for B. - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); assertLog(['A', 'B [Initial]', 'C']); expect(getVisibleChildren(container)).toEqual( diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js index f578748e923d2..b83abb5693bcb 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js @@ -10,6 +10,7 @@ 'use strict'; import {insertNodesAndExecuteScripts} from '../test-utils/FizzTestUtils'; +import {patchMessageChannel} from '../../../../scripts/jest/patchMessageChannel'; // Polyfills for test environment global.ReadableStream = @@ -24,10 +25,13 @@ let ReactDOMClient; let useFormStatus; let useOptimistic; let useActionState; +let Scheduler; describe('ReactDOMFizzForm', () => { beforeEach(() => { jest.resetModules(); + Scheduler = require('scheduler'); + patchMessageChannel(Scheduler); React = require('react'); ReactDOMServer = require('react-dom/server.browser'); ReactDOMClient = require('react-dom/client'); @@ -48,6 +52,14 @@ describe('ReactDOMFizzForm', () => { document.body.removeChild(container); }); + async function serverAct(callback) { + let maybePromise; + await act(() => { + maybePromise = callback(); + }); + return maybePromise; + } + function submit(submitter) { const form = submitter.form || submitter; if (!submitter.form) { @@ -96,7 +108,9 @@ describe('ReactDOMFizzForm', () => { ); } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); await act(async () => { ReactDOMClient.hydrateRoot(container, ); @@ -143,7 +157,9 @@ describe('ReactDOMFizzForm', () => { ); } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); await act(async () => { ReactDOMClient.hydrateRoot(container, ); @@ -175,7 +191,9 @@ describe('ReactDOMFizzForm', () => { ); } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); await expect(async () => { await act(async () => { @@ -197,7 +215,9 @@ describe('ReactDOMFizzForm', () => { ); } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); // This should ideally warn because only the client provides a function that doesn't line up. await act(async () => { @@ -231,7 +251,9 @@ describe('ReactDOMFizzForm', () => { ); } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); let root; await act(async () => { @@ -278,7 +300,9 @@ describe('ReactDOMFizzForm', () => { ); } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); let root; await act(async () => { @@ -334,7 +358,9 @@ describe('ReactDOMFizzForm', () => { // Specifying the extra form fields are a DEV error, but we expect it // to eventually still be patched up after an update. await expect(async () => { - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); }).toErrorDev([ 'Cannot specify a encType or method for a form that specifies a function as the action.', @@ -379,7 +405,9 @@ describe('ReactDOMFizzForm', () => { return 'Pending: ' + pending; } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); expect(container.textContent).toBe('Pending: false'); @@ -400,7 +428,9 @@ describe('ReactDOMFizzForm', () => { ); } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); // Dispatch an event before hydration @@ -441,7 +471,9 @@ describe('ReactDOMFizzForm', () => { ); } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); submit(container.getElementsByTagName('input')[1]); @@ -463,7 +495,9 @@ describe('ReactDOMFizzForm', () => { return optimisticState; } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); expect(container.textContent).toBe('hi'); @@ -484,7 +518,9 @@ describe('ReactDOMFizzForm', () => { return state; } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); expect(container.textContent).toBe('0'); @@ -521,7 +557,9 @@ describe('ReactDOMFizzForm', () => { ); } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); const form = container.firstChild; @@ -581,7 +619,9 @@ describe('ReactDOMFizzForm', () => { ); } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); const input = container.getElementsByTagName('input')[1]; @@ -651,7 +691,9 @@ describe('ReactDOMFizzForm', () => { ); } - const stream = await ReactDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactDOMServer.renderToReadableStream(), + ); await readIntoContainer(stream); const barField = container.querySelector('[name=bar]'); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index f6ac8739f04e8..cfeade2ff614a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -9,6 +9,8 @@ 'use strict'; +import {patchMessageChannel} from '../../../../scripts/jest/patchMessageChannel'; + // Polyfills for test environment global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStream; @@ -17,15 +19,33 @@ global.TextEncoder = require('util').TextEncoder; let React; let ReactDOMFizzServer; let Suspense; +let Scheduler; +let act; describe('ReactDOMFizzServerBrowser', () => { beforeEach(() => { jest.resetModules(); + + Scheduler = require('scheduler'); + patchMessageChannel(Scheduler); + act = require('internal-test-utils').act; + React = require('react'); ReactDOMFizzServer = require('react-dom/server.browser'); Suspense = React.Suspense; }); + async function serverAct(callback) { + let maybePromise; + await act(() => { + maybePromise = callback(); + if (maybePromise && typeof maybePromise.catch === 'function') { + maybePromise.catch(() => {}); + } + }); + return maybePromise; + } + const theError = new Error('This is an error'); function Throw() { throw theError; @@ -48,18 +68,20 @@ describe('ReactDOMFizzServerBrowser', () => { } it('should call renderToReadableStream', async () => { - const stream = await ReactDOMFizzServer.renderToReadableStream( -
hello world
, + const stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream(
hello world
), ); const result = await readResult(stream); expect(result).toMatchInlineSnapshot(`"
hello world
"`); }); it('should emit DOCTYPE at the root of the document', async () => { - const stream = await ReactDOMFizzServer.renderToReadableStream( - - hello world - , + const stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream( + + hello world + , + ), ); const result = await readResult(stream); expect(result).toMatchInlineSnapshot( @@ -68,13 +90,12 @@ describe('ReactDOMFizzServerBrowser', () => { }); it('should emit bootstrap script src at the end', async () => { - const stream = await ReactDOMFizzServer.renderToReadableStream( -
hello world
, - { + const stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream(
hello world
, { bootstrapScriptContent: 'INIT();', bootstrapScripts: ['init.js'], bootstrapModules: ['init.mjs'], - }, + }), ); const result = await readResult(stream); expect(result).toMatchInlineSnapshot( @@ -93,23 +114,22 @@ describe('ReactDOMFizzServerBrowser', () => { return 'Done'; } let isComplete = false; - const stream = await ReactDOMFizzServer.renderToReadableStream( -
- - - -
, + const stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream( +
+ + + +
, + ), ); stream.allReady.then(() => (isComplete = true)); - await jest.runAllTimers(); expect(isComplete).toBe(false); // Resolve the loading. hasLoaded = true; - await resolve(); - - await jest.runAllTimers(); + await serverAct(() => resolve()); expect(isComplete).toBe(true); @@ -123,15 +143,17 @@ describe('ReactDOMFizzServerBrowser', () => { const reportedErrors = []; let caughtError = null; try { - await ReactDOMFizzServer.renderToReadableStream( -
- -
, - { - onError(x) { - reportedErrors.push(x); + await serverAct(() => + ReactDOMFizzServer.renderToReadableStream( +
+ +
, + { + onError(x) { + reportedErrors.push(x); + }, }, - }, + ), ); } catch (error) { caughtError = error; @@ -144,17 +166,19 @@ describe('ReactDOMFizzServerBrowser', () => { const reportedErrors = []; let caughtError = null; try { - await ReactDOMFizzServer.renderToReadableStream( -
- }> - - -
, - { - onError(x) { - reportedErrors.push(x); + await serverAct(() => + ReactDOMFizzServer.renderToReadableStream( +
+ }> + + +
, + { + onError(x) { + reportedErrors.push(x); + }, }, - }, + ), ); } catch (error) { caughtError = error; @@ -165,17 +189,19 @@ describe('ReactDOMFizzServerBrowser', () => { it('should not error the stream when an error is thrown inside suspense boundary', async () => { const reportedErrors = []; - const stream = await ReactDOMFizzServer.renderToReadableStream( -
- Loading
}> - - -
, - { - onError(x) { - reportedErrors.push(x); + const stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream( +
+ Loading
}> + + +
, + { + onError(x) { + reportedErrors.push(x); + }, }, - }, + ), ); const result = await readResult(stream); @@ -186,18 +212,20 @@ describe('ReactDOMFizzServerBrowser', () => { it('should be able to complete by aborting even if the promise never resolves', async () => { const errors = []; const controller = new AbortController(); - const stream = await ReactDOMFizzServer.renderToReadableStream( -
- Loading
}> - - -
, - { - signal: controller.signal, - onError(x) { - errors.push(x.message); + const stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream( +
+ Loading
}> + + + , + { + signal: controller.signal, + onError(x) { + errors.push(x.message); + }, }, - }, + ), ); controller.abort(); @@ -211,20 +239,20 @@ describe('ReactDOMFizzServerBrowser', () => { it('should reject if aborting before the shell is complete', async () => { const errors = []; const controller = new AbortController(); - const promise = ReactDOMFizzServer.renderToReadableStream( -
- -
, - { - signal: controller.signal, - onError(x) { - errors.push(x.message); + const promise = serverAct(() => + ReactDOMFizzServer.renderToReadableStream( +
+ +
, + { + signal: controller.signal, + onError(x) { + errors.push(x.message); + }, }, - }, + ), ); - await jest.runAllTimers(); - const theReason = new Error('aborted for reasons'); controller.abort(theReason); @@ -249,16 +277,18 @@ describe('ReactDOMFizzServerBrowser', () => { ); } - const streamPromise = ReactDOMFizzServer.renderToReadableStream( -
- -
, - { - signal: controller.signal, - onError(x) { - errors.push(x.message); + const streamPromise = serverAct(() => + ReactDOMFizzServer.renderToReadableStream( +
+ +
, + { + signal: controller.signal, + onError(x) { + errors.push(x.message); + }, }, - }, + ), ); let caughtError = null; @@ -277,18 +307,20 @@ describe('ReactDOMFizzServerBrowser', () => { const theReason = new Error('aborted for reasons'); controller.abort(theReason); - const promise = ReactDOMFizzServer.renderToReadableStream( -
- Loading
}> - - - , - { - signal: controller.signal, - onError(x) { - errors.push(x.message); + const promise = serverAct(() => + ReactDOMFizzServer.renderToReadableStream( +
+ Loading
}> + + + , + { + signal: controller.signal, + onError(x) { + errors.push(x.message); + }, }, - }, + ), ); // Technically we could still continue rendering the shell but currently the @@ -317,17 +349,19 @@ describe('ReactDOMFizzServerBrowser', () => { return 'Done'; } const errors = []; - const stream = await ReactDOMFizzServer.renderToReadableStream( -
- Loading
}> - - - , - { - onError(x) { - errors.push(x.message); + const stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream( +
+ Loading
}> + + + , + { + onError(x) { + errors.push(x.message); + }, }, - }, + ), ); stream.allReady.then(() => (isComplete = true)); @@ -344,9 +378,7 @@ describe('ReactDOMFizzServerBrowser', () => { ]); hasLoaded = true; - resolve(); - - await jest.runAllTimers(); + await serverAct(() => resolve()); expect(rendered).toBe(false); expect(isComplete).toBe(true); @@ -366,14 +398,16 @@ describe('ReactDOMFizzServerBrowser', () => { // as such for now. I don't think it needs to be maintained if in the future // the view sizes change or become dynamic becasue of the use of byobRequest let stream; - stream = await ReactDOMFizzServer.renderToReadableStream( - <> -
- {''} -
-
{str492}
-
{str492}
- , + stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream( + <> +
+ {''} +
+
{str492}
+
{str492}
+ , + ), ); let result; @@ -385,10 +419,12 @@ describe('ReactDOMFizzServerBrowser', () => { // this size 2049 was chosen to be a couple base 2 orders larger than the current view // size. if the size changes in the future hopefully this will still exercise // a chunk that is too large for the view size. - stream = await ReactDOMFizzServer.renderToReadableStream( - <> -
{str2049}
- , + stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream( + <> +
{str2049}
+ , + ), ); result = await readResult(stream); @@ -419,13 +455,15 @@ describe('ReactDOMFizzServerBrowser', () => { const errors = []; const controller = new AbortController(); - await ReactDOMFizzServer.renderToReadableStream(, { - signal: controller.signal, - onError(x) { - errors.push(x); - return 'a digest'; - }, - }); + await serverAct(() => + ReactDOMFizzServer.renderToReadableStream(, { + signal: controller.signal, + onError(x) { + errors.push(x); + return 'a digest'; + }, + }), + ); controller.abort('foobar'); @@ -456,13 +494,15 @@ describe('ReactDOMFizzServerBrowser', () => { const errors = []; const controller = new AbortController(); - await ReactDOMFizzServer.renderToReadableStream(, { - signal: controller.signal, - onError(x) { - errors.push(x.message); - return 'a digest'; - }, - }); + await serverAct(() => + ReactDOMFizzServer.renderToReadableStream(, { + signal: controller.signal, + onError(x) { + errors.push(x.message); + return 'a digest'; + }, + }), + ); controller.abort(new Error('uh oh')); @@ -471,13 +511,15 @@ describe('ReactDOMFizzServerBrowser', () => { // https://github.com/facebook/react/pull/25534/files - fix transposed escape functions it('should encode title properly', async () => { - const stream = await ReactDOMFizzServer.renderToReadableStream( - - - foo - - bar - , + const stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream( + + + foo + + bar + , + ), ); const result = await readResult(stream); @@ -488,14 +530,13 @@ describe('ReactDOMFizzServerBrowser', () => { it('should support nonce attribute for bootstrap scripts', async () => { const nonce = 'R4nd0m'; - const stream = await ReactDOMFizzServer.renderToReadableStream( -
hello world
, - { + const stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream(
hello world
, { nonce, bootstrapScriptContent: 'INIT();', bootstrapScripts: ['init.js'], bootstrapModules: ['init.mjs'], - }, + }), ); const result = await readResult(stream); expect(result).toMatchInlineSnapshot( @@ -523,14 +564,16 @@ describe('ReactDOMFizzServerBrowser', () => { let caughtError = null; try { - await ReactDOMFizzServer.renderToReadableStream(, { - onError(error) { - errors.push(error.message); - }, - onPostpone(reason) { - postponed.push(reason); - }, - }); + await serverAct(() => + ReactDOMFizzServer.renderToReadableStream(, { + onError(error) { + errors.push(error.message); + }, + onPostpone(reason) { + postponed.push(reason); + }, + }), + ); } catch (error) { caughtError = error; } diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js index 043c5fc42a923..7a3db48b016e3 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js @@ -9,6 +9,8 @@ 'use strict'; +import {patchMessageChannel} from '../../../../scripts/jest/patchMessageChannel'; + import { getVisibleChildren, insertNodesAndExecuteScripts, @@ -26,10 +28,17 @@ let ReactDOMFizzServer; let ReactDOMFizzStatic; let Suspense; let container; +let Scheduler; +let act; describe('ReactDOMFizzStaticBrowser', () => { beforeEach(() => { jest.resetModules(); + + Scheduler = require('scheduler'); + patchMessageChannel(Scheduler); + act = require('internal-test-utils').act; + React = require('react'); ReactDOM = require('react-dom'); ReactDOMFizzServer = require('react-dom/server.browser'); @@ -45,6 +54,17 @@ describe('ReactDOMFizzStaticBrowser', () => { document.body.removeChild(container); }); + async function serverAct(callback) { + let maybePromise; + await act(() => { + maybePromise = callback(); + if (maybePromise && typeof maybePromise.catch === 'function') { + maybePromise.catch(() => {}); + } + }); + return maybePromise; + } + const theError = new Error('This is an error'); function Throw() { throw theError; @@ -113,17 +133,21 @@ describe('ReactDOMFizzStaticBrowser', () => { // @gate experimental it('should call prerender', async () => { - const result = await ReactDOMFizzStatic.prerender(
hello world
); + const result = await serverAct(() => + ReactDOMFizzStatic.prerender(
hello world
), + ); const prelude = await readContent(result.prelude); expect(prelude).toMatchInlineSnapshot(`"
hello world
"`); }); // @gate experimental it('should emit DOCTYPE at the root of the document', async () => { - const result = await ReactDOMFizzStatic.prerender( - - hello world - , + const result = await serverAct(() => + ReactDOMFizzStatic.prerender( + + hello world + , + ), ); const prelude = await readContent(result.prelude); expect(prelude).toMatchInlineSnapshot( @@ -133,11 +157,13 @@ describe('ReactDOMFizzStaticBrowser', () => { // @gate experimental it('should emit bootstrap script src at the end', async () => { - const result = await ReactDOMFizzStatic.prerender(
hello world
, { - bootstrapScriptContent: 'INIT();', - bootstrapScripts: ['init.js'], - bootstrapModules: ['init.mjs'], - }); + const result = await serverAct(() => + ReactDOMFizzStatic.prerender(
hello world
, { + bootstrapScriptContent: 'INIT();', + bootstrapScripts: ['init.js'], + bootstrapModules: ['init.mjs'], + }), + ); const prelude = await readContent(result.prelude); expect(prelude).toMatchInlineSnapshot( `"
hello world
"`, @@ -155,12 +181,14 @@ describe('ReactDOMFizzStaticBrowser', () => { } return 'Done'; } - const resultPromise = ReactDOMFizzStatic.prerender( -
- - - -
, + const resultPromise = serverAct(() => + ReactDOMFizzStatic.prerender( +
+ + + +
, + ), ); await jest.runAllTimers(); @@ -171,9 +199,7 @@ describe('ReactDOMFizzStaticBrowser', () => { const result = await resultPromise; const prelude = await readContent(result.prelude); - expect(prelude).toMatchInlineSnapshot( - `"
Done
"`, - ); + expect(prelude).toMatchInlineSnapshot(`"
Done
"`); }); // @gate experimental @@ -181,15 +207,17 @@ describe('ReactDOMFizzStaticBrowser', () => { const reportedErrors = []; let caughtError = null; try { - await ReactDOMFizzStatic.prerender( -
- -
, - { - onError(x) { - reportedErrors.push(x); + await serverAct(() => + ReactDOMFizzStatic.prerender( +
+ +
, + { + onError(x) { + reportedErrors.push(x); + }, }, - }, + ), ); } catch (error) { caughtError = error; @@ -203,17 +231,19 @@ describe('ReactDOMFizzStaticBrowser', () => { const reportedErrors = []; let caughtError = null; try { - await ReactDOMFizzStatic.prerender( -
- }> - - -
, - { - onError(x) { - reportedErrors.push(x); + await serverAct(() => + ReactDOMFizzStatic.prerender( +
+ }> + + +
, + { + onError(x) { + reportedErrors.push(x); + }, }, - }, + ), ); } catch (error) { caughtError = error; @@ -225,17 +255,19 @@ describe('ReactDOMFizzStaticBrowser', () => { // @gate experimental it('should not error the stream when an error is thrown inside suspense boundary', async () => { const reportedErrors = []; - const result = await ReactDOMFizzStatic.prerender( -
- Loading
}> - - - , - { - onError(x) { - reportedErrors.push(x); + const result = await serverAct(() => + ReactDOMFizzStatic.prerender( +
+ Loading
}> + + + , + { + onError(x) { + reportedErrors.push(x); + }, }, - }, + ), ); const prelude = await readContent(result.prelude); @@ -247,21 +279,22 @@ describe('ReactDOMFizzStaticBrowser', () => { it('should be able to complete by aborting even if the promise never resolves', async () => { const errors = []; const controller = new AbortController(); - const resultPromise = ReactDOMFizzStatic.prerender( -
- Loading
}> - - - , - { - signal: controller.signal, - onError(x) { - errors.push(x.message); + let resultPromise; + await serverAct(() => { + resultPromise = ReactDOMFizzStatic.prerender( +
+ Loading
}> + + + , + { + signal: controller.signal, + onError(x) { + errors.push(x.message); + }, }, - }, - ); - - await jest.runAllTimers(); + ); + }); controller.abort(); @@ -277,16 +310,18 @@ describe('ReactDOMFizzStaticBrowser', () => { it('should reject if aborting before the shell is complete', async () => { const errors = []; const controller = new AbortController(); - const promise = ReactDOMFizzStatic.prerender( -
- -
, - { - signal: controller.signal, - onError(x) { - errors.push(x.message); + const promise = serverAct(() => + ReactDOMFizzStatic.prerender( +
+ +
, + { + signal: controller.signal, + onError(x) { + errors.push(x.message); + }, }, - }, + ), ); await jest.runAllTimers(); @@ -316,16 +351,18 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const streamPromise = ReactDOMFizzStatic.prerender( -
- -
, - { - signal: controller.signal, - onError(x) { - errors.push(x.message); + const streamPromise = serverAct(() => + ReactDOMFizzStatic.prerender( +
+ +
, + { + signal: controller.signal, + onError(x) { + errors.push(x.message); + }, }, - }, + ), ); let caughtError = null; @@ -345,18 +382,20 @@ describe('ReactDOMFizzStaticBrowser', () => { const theReason = new Error('aborted for reasons'); controller.abort(theReason); - const promise = ReactDOMFizzStatic.prerender( -
- Loading
}> - - - , - { - signal: controller.signal, - onError(x) { - errors.push(x.message); + const promise = serverAct(() => + ReactDOMFizzStatic.prerender( +
+ Loading
}> + + + , + { + signal: controller.signal, + onError(x) { + errors.push(x.message); + }, }, - }, + ), ); // Technically we could still continue rendering the shell but currently the @@ -396,12 +435,15 @@ describe('ReactDOMFizzStaticBrowser', () => { const errors = []; const controller = new AbortController(); - const resultPromise = ReactDOMFizzStatic.prerender(, { - signal: controller.signal, - onError(x) { - errors.push(x); - return 'a digest'; - }, + let resultPromise; + await serverAct(() => { + resultPromise = ReactDOMFizzStatic.prerender(, { + signal: controller.signal, + onError(x) { + errors.push(x); + return 'a digest'; + }, + }); }); controller.abort('foobar'); @@ -436,12 +478,15 @@ describe('ReactDOMFizzStaticBrowser', () => { const errors = []; const controller = new AbortController(); - const resultPromise = ReactDOMFizzStatic.prerender(, { - signal: controller.signal, - onError(x) { - errors.push(x.message); - return 'a digest'; - }, + let resultPromise; + await serverAct(() => { + resultPromise = ReactDOMFizzStatic.prerender(, { + signal: controller.signal, + onError(x) { + errors.push(x.message); + return 'a digest'; + }, + }); }); controller.abort(new Error('uh oh')); @@ -471,14 +516,18 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); prerendering = false; - const resumed = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const resumed = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); await readIntoContainer(prerendered.prelude); @@ -513,14 +562,18 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); prerendering = false; - const resumed = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const resumed = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); await readIntoContainer(prerendered.prelude); @@ -552,14 +605,18 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); prerendering = false; - const resumed = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const resumed = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); await readIntoContainer(prerendered.prelude); @@ -600,14 +657,18 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); prerendering = false; - const resumed = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const resumed = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); await readIntoContainer(prerendered.prelude); @@ -641,14 +702,18 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); prerendering = false; - const resumed = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const resumed = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); await readIntoContainer(prerendered.prelude); @@ -682,14 +747,18 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); prerendering = false; - const content = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const content = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); const html = await readContent(concat(prerendered.prelude, content)); @@ -748,9 +817,11 @@ describe('ReactDOMFizzStaticBrowser', () => { {virtual: true}, ); - const prerendered = await ReactDOMFizzStatic.prerender(, { - bootstrapScripts: ['init.js'], - }); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(, { + bootstrapScripts: ['init.js'], + }), + ); expect(prerendered.postponed).not.toBe(null); await readIntoContainer(prerendered.prelude); @@ -779,9 +850,11 @@ describe('ReactDOMFizzStaticBrowser', () => { ]); prerendering = false; - const content = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const content = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); await readIntoContainer(content); @@ -860,14 +933,18 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); prerendering = false; - const resumed = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const resumed = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); await readIntoContainer(prerendered.prelude); @@ -911,14 +988,18 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); prerendering = false; - const resumed = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const resumed = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); await readIntoContainer(prerendered.prelude); @@ -957,7 +1038,9 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); // TODO: This should actually be null because we should've been able to fully // resolve the render on the server eventually, even though the fallback postponed. // So we should not need to resume. @@ -967,9 +1050,11 @@ describe('ReactDOMFizzStaticBrowser', () => { expect(getVisibleChildren(container)).toEqual(
Outer
); - const resumed = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const resumed = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); await readIntoContainer(resumed); @@ -1020,7 +1105,9 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); await readIntoContainer(prerendered.prelude); @@ -1033,14 +1120,16 @@ describe('ReactDOMFizzStaticBrowser', () => { prerendering = false; const errors = []; - const resumed = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), - { - onError(x) { - errors.push(x.message); + const resumed = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + { + onError(x) { + errors.push(x.message); + }, }, - }, + ), ); expect(errors).toEqual([ @@ -1085,7 +1174,9 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); await readIntoContainer(prerendered.prelude); @@ -1098,15 +1189,17 @@ describe('ReactDOMFizzStaticBrowser', () => { const errors = []; - const resumedPromise = ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), - { - signal: controller.signal, - onError(x) { - errors.push(x); + const resumedPromise = serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + { + signal: controller.signal, + onError(x) { + errors.push(x); + }, }, - }, + ), ); controller.abort('abort'); @@ -1160,16 +1253,20 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); await readIntoContainer(prerendered.prelude); prerendering = false; - const resumedPromise = ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const resumedPromise = serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); await jest.runAllTimers(); @@ -1204,16 +1301,20 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); prerendering = false; expect(await readContent(prerendered.prelude)).toBe(''); - const content = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const content = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); expect(await readContent(content)).toBe( @@ -1246,16 +1347,20 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); prerendering = false; expect(await readContent(prerendered.prelude)).toBe(''); - const content = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const content = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); expect(await readContent(content)).toBe( @@ -1293,16 +1398,20 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); prerendering = false; expect(await readContent(prerendered.prelude)).toBe(''); - const content = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const content = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); expect(await readContent(content)).toBe( @@ -1356,9 +1465,11 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(, { - onHeaders, - }); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(, { + onHeaders, + }), + ); expect(prerendered.postponed).not.toBe(null); prerendering = false; @@ -1375,9 +1486,11 @@ describe('ReactDOMFizzStaticBrowser', () => { }), ); - const content = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const content = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); const decoder = new TextDecoder(); @@ -1391,7 +1504,7 @@ describe('ReactDOMFizzStaticBrowser', () => { await 1; hasLoaded = true; - resolve(); + await serverAct(resolve); while (true) { ({value, done} = await reader.read()); @@ -1425,10 +1538,12 @@ describe('ReactDOMFizzStaticBrowser', () => { throw new Error('bad onHeaders'); } - const prerendered = await ReactDOMFizzStatic.prerender(
hello
, { - onHeaders, - onError, - }); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(
hello
, { + onHeaders, + onError, + }), + ); expect(prerendered.postponed).toBe(null); expect(errors).toEqual(['bad onHeaders']); @@ -1469,9 +1584,11 @@ describe('ReactDOMFizzStaticBrowser', () => { {virtual: true}, ); - const prerendered = await ReactDOMFizzStatic.prerender(, { - bootstrapScripts: ['init.js'], - }); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(, { + bootstrapScripts: ['init.js'], + }), + ); const postponedSerializedState = JSON.stringify(prerendered.postponed); @@ -1497,9 +1614,8 @@ describe('ReactDOMFizzStaticBrowser', () => { prerendering = false; - const content = await ReactDOMFizzServer.resume( - , - JSON.parse(postponedSerializedState), + const content = await serverAct(() => + ReactDOMFizzServer.resume(, JSON.parse(postponedSerializedState)), ); await readIntoContainer(content); @@ -1542,7 +1658,9 @@ describe('ReactDOMFizzStaticBrowser', () => { ); } - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); const postponedState = JSON.stringify(prerendered.postponed); await readIntoContainer(prerendered.prelude); @@ -1550,9 +1668,8 @@ describe('ReactDOMFizzStaticBrowser', () => { isPrerendering = false; - const dynamic = await ReactDOMFizzServer.resume( - , - JSON.parse(postponedState), + const dynamic = await serverAct(() => + ReactDOMFizzServer.resume(, JSON.parse(postponedState)), ); await readIntoContainer(dynamic); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticFloat-test.js index 9a825bf1e3871..baa65c806c0ba 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticFloat-test.js @@ -9,6 +9,8 @@ 'use strict'; +import {patchMessageChannel} from '../../../../scripts/jest/patchMessageChannel'; + import { getVisibleChildren, insertNodesAndExecuteScripts, @@ -25,10 +27,16 @@ let ReactDOMFizzServer; let ReactDOMFizzStatic; let Suspense; let container; +let Scheduler; +let act; describe('ReactDOMFizzStaticFloat', () => { beforeEach(() => { jest.resetModules(); + Scheduler = require('scheduler'); + patchMessageChannel(Scheduler); + act = require('internal-test-utils').act; + React = require('react'); ReactDOM = require('react-dom'); ReactDOMFizzServer = require('react-dom/server.browser'); @@ -44,6 +52,17 @@ describe('ReactDOMFizzStaticFloat', () => { document.body.removeChild(container); }); + async function serverAct(callback) { + let maybePromise; + await act(() => { + maybePromise = callback(); + if (maybePromise && typeof maybePromise.catch === 'function') { + maybePromise.catch(() => {}); + } + }); + return maybePromise; + } + async function readIntoContainer(stream) { const reader = stream.getReader(); let result = ''; @@ -135,7 +154,9 @@ describe('ReactDOMFizzStaticFloat', () => { virtual: true, }); - const prerendered = await ReactDOMFizzStatic.prerender(); + const prerendered = await serverAct(() => + ReactDOMFizzStatic.prerender(), + ); expect(prerendered.postponed).not.toBe(null); await readIntoContainer(prerendered.prelude); @@ -171,28 +192,28 @@ describe('ReactDOMFizzStaticFloat', () => { ]); prerendering = false; - const content = await ReactDOMFizzServer.resume( - , - JSON.parse(JSON.stringify(prerendered.postponed)), + const content = await serverAct(() => + ReactDOMFizzServer.resume( + , + JSON.parse(JSON.stringify(prerendered.postponed)), + ), ); await readIntoContainer(content); - // Dispatch load event to injected stylesheet - const linkCreds = document.querySelector( - 'link[rel="stylesheet"][href="style creds"]', - ); - const linkAnon = document.querySelector( - 'link[rel="stylesheet"][href="style anon"]', - ); - const event = document.createEvent('Events'); - event.initEvent('load', true, true); - linkCreds.dispatchEvent(event); - linkAnon.dispatchEvent(event); - - // Wait for the instruction microtasks to flush. - await 0; - await 0; + await act(() => { + // Dispatch load event to injected stylesheet + const linkCreds = document.querySelector( + 'link[rel="stylesheet"][href="style creds"]', + ); + const linkAnon = document.querySelector( + 'link[rel="stylesheet"][href="style anon"]', + ); + const event = document.createEvent('Events'); + event.initEvent('load', true, true); + linkCreds.dispatchEvent(event); + linkAnon.dispatchEvent(event); + }); expect(getVisibleChildren(document)).toEqual( diff --git a/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOM-test.js b/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOM-test.js index f74143b220634..eef2e824543e7 100644 --- a/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOM-test.js +++ b/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOM-test.js @@ -9,16 +9,14 @@ 'use strict'; +import {patchSetImmediate} from '../../../../scripts/jest/patchSetImmediate'; + // Polyfills for test environment global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStream; global.TextEncoder = require('util').TextEncoder; global.TextDecoder = require('util').TextDecoder; -// Don't wait before processing work on the server. -// TODO: we can replace this with FlightServer.act(). -global.setImmediate = cb => cb(); - let act; let use; let clientExports; @@ -29,6 +27,8 @@ let ReactDOMClient; let ReactServerDOMServer; let ReactServerDOMClient; let Suspense; +let ReactServerScheduler; +let reactServerAct; describe('ReactFlightDOM', () => { beforeEach(() => { @@ -37,6 +37,10 @@ describe('ReactFlightDOM', () => { // condition jest.resetModules(); + ReactServerScheduler = require('scheduler'); + patchSetImmediate(ReactServerScheduler); + reactServerAct = require('internal-test-utils').act; + // Simulate the condition resolution jest.mock('react-server-dom-turbopack/server', () => require('react-server-dom-turbopack/server.node.unbundled'), @@ -61,6 +65,17 @@ describe('ReactFlightDOM', () => { ReactServerDOMClient = require('react-server-dom-turbopack/client'); }); + async function serverAct(callback) { + let maybePromise; + await reactServerAct(() => { + maybePromise = callback(); + if (maybePromise && typeof maybePromise.catch === 'function') { + maybePromise.catch(() => {}); + } + }); + return maybePromise; + } + function getTestStream() { const writable = new Stream.PassThrough(); const readable = new ReadableStream({ @@ -100,9 +115,8 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - turbopackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream(, turbopackMap), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -149,9 +163,8 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - turbopackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream(, turbopackMap), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -191,9 +204,11 @@ describe('ReactFlightDOM', () => { const AsyncModuleRef2 = await clientExports(AsyncModule2); const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - turbopackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + turbopackMap, + ), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); diff --git a/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOMBrowser-test.js b/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOMBrowser-test.js index d797946a3fd3d..a47cca7068801 100644 --- a/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOMBrowser-test.js +++ b/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOMBrowser-test.js @@ -9,6 +9,8 @@ 'use strict'; +import {patchMessageChannel} from '../../../../scripts/jest/patchMessageChannel'; + // Polyfills for test environment global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStream; @@ -18,11 +20,17 @@ global.TextDecoder = require('util').TextDecoder; let React; let ReactServerDOMServer; let ReactServerDOMClient; +let ReactServerScheduler; +let reactServerAct; describe('ReactFlightDOMBrowser', () => { beforeEach(() => { jest.resetModules(); + ReactServerScheduler = require('scheduler'); + patchMessageChannel(ReactServerScheduler); + reactServerAct = require('internal-test-utils').act; + // Simulate the condition resolution jest.mock('react', () => require('react/react.react-server')); jest.mock('react-server-dom-turbopack/server', () => @@ -38,6 +46,17 @@ describe('ReactFlightDOMBrowser', () => { ReactServerDOMClient = require('react-server-dom-turbopack/client'); }); + async function serverAct(callback) { + let maybePromise; + await reactServerAct(() => { + maybePromise = callback(); + if (maybePromise && typeof maybePromise.catch === 'function') { + maybePromise.catch(() => {}); + } + }); + return maybePromise; + } + it('should resolve HTML using W3C streams', async () => { function Text({children}) { return {children}; @@ -58,7 +77,9 @@ describe('ReactFlightDOMBrowser', () => { return model; } - const stream = ReactServerDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(), + ); const response = ReactServerDOMClient.createFromReadableStream(stream); const model = await response; expect(model).toEqual({ diff --git a/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOMNode-test.js b/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOMNode-test.js index e06ee0a32f950..1276d4d0be40b 100644 --- a/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOMNode-test.js +++ b/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOMNode-test.js @@ -9,9 +9,7 @@ 'use strict'; -// Don't wait before processing work on the server. -// TODO: we can replace this with FlightServer.act(). -global.setImmediate = cb => cb(); +import {patchSetImmediate} from '../../../../scripts/jest/patchSetImmediate'; let clientExports; let turbopackMap; @@ -23,11 +21,17 @@ let ReactServerDOMServer; let ReactServerDOMClient; let Stream; let use; +let ReactServerScheduler; +let reactServerAct; describe('ReactFlightDOMNode', () => { beforeEach(() => { jest.resetModules(); + ReactServerScheduler = require('scheduler'); + patchSetImmediate(ReactServerScheduler); + reactServerAct = require('internal-test-utils').act; + // Simulate the condition resolution jest.mock('react', () => require('react/react.react-server')); jest.mock('react-server-dom-turbopack/server', () => @@ -55,6 +59,17 @@ describe('ReactFlightDOMNode', () => { use = React.use; }); + async function serverAct(callback) { + let maybePromise; + await reactServerAct(() => { + maybePromise = callback(); + if (maybePromise && typeof maybePromise.catch === 'function') { + maybePromise.catch(() => {}); + } + }); + return maybePromise; + } + function readResult(stream) { return new Promise((resolve, reject) => { let buffer = ''; @@ -102,9 +117,8 @@ describe('ReactFlightDOMNode', () => { return ; } - const stream = ReactServerDOMServer.renderToPipeableStream( - , - turbopackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream(, turbopackMap), ); const readable = new Stream.PassThrough(); @@ -121,8 +135,8 @@ describe('ReactFlightDOMNode', () => { return use(response); } - const ssrStream = await ReactDOMServer.renderToPipeableStream( - , + const ssrStream = await serverAct(() => + ReactDOMServer.renderToPipeableStream(), ); const result = await readResult(ssrStream); expect(result).toEqual( diff --git a/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOMReply-test.js b/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOMReply-test.js index e47352cfe981d..cf328ab2e8fe3 100644 --- a/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOMReply-test.js +++ b/packages/react-server-dom-turbopack/src/__tests__/ReactFlightTurbopackDOMReply-test.js @@ -9,6 +9,8 @@ 'use strict'; +import {patchMessageChannel} from '../../../../scripts/jest/patchMessageChannel'; + // Polyfills for test environment global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStream; @@ -19,10 +21,15 @@ global.TextDecoder = require('util').TextDecoder; let turbopackServerMap; let ReactServerDOMServer; let ReactServerDOMClient; +let ReactServerScheduler; describe('ReactFlightDOMReply', () => { beforeEach(() => { jest.resetModules(); + + ReactServerScheduler = require('scheduler'); + patchMessageChannel(ReactServerScheduler); + // Simulate the condition resolution jest.mock('react', () => require('react/react.react-server')); jest.mock('react-server-dom-turbopack/server', () => diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js index 5315b990d8f78..1ead6efe4b25a 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -9,16 +9,14 @@ 'use strict'; +import {patchSetImmediate} from '../../../../scripts/jest/patchSetImmediate'; + // Polyfills for test environment global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStream; global.TextEncoder = require('util').TextEncoder; global.TextDecoder = require('util').TextDecoder; -// Don't wait before processing work on the server. -// TODO: we can replace this with FlightServer.act(). -global.setImmediate = cb => cb(); - let act; let use; let clientExports; @@ -36,6 +34,8 @@ let ReactDOMStaticServer; let Suspense; let ErrorBoundary; let JSDOM; +let ReactServerScheduler; +let reactServerAct; describe('ReactFlightDOM', () => { beforeEach(() => { @@ -46,6 +46,10 @@ describe('ReactFlightDOM', () => { JSDOM = require('jsdom').JSDOM; + ReactServerScheduler = require('scheduler'); + patchSetImmediate(ReactServerScheduler); + reactServerAct = require('internal-test-utils').act; + // Simulate the condition resolution jest.mock('react', () => require('react/react.react-server')); FlightReact = require('react'); @@ -92,6 +96,17 @@ describe('ReactFlightDOM', () => { }; }); + async function serverAct(callback) { + let maybePromise; + await reactServerAct(() => { + maybePromise = callback(); + if (maybePromise && typeof maybePromise.catch === 'function') { + maybePromise.catch(() => {}); + } + }); + return maybePromise; + } + function getTestStream() { const writable = new Stream.PassThrough(); const readable = new ReadableStream({ @@ -181,9 +196,8 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream(, webpackMap), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -230,9 +244,8 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream(, webpackMap), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -266,9 +279,8 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream(, webpackMap), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -300,9 +312,8 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream(, webpackMap), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -349,9 +360,11 @@ describe('ReactFlightDOM', () => { ); const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -386,9 +399,11 @@ describe('ReactFlightDOM', () => { const {Component} = clientExports(Module); const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -424,9 +439,11 @@ describe('ReactFlightDOM', () => { const {split: Component} = clientExports(Module); const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -464,9 +481,11 @@ describe('ReactFlightDOM', () => { const AsyncModuleRef2 = await clientExports(AsyncModule2); const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -502,9 +521,11 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -539,9 +560,8 @@ describe('ReactFlightDOM', () => { const ThenRef = clientExports(thenExports).then; const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream(, webpackMap), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -719,15 +739,13 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - model, - webpackMap, - { + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream(model, webpackMap, { onError(x) { reportedErrors.push(x); return __DEV__ ? 'a dev digest' : `digest("${x.message}")`; }, - }, + }), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -744,14 +762,18 @@ describe('ReactFlightDOM', () => { expect(container.innerHTML).toBe('

(loading)

'); // This isn't enough to show anything. - await act(() => { - resolveFriends(); + await serverAct(async () => { + await act(() => { + resolveFriends(); + }); }); expect(container.innerHTML).toBe('

(loading)

'); // We can now show the details. Sidebar and posts are still loading. - await act(() => { - resolveName(); + await serverAct(async () => { + await act(() => { + resolveName(); + }); }); // Advance time enough to trigger a nested fallback. await act(() => { @@ -768,9 +790,11 @@ describe('ReactFlightDOM', () => { const theError = new Error('Game over'); // Let's *fail* loading games. - await act(async () => { - await rejectGames(theError); - await 'the inner async function'; + await serverAct(async () => { + await act(async () => { + await rejectGames(theError); + await 'the inner async function'; + }); }); const expectedGamesValue = __DEV__ ? '

Game over + a dev digest

' @@ -786,9 +810,11 @@ describe('ReactFlightDOM', () => { reportedErrors = []; // We can now show the sidebar. - await act(async () => { - await resolvePhotos(); - await 'the inner async function'; + await serverAct(async () => { + await act(async () => { + await resolvePhotos(); + await 'the inner async function'; + }); }); expect(container.innerHTML).toBe( '
:name::avatar:
' + @@ -798,9 +824,11 @@ describe('ReactFlightDOM', () => { ); // Show everything. - await act(async () => { - await resolvePosts(); - await 'the inner async function'; + await serverAct(async () => { + await act(async () => { + await resolvePosts(); + await 'the inner async function'; + }); }); expect(container.innerHTML).toBe( '
:name::avatar:
' + @@ -867,14 +895,16 @@ describe('ReactFlightDOM', () => { const [Photos, resolvePhotosData] = makeDelayedText(); const suspendedChunk = createSuspendedChunk(

loading

); const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - suspendedChunk.row, - webpackMap, - { - onError(error) { - reportedErrors.push(error); + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + suspendedChunk.row, + webpackMap, + { + onError(error) { + reportedErrors.push(error); + }, }, - }, + ), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -900,16 +930,20 @@ describe('ReactFlightDOM', () => { ); - await act(async () => { - suspendedChunk.resolve({value, done: false, next: donePromise.promise}); - donePromise.resolve({value, done: true}); + await serverAct(async () => { + await act(async () => { + suspendedChunk.resolve({value, done: false, next: donePromise.promise}); + donePromise.resolve({value, done: true}); + }); }); expect(container.innerHTML).toBe('

loading posts and photos

'); - await act(async () => { - await resolvePostsData('posts'); - await resolvePhotosData('photos'); + await serverAct(async () => { + await act(async () => { + await resolvePostsData('posts'); + await resolvePhotosData('photos'); + }); }); expect(container.innerHTML).toBe('
posts
photos
'); @@ -945,9 +979,11 @@ describe('ReactFlightDOM', () => { const root = ReactDOMClient.createRoot(container); const stream1 = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ), ); pipe(stream1.writable); const response1 = ReactServerDOMClient.createFromReadableStream( @@ -973,9 +1009,11 @@ describe('ReactFlightDOM', () => { inputB.value = 'goodbye'; const stream2 = getTestStream(); - const {pipe: pipe2} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe: pipe2} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ), ); pipe2(stream2.writable); const response2 = ReactServerDOMClient.createFromReadableStream( @@ -1005,18 +1043,20 @@ describe('ReactFlightDOM', () => { const reportedErrors = []; const {writable, readable} = getTestStream(); - const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream( -
- -
, - webpackMap, - { - onError(x) { - reportedErrors.push(x); - const message = typeof x === 'string' ? x : x.message; - return __DEV__ ? 'a dev digest' : `digest("${message}")`; + const {pipe, abort} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( +
+ +
, + webpackMap, + { + onError(x) { + reportedErrors.push(x); + const message = typeof x === 'string' ? x : x.message; + return __DEV__ ? 'a dev digest' : `digest("${message}")`; + }, }, - }, + ), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -1067,16 +1107,18 @@ describe('ReactFlightDOM', () => { const ClientReference = clientModuleError(new Error('module init error')); const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( -
- -
, - webpackMap, - { - onError(x) { - reportedErrors.push(x); + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( +
+ +
, + webpackMap, + { + onError(x) { + reportedErrors.push(x); + }, }, - }, + ), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -1117,16 +1159,18 @@ describe('ReactFlightDOM', () => { ); const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( -
- -
, - webpackMap, - { - onError(x) { - reportedErrors.push(x); + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( +
+ +
, + webpackMap, + { + onError(x) { + reportedErrors.push(x); + }, }, - }, + ), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -1176,17 +1220,19 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( -
- -
, - webpackMap, - { - onError(x) { - reportedErrors.push(x.message); - return __DEV__ ? 'a dev digest' : `digest("${x.message}")`; + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( +
+ +
, + webpackMap, + { + onError(x) { + reportedErrors.push(x.message); + return __DEV__ ? 'a dev digest' : `digest("${x.message}")`; + }, }, - }, + ), ); pipe(writable); @@ -1255,9 +1301,11 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -1311,15 +1359,17 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, - { - onError(x) { - reportedErrors.push(x); - return __DEV__ ? 'a dev digest' : `digest("${x.message}")`; + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + { + onError(x) { + reportedErrors.push(x); + return __DEV__ ? 'a dev digest' : `digest("${x.message}")`; + }, }, - }, + ), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); @@ -1368,9 +1418,11 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ), ); pipe(writable); @@ -1463,9 +1515,8 @@ describe('ReactFlightDOM', () => { const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream(, webpackMap), ); pipe(writable); @@ -1485,11 +1536,10 @@ describe('ReactFlightDOM', () => { function onError(error, errorInfo) { errors.push(error, errorInfo); } - const result = await ReactDOMStaticServer.prerenderToNodeStream( - , - { + const result = await serverAct(() => + ReactDOMStaticServer.prerenderToNodeStream(, { onError, - }, + }), ); const prelude = await new Promise((resolve, reject) => { @@ -1554,9 +1604,11 @@ describe('ReactFlightDOM', () => { // module graphs and we are contriving the sequencing to work in a way where // the right HostDispatcher is in scope during the Flight Server Float calls and the // Flight Client hint dispatches - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ), ); pipe(flightWritable); @@ -1577,7 +1629,7 @@ describe('ReactFlightDOM', () => { ); } - await act(async () => { + await serverAct(async () => { ReactDOMFizzServer.renderToPipeableStream().pipe(fizzWritable); }); @@ -1680,11 +1732,11 @@ describe('ReactFlightDOM', () => { // pausing to let Flight runtime tick. This is a test only artifact of the fact that // we aren't operating separate module graphs for flight and fiber. In a real app // each would have their own dispatcher and there would be no cross dispatching. - await 1; + await serverAct(() => {}); const {writable: fizzWritable1, readable: fizzReadable1} = getTestStream(); const {writable: fizzWritable2, readable: fizzReadable2} = getTestStream(); - await act(async () => { + await serverAct(async () => { ReactDOMFizzServer.renderToPipeableStream( , ).pipe(fizzWritable1); @@ -1751,10 +1803,12 @@ describe('ReactFlightDOM', () => { const {writable, readable} = getTestStream(); - ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, - ).pipe(writable); + await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ).pipe(writable), + ); const hintRows = []; async function collectHints(stream) { @@ -1798,16 +1852,18 @@ describe('ReactFlightDOM', () => { class InvalidValue {} const {writable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( -
- -
, - webpackMap, - { - onError(x) { - reportedErrors.push(x); + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( +
+ +
, + webpackMap, + { + onError(x) { + reportedErrors.push(x); + }, }, - }, + ), ); pipe(writable); @@ -1839,9 +1895,11 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - const {pipe} = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const {pipe} = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ), ); pipe(writable); const response = ReactServerDOMClient.createFromReadableStream(readable); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js index ed9de3ceb2cb4..1c0d3180ebec1 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -15,6 +15,10 @@ global.ReadableStream = global.TextEncoder = require('util').TextEncoder; global.TextDecoder = require('util').TextDecoder; +const { + patchMessageChannel, +} = require('../../../../scripts/jest/patchMessageChannel'); + let clientExports; let serverExports; let webpackMap; @@ -30,11 +34,18 @@ let Suspense; let use; let ReactServer; let ReactServerDOM; +let Scheduler; +let ReactServerScheduler; +let reactServerAct; describe('ReactFlightDOMBrowser', () => { beforeEach(() => { jest.resetModules(); + ReactServerScheduler = require('scheduler'); + patchMessageChannel(ReactServerScheduler); + reactServerAct = require('internal-test-utils').act; + // Simulate the condition resolution jest.mock('react', () => require('react/react.react-server')); @@ -54,6 +65,9 @@ describe('ReactFlightDOMBrowser', () => { __unmockReact(); jest.resetModules(); + Scheduler = require('scheduler'); + patchMessageChannel(Scheduler); + act = require('internal-test-utils').act; React = require('react'); ReactDOM = require('react-dom'); @@ -64,6 +78,17 @@ describe('ReactFlightDOMBrowser', () => { use = React.use; }); + async function serverAct(callback) { + let maybePromise; + await reactServerAct(() => { + maybePromise = callback(); + if (maybePromise && typeof maybePromise.catch === 'function') { + maybePromise.catch(() => {}); + } + }); + return maybePromise; + } + function makeDelayedText(Model) { let error, _resolve, _reject; let promise = new Promise((resolve, reject) => { @@ -152,7 +177,9 @@ describe('ReactFlightDOMBrowser', () => { return model; } - const stream = ReactServerDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(), + ); const response = ReactServerDOMClient.createFromReadableStream(stream); const model = await response; expect(model).toEqual({ @@ -185,7 +212,9 @@ describe('ReactFlightDOMBrowser', () => { return model; } - const stream = ReactServerDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(), + ); const response = ReactServerDOMClient.createFromReadableStream(stream); const model = await response; expect(model).toEqual({ @@ -221,9 +250,8 @@ describe('ReactFlightDOMBrowser', () => { return Hello, World!; } - const stream = ReactServerDOMServer.renderToReadableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(, webpackMap), ); function ClientRoot({response}) { @@ -270,9 +298,11 @@ describe('ReactFlightDOMBrowser', () => { const shared = [1, 2, 3]; const value = [shared, shared]; - const stream = ReactServerDOMServer.renderToReadableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + , + webpackMap, + ), ); function ClientRoot({response}) { @@ -319,9 +349,11 @@ describe('ReactFlightDOMBrowser', () => { const shared = [1, 2, 3]; const value = [shared, shared]; - const stream = ReactServerDOMServer.renderToReadableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + , + webpackMap, + ), ); function ClientRoot({response}) { @@ -457,15 +489,13 @@ describe('ReactFlightDOMBrowser', () => { return use(response).rootContent; } - const stream = ReactServerDOMServer.renderToReadableStream( - model, - webpackMap, - { + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(model, webpackMap, { onError(x) { reportedErrors.push(x); return __DEV__ ? `a dev digest` : `digest("${x.message}")`; }, - }, + }), ); const response = ReactServerDOMClient.createFromReadableStream(stream); @@ -481,14 +511,18 @@ describe('ReactFlightDOMBrowser', () => { expect(container.innerHTML).toBe('

(loading)

'); // This isn't enough to show anything. - await act(() => { - resolveFriends(); + await serverAct(async () => { + await act(() => { + resolveFriends(); + }); }); expect(container.innerHTML).toBe('

(loading)

'); // We can now show the details. Sidebar and posts are still loading. - await act(() => { - resolveName(); + await serverAct(async () => { + await act(() => { + resolveName(); + }); }); // Advance time enough to trigger a nested fallback. jest.advanceTimersByTime(500); @@ -503,8 +537,10 @@ describe('ReactFlightDOMBrowser', () => { const theError = new Error('Game over'); // Let's *fail* loading games. - await act(() => { - rejectGames(theError); + await serverAct(async () => { + await act(() => { + rejectGames(theError); + }); }); const gamesExpectedValue = __DEV__ @@ -522,8 +558,10 @@ describe('ReactFlightDOMBrowser', () => { reportedErrors = []; // We can now show the sidebar. - await act(() => { - resolvePhotos(); + await serverAct(async () => { + await act(() => { + resolvePhotos(); + }); }); expect(container.innerHTML).toBe( '
:name::avatar:
' + @@ -533,8 +571,10 @@ describe('ReactFlightDOMBrowser', () => { ); // Show everything. - await act(() => { - resolvePosts(); + await serverAct(async () => { + await act(() => { + resolvePosts(); + }); }); expect(container.innerHTML).toBe( '
:name::avatar:
' + @@ -596,9 +636,8 @@ describe('ReactFlightDOMBrowser', () => { rootContent: , }; - const stream = ReactServerDOMServer.renderToReadableStream( - model, - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(model, webpackMap), ); const reader = stream.getReader(); @@ -621,7 +660,7 @@ describe('ReactFlightDOMBrowser', () => { // Advance time enough to trigger a nested fallback. jest.advanceTimersByTime(500); - await act(() => {}); + await serverAct(() => {}); expect(flightResponse).toContain('(loading everything)'); expect(flightResponse).toContain('(loading sidebar)'); @@ -629,25 +668,25 @@ describe('ReactFlightDOMBrowser', () => { expect(flightResponse).not.toContain(':friends:'); expect(flightResponse).not.toContain(':name:'); - await act(() => { + await serverAct(() => { resolveFriends(); }); expect(flightResponse).toContain(':friends:'); - await act(() => { + await serverAct(() => { resolveName(); }); expect(flightResponse).toContain(':name:'); - await act(() => { + await serverAct(() => { resolvePhotos(); }); expect(flightResponse).toContain(':photos:'); - await act(() => { + await serverAct(() => { resolvePosts(); }); @@ -695,19 +734,21 @@ describe('ReactFlightDOMBrowser', () => { } const controller = new AbortController(); - const stream = ReactServerDOMServer.renderToReadableStream( -
- -
, - webpackMap, - { - signal: controller.signal, - onError(x) { - const message = typeof x === 'string' ? x : x.message; - reportedErrors.push(x); - return __DEV__ ? 'a dev digest' : `digest("${message}")`; + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( +
+ +
, + webpackMap, + { + signal: controller.signal, + onError(x) { + const message = typeof x === 'string' ? x : x.message; + reportedErrors.push(x); + return __DEV__ ? 'a dev digest' : `digest("${message}")`; + }, }, - }, + ), ); const response = ReactServerDOMClient.createFromReadableStream(stream); @@ -751,17 +792,20 @@ describe('ReactFlightDOMBrowser', () => { const root = ReactDOMClient.createRoot(container); await expect(async () => { - const stream = ReactServerDOMServer.renderToReadableStream( - <> - {Array(6).fill(
no key
)}
- - {Array(6).fill(
no key
)} -
- , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + <> + {Array(6).fill(
no key
)}
+ + {Array(6).fill(
no key
)} +
+ , + webpackMap, + ), ); const result = await ReactServerDOMClient.createFromReadableStream(stream); + await act(() => { root.render(result); }); @@ -777,7 +821,9 @@ describe('ReactFlightDOMBrowser', () => { ); } - const stream = ReactServerDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(), + ); const response = ReactServerDOMClient.createFromReadableStream(stream); function Client() { @@ -816,7 +862,9 @@ describe('ReactFlightDOMBrowser', () => { ); } - const stream = ReactServerDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(), + ); const response = ReactServerDOMClient.createFromReadableStream(stream); function Client() { @@ -853,15 +901,13 @@ describe('ReactFlightDOMBrowser', () => { } const reportedErrors = []; - const stream = ReactServerDOMServer.renderToReadableStream( - , - webpackMap, - { + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(, webpackMap, { onError(x) { reportedErrors.push(x); return __DEV__ ? 'a dev digest' : `digest("${x.message}")`; }, - }, + }), ); const response = ReactServerDOMClient.createFromReadableStream(stream); @@ -912,7 +958,9 @@ describe('ReactFlightDOMBrowser', () => { return ReactServer.use(thenable); } - const stream = ReactServerDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(), + ); const response = ReactServerDOMClient.createFromReadableStream(stream); function Client() { @@ -947,7 +995,9 @@ describe('ReactFlightDOMBrowser', () => { // Because the thenable resolves synchronously, we should be able to finish // rendering synchronously, with no fallback. - const stream = ReactServerDOMServer.renderToReadableStream(); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(), + ); const response = ReactServerDOMClient.createFromReadableStream(stream); function Client() { @@ -988,9 +1038,11 @@ describe('ReactFlightDOMBrowser', () => { const boundFn = ServerModuleA.greet.bind(null, ServerModuleB.upper); - const stream = ReactServerDOMServer.renderToReadableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + , + webpackMap, + ), ); const response = ReactServerDOMClient.createFromReadableStream(stream, { @@ -1035,9 +1087,11 @@ describe('ReactFlightDOMBrowser', () => { }); const ClientRef = clientExports(Client); - const stream = ReactServerDOMServer.renderToReadableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + , + webpackMap, + ), ); const response = ReactServerDOMClient.createFromReadableStream(stream, { @@ -1100,9 +1154,11 @@ describe('ReactFlightDOMBrowser', () => { const ClientRef = clientExports(Client); - const stream = ReactServerDOMServer.renderToReadableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + , + webpackMap, + ), ); const response = ReactServerDOMClient.createFromReadableStream(stream, { @@ -1140,9 +1196,11 @@ describe('ReactFlightDOMBrowser', () => { }); const ClientRef = clientExports(Client); - const stream = ReactServerDOMServer.renderToReadableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + , + webpackMap, + ), ); const response = ReactServerDOMClient.createFromReadableStream(stream, { @@ -1178,26 +1236,29 @@ describe('ReactFlightDOMBrowser', () => { } async function send(text) { - return Promise.reject(new Error(`Error for ${text}`)); + throw new Error(`Error for ${text}`); } const ServerModule = serverExports({send}); const ClientRef = clientExports(Client); - const stream = ReactServerDOMServer.renderToReadableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + , + webpackMap, + ), ); - const response = ReactServerDOMClient.createFromReadableStream(stream, { async callServer(actionId, args) { const body = await ReactServerDOMClient.encodeReply(args); + const result = callServer(actionId, body); + // Flight doesn't attach error handlers early enough. we suppress the warning + // by putting a dummy catch on the result here + result.catch(() => {}); return ReactServerDOMClient.createFromReadableStream( - ReactServerDOMServer.renderToReadableStream( - callServer(actionId, body), - null, - {onError: error => 'test-error-digest'}, - ), + ReactServerDOMServer.renderToReadableStream(result, null, { + onError: error => 'test-error-digest', + }), ); }, }); @@ -1212,17 +1273,17 @@ describe('ReactFlightDOMBrowser', () => { root.render(); }); - if (__DEV__) { - await expect(actionProxy('test')).rejects.toThrow('Error for test'); - } else { - let thrownError; + let thrownError; - try { - await actionProxy('test'); - } catch (error) { - thrownError = error; - } + try { + await serverAct(() => actionProxy('test')); + } catch (error) { + thrownError = error; + } + if (__DEV__) { + expect(thrownError).toEqual(new Error('Error for test')); + } else { expect(thrownError).toEqual( new Error( 'An error occurred in the Server Components render. The specific message is omitted in production builds to avoid leaking sensitive details. A digest property is included on this error instance which may provide additional details about the nature of the error.', @@ -1253,9 +1314,14 @@ describe('ReactFlightDOMBrowser', () => { }); const ClientRef = clientExports(Client); - const stream = ReactServerDOMServer.renderToReadableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + , + webpackMap, + ), ); const response = ReactServerDOMClient.createFromReadableStream(stream, { @@ -1298,9 +1364,11 @@ describe('ReactFlightDOMBrowser', () => { ); // Send the action to the client - const stream = ReactServerDOMServer.renderToReadableStream( - {action: serverModule.action}, - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + {action: serverModule.action}, + webpackMap, + ), ); const response = await ReactServerDOMClient.createFromReadableStream(stream); @@ -1340,9 +1408,11 @@ describe('ReactFlightDOMBrowser', () => { return ; } - const stream = ReactServerDOMServer.renderToReadableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + , + webpackMap, + ), ); let response = null; @@ -1406,9 +1476,11 @@ describe('ReactFlightDOMBrowser', () => { return ; } - const stream = ReactServerDOMServer.renderToReadableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + , + webpackMap, + ), ); let response = null; @@ -1427,15 +1499,11 @@ describe('ReactFlightDOMBrowser', () => { ); } - // pausing to let Flight runtime tick. This is a test only artifact of the fact that - // we aren't operating separate module graphs for flight and fiber. In a real app - // each would have their own dispatcher and there would be no cross dispatching. - await 1; - - let fizzStream; + let fizzPromise; await act(async () => { - fizzStream = await ReactDOMFizzServer.renderToReadableStream(); + fizzPromise = ReactDOMFizzServer.renderToReadableStream(); }); + const fizzStream = await fizzPromise; const decoder = new TextDecoder(); const reader = fizzStream.getReader(); @@ -1464,16 +1532,18 @@ describe('ReactFlightDOMBrowser', () => { let postponed = null; - const stream = ReactServerDOMServer.renderToReadableStream( - - - , - null, - { - onPostpone(reason) { - postponed = reason; + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + + + , + null, + { + onPostpone(reason) { + postponed = reason; + }, }, - }, + ), ); const response = ReactServerDOMClient.createFromReadableStream(stream); @@ -1512,18 +1582,20 @@ describe('ReactFlightDOMBrowser', () => { return 'Done'; } const errors = []; - const stream = await ReactServerDOMServer.renderToReadableStream( -
- Loading
}> - - - , - null, - { - onError(x) { - errors.push(x.message); + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( +
+ Loading
}> + + + , + null, + { + onError(x) { + errors.push(x.message); + }, }, - }, + ), ); expect(rendered).toBe(false); @@ -1559,20 +1631,22 @@ describe('ReactFlightDOMBrowser', () => { let error = null; const controller = new AbortController(); - const stream = ReactServerDOMServer.renderToReadableStream( - - - , - null, - { - onError(x) { - error = x; - }, - onPostpone(reason) { - postponed = reason; + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + + + , + null, + { + onError(x) { + error = x; + }, + onPostpone(reason) { + postponed = reason; + }, + signal: controller.signal, }, - signal: controller.signal, - }, + ), ); try { @@ -1589,7 +1663,7 @@ describe('ReactFlightDOMBrowser', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await act(async () => { + await act(() => { root.render(
Shell: @@ -1643,27 +1717,33 @@ describe('ReactFlightDOMBrowser', () => { controller2 = c; }, }); - const rscStream = ReactServerDOMServer.renderToReadableStream( - { - s1, - s2, - }, - {}, - { - onError(x) { - errors.push(x); - return x; + const rscStream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + { + s1, + s2, }, - }, + {}, + { + onError(x) { + errors.push(x); + return x; + }, + }, + ), ); const result = await ReactServerDOMClient.createFromReadableStream( passThrough(rscStream), ); + const reader1 = result.s1.getReader(); const reader2 = result.s2.getReader(); - controller1.enqueue({hello: 'world'}); - controller2.enqueue({hi: 'there'}); + await serverAct(() => { + controller1.enqueue({hello: 'world'}); + controller2.enqueue({hi: 'there'}); + }); + expect(await reader1.read()).toEqual({ value: {hello: 'world'}, done: false, @@ -1673,10 +1753,11 @@ describe('ReactFlightDOMBrowser', () => { done: false, }); - controller1.enqueue('text1'); - controller2.enqueue('text2'); - controller1.close(); - controller2.error('rejected'); + await serverAct(async () => { + controller1.enqueue('text1'); + controller2.enqueue('text2'); + controller1.close(); + }); expect(await reader1.read()).toEqual({ value: 'text1', @@ -1690,6 +1771,9 @@ describe('ReactFlightDOMBrowser', () => { value: 'text2', done: false, }); + await serverAct(async () => { + controller2.error('rejected'); + }); let error = null; try { await reader2.read(); @@ -1713,14 +1797,16 @@ describe('ReactFlightDOMBrowser', () => { }, }); let loggedReason; - const rscStream = ReactServerDOMServer.renderToReadableStream( - s, - {}, - { - onError(reason) { - loggedReason = reason; + const rscStream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + s, + {}, + { + onError(reason) { + loggedReason = reason; + }, }, - }, + ), ); const reader = rscStream.getReader(); controller.enqueue('hi'); @@ -1745,21 +1831,25 @@ describe('ReactFlightDOMBrowser', () => { cancelReason = r; }, }); - const rscStream = ReactServerDOMServer.renderToReadableStream( - s, - {}, - { - signal: abortController.signal, - onError(x) { - errors.push(x); - return x.message; + + const rscStream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + s, + {}, + { + signal: abortController.signal, + onError(x) { + errors.push(x); + return x.message; + }, }, - }, + ), ); const result = await ReactServerDOMClient.createFromReadableStream( passThrough(rscStream), ); const reader = result.getReader(); + controller.enqueue('hi'); await 0; @@ -1808,18 +1898,20 @@ describe('ReactFlightDOMBrowser', () => { throw 'F'; })(); - const rscStream = ReactServerDOMServer.renderToReadableStream( - { - multiShotIterable, - singleShotIterator, - }, - {}, - { - onError(x) { - errors.push(x); - return x; + const rscStream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + { + multiShotIterable, + singleShotIterator, }, - }, + {}, + { + onError(x) { + errors.push(x); + return x; + }, + }, + ), ); const result = await ReactServerDOMClient.createFromReadableStream( passThrough(rscStream), @@ -1840,7 +1932,9 @@ describe('ReactFlightDOMBrowser', () => { done: false, }); - await resolve(); + await serverAct(() => { + resolve(); + }); expect(await iterator1.next()).toEqual({ value: {hi: 'B'}, @@ -1914,16 +2008,21 @@ describe('ReactFlightDOMBrowser', () => { yield 'c'; })(); let loggedReason; - const rscStream = ReactServerDOMServer.renderToReadableStream( - iterator, - {}, - { - onError(reason) { - loggedReason = reason; + + const rscStream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + iterator, + {}, + { + onError(reason) { + loggedReason = reason; + }, }, - }, + ), ); + const reader = rscStream.getReader(); + const reason = new Error('aborted'); reader.cancel(reason); await resolve(); @@ -1949,16 +2048,18 @@ describe('ReactFlightDOMBrowser', () => { } yield 'c'; })(); - const rscStream = ReactServerDOMServer.renderToReadableStream( - iterator, - {}, - { - signal: abortController.signal, - onError(x) { - errors.push(x); - return x.message; + const rscStream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + iterator, + {}, + { + signal: abortController.signal, + onError(x) { + errors.push(x); + return x.message; + }, }, - }, + ), ); const result = await ReactServerDOMClient.createFromReadableStream( passThrough(rscStream), @@ -1967,7 +2068,9 @@ describe('ReactFlightDOMBrowser', () => { const reason = new Error('aborted'); abortController.abort(reason); - await resolve(); + await serverAct(() => { + resolve(); + }); // We should be able to read the part we already emitted before the abort expect(await result.next()).toEqual({ diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js index df1850896d827..6f6a825e5e7de 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js @@ -9,13 +9,11 @@ 'use strict'; +import {patchSetImmediate} from '../../../../scripts/jest/patchSetImmediate'; + global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStream; -// Don't wait before processing work on the server. -// TODO: we can replace this with FlightServer.act(). -global.setImmediate = cb => cb(); - let clientExports; let webpackMap; let webpackModules; @@ -26,11 +24,17 @@ let ReactServerDOMServer; let ReactServerDOMClient; let Stream; let use; +let ReactServerScheduler; +let reactServerAct; describe('ReactFlightDOMNode', () => { beforeEach(() => { jest.resetModules(); + ReactServerScheduler = require('scheduler'); + patchSetImmediate(ReactServerScheduler); + reactServerAct = require('internal-test-utils').act; + // Simulate the condition resolution jest.mock('react', () => require('react/react.react-server')); jest.mock('react-server-dom-webpack/server', () => @@ -58,6 +62,17 @@ describe('ReactFlightDOMNode', () => { use = React.use; }); + async function serverAct(callback) { + let maybePromise; + await reactServerAct(() => { + maybePromise = callback(); + if (maybePromise && typeof maybePromise.catch === 'function') { + maybePromise.catch(() => {}); + } + }); + return maybePromise; + } + function readResult(stream) { return new Promise((resolve, reject) => { let buffer = ''; @@ -110,9 +125,8 @@ describe('ReactFlightDOMNode', () => { return ; } - const stream = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream(, webpackMap), ); const readable = new Stream.PassThrough(); let response; @@ -128,8 +142,8 @@ describe('ReactFlightDOMNode', () => { return use(response); } - const ssrStream = await ReactDOMServer.renderToPipeableStream( - , + const ssrStream = await serverAct(() => + ReactDOMServer.renderToPipeableStream(), ); const result = await readResult(ssrStream); expect(result).toEqual( @@ -140,9 +154,11 @@ describe('ReactFlightDOMNode', () => { it('should encode long string in a compact format', async () => { const testString = '"\n\t'.repeat(500) + '🙃'; - const stream = ReactServerDOMServer.renderToPipeableStream({ - text: testString, - }); + const stream = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream({ + text: testString, + }), + ); const readable = new Stream.PassThrough(); @@ -187,7 +203,9 @@ describe('ReactFlightDOMNode', () => { new BigUint64Array(buffer, 0), new DataView(buffer, 3), ]; - const stream = ReactServerDOMServer.renderToPipeableStream(buffers); + const stream = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream(buffers), + ); const readable = new Stream.PassThrough(); const promise = ReactServerDOMClient.createFromNodeStream(readable, { moduleMap: {}, @@ -232,9 +250,8 @@ describe('ReactFlightDOMNode', () => { return ; } - const stream = ReactServerDOMServer.renderToPipeableStream( - , - webpackMap, + const stream = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream(, webpackMap), ); const readable = new Stream.PassThrough(); let response; @@ -253,8 +270,8 @@ describe('ReactFlightDOMNode', () => { return use(response); } - const ssrStream = await ReactDOMServer.renderToPipeableStream( - , + const ssrStream = await serverAct(() => + ReactDOMServer.renderToPipeableStream(), ); const result = await readResult(ssrStream); expect(result).toEqual( @@ -275,14 +292,16 @@ describe('ReactFlightDOMNode', () => { }, }); - const rscStream = ReactServerDOMServer.renderToPipeableStream( - s, - {}, - { - onError(error) { - return error.message; + const rscStream = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + s, + {}, + { + onError(error) { + return error.message; + }, }, - }, + ), ); const writable = new Stream.PassThrough(); @@ -317,15 +336,17 @@ describe('ReactFlightDOMNode', () => { cancelReason = r; }, }); - const rscStream = ReactServerDOMServer.renderToPipeableStream( - s, - {}, - { - onError(x) { - errors.push(x); - return x.message; + const rscStream = await serverAct(() => + ReactServerDOMServer.renderToPipeableStream( + s, + {}, + { + onError(x) { + errors.push(x); + return x.message; + }, }, - }, + ), ); const readable = new Stream.PassThrough(); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js index bd92c88493fa8..30aa539e5ab5b 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js @@ -9,6 +9,8 @@ 'use strict'; +import {patchMessageChannel} from '../../../../scripts/jest/patchMessageChannel'; + // Polyfills for test environment global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStream; @@ -20,10 +22,17 @@ let webpackServerMap; let React; let ReactServerDOMServer; let ReactServerDOMClient; +let ReactServerScheduler; +let reactServerAct; describe('ReactFlightDOMReply', () => { beforeEach(() => { jest.resetModules(); + + ReactServerScheduler = require('scheduler'); + patchMessageChannel(ReactServerScheduler); + reactServerAct = require('internal-test-utils').act; + // Simulate the condition resolution jest.mock('react', () => require('react/react.react-server')); jest.mock('react-server-dom-webpack/server', () => @@ -39,6 +48,17 @@ describe('ReactFlightDOMReply', () => { ReactServerDOMClient = require('react-server-dom-webpack/client'); }); + async function serverAct(callback) { + let maybePromise; + await reactServerAct(() => { + maybePromise = callback(); + if (maybePromise && typeof maybePromise.catch === 'function') { + maybePromise.catch(() => {}); + } + }); + return maybePromise; + } + // This method should exist on File but is not implemented in JSDOM async function arrayBuffer(file) { return new Promise((resolve, reject) => { @@ -369,12 +389,10 @@ describe('ReactFlightDOMReply', () => { webpackServerMap, {temporaryReferences: temporaryReferencesServer}, ); - const stream = ReactServerDOMServer.renderToReadableStream( - serverPayload, - null, - { + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(serverPayload, null, { temporaryReferences: temporaryReferencesServer, - }, + }), ); const response = await ReactServerDOMClient.createFromReadableStream( stream, @@ -408,13 +426,15 @@ describe('ReactFlightDOMReply', () => { webpackServerMap, {temporaryReferences: temporaryReferencesServer}, ); - const stream = ReactServerDOMServer.renderToReadableStream( - { - root: serverPayload, - obj: serverPayload.obj, - }, - null, - {temporaryReferences: temporaryReferencesServer}, + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + { + root: serverPayload, + obj: serverPayload.obj, + }, + null, + {temporaryReferences: temporaryReferencesServer}, + ), ); const response = await ReactServerDOMClient.createFromReadableStream( stream, diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 790bda2457340..4231cfc146784 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -3506,9 +3506,14 @@ function enqueueFlush(request: Request): void { // happen when we start flowing again request.destination !== null ) { - const destination = request.destination; request.flushScheduled = true; - scheduleWork(() => flushCompletedChunks(request, destination)); + scheduleWork(() => { + request.flushScheduled = false; + const destination = request.destination; + if (destination) { + flushCompletedChunks(request, destination); + } + }); } } diff --git a/packages/react-server/src/ReactServerStreamConfigBrowser.js b/packages/react-server/src/ReactServerStreamConfigBrowser.js index f9371303844fa..a1f8a33d43e10 100644 --- a/packages/react-server/src/ReactServerStreamConfigBrowser.js +++ b/packages/react-server/src/ReactServerStreamConfigBrowser.js @@ -13,8 +13,18 @@ export type PrecomputedChunk = Uint8Array; export opaque type Chunk = Uint8Array; export type BinaryChunk = Uint8Array; +const channel = new MessageChannel(); +const taskQueue = []; +channel.port1.onmessage = () => { + const task = taskQueue.shift(); + if (task) { + task(); + } +}; + export function scheduleWork(callback: () => void) { - callback(); + taskQueue.push(callback); + channel.port2.postMessage(null); } export function flushBuffered(destination: Destination) { diff --git a/packages/react-server/src/ReactServerStreamConfigBun.js b/packages/react-server/src/ReactServerStreamConfigBun.js index 4686e0e970b12..36c94570ec91c 100644 --- a/packages/react-server/src/ReactServerStreamConfigBun.js +++ b/packages/react-server/src/ReactServerStreamConfigBun.js @@ -22,7 +22,7 @@ export opaque type Chunk = string; export type BinaryChunk = $ArrayBufferView; export function scheduleWork(callback: () => void) { - callback(); + setTimeout(callback, 0); } export function flushBuffered(destination: Destination) { diff --git a/packages/react-server/src/forks/ReactServerStreamConfig.dom-fb-experimental.js b/packages/react-server/src/forks/ReactServerStreamConfig.dom-fb-experimental.js index 03cc3e1b825be..86cd8d27712a7 100644 --- a/packages/react-server/src/forks/ReactServerStreamConfig.dom-fb-experimental.js +++ b/packages/react-server/src/forks/ReactServerStreamConfig.dom-fb-experimental.js @@ -42,8 +42,22 @@ export interface Destination { onError(error: mixed): void; } +function handleErrorInNextTick(error: any) { + setTimeout(() => { + throw error; + }); +} + +const LocalPromise = Promise; + +/** + * Since this environment doesn't have a way to schedule tasks from JS we schedule + * using a microtask instead. This isn't necessarily ideal since we would like to give + * other IO a chance to run before performing work typically but it's the best we can + * do in this environment + */ export function scheduleWork(callback: () => void) { - callback(); + LocalPromise.resolve().then(callback).catch(handleErrorInNextTick); } export function beginWriting(destination: Destination) { diff --git a/packages/react/src/__tests__/ReactMismatchedVersions-test.js b/packages/react/src/__tests__/ReactMismatchedVersions-test.js index cee86e5087d6b..602b71476d2af 100644 --- a/packages/react/src/__tests__/ReactMismatchedVersions-test.js +++ b/packages/react/src/__tests__/ReactMismatchedVersions-test.js @@ -9,6 +9,8 @@ 'use strict'; +import {patchMessageChannel} from '../../../../scripts/jest/patchMessageChannel'; + describe('ReactMismatchedVersions-test', () => { // Polyfills for test environment global.ReadableStream = @@ -20,6 +22,9 @@ describe('ReactMismatchedVersions-test', () => { beforeEach(() => { jest.resetModules(); + + patchMessageChannel(); + jest.mock('react', () => { const actualReact = jest.requireActual('react'); return { diff --git a/scripts/jest/patchMessageChannel.js b/scripts/jest/patchMessageChannel.js new file mode 100644 index 0000000000000..bbcc6690c529c --- /dev/null +++ b/scripts/jest/patchMessageChannel.js @@ -0,0 +1,30 @@ +'use strict'; + +export function patchMessageChannel(Scheduler) { + global.MessageChannel = class { + constructor() { + const port1 = { + onmesssage: () => {}, + }; + + this.port1 = port1; + + this.port2 = { + postMessage(msg) { + if (Scheduler) { + Scheduler.unstable_scheduleCallback( + Scheduler.unstable_NormalPriority, + () => { + port1.onmessage(msg); + } + ); + } else { + throw new Error( + 'MessageChannel patch was used without providing a Scheduler implementation. This is useful for tests that require this class to exist but are not actually utilizing the MessageChannel class. However it appears some test is trying to use this class so you should pass a Scheduler implemenation to the patch method' + ); + } + }, + }; + } + }; +} diff --git a/scripts/jest/patchSetImmediate.js b/scripts/jest/patchSetImmediate.js new file mode 100644 index 0000000000000..831314c664510 --- /dev/null +++ b/scripts/jest/patchSetImmediate.js @@ -0,0 +1,13 @@ +'use strict'; + +export function patchSetImmediate(Scheduler) { + if (!Scheduler) { + throw new Error( + 'setImmediate patch was used without providing a Scheduler implementation. If you are patching setImmediate you must provide a Scheduler.' + ); + } + + global.setImmediate = cb => { + Scheduler.unstable_scheduleCallback(Scheduler.unstable_NormalPriority, cb); + }; +} diff --git a/scripts/jest/setupEnvironment.js b/scripts/jest/setupEnvironment.js index 3b9f004bc2b82..44acb04f181a5 100644 --- a/scripts/jest/setupEnvironment.js +++ b/scripts/jest/setupEnvironment.js @@ -21,19 +21,6 @@ global.__EXPERIMENTAL__ = global.__VARIANT__ = !!process.env.VARIANT; if (typeof window !== 'undefined') { - global.requestIdleCallback = function (callback) { - return setTimeout(() => { - callback({ - timeRemaining() { - return Infinity; - }, - }); - }); - }; - - global.cancelIdleCallback = function (callbackID) { - clearTimeout(callbackID); - }; } else { global.AbortController = require('abortcontroller-polyfill/dist/cjs-ponyfill').AbortController; From 1e1e5cd25223fddbce0e3fb7889b06df0d93a950 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Thu, 6 Jun 2024 10:19:57 -0700 Subject: [PATCH 10/15] [Flight] Schedule work in a microtask (#29491) Stacked on #29551 Flight pings much more often than Fizz because async function components will always take at least a microtask to resolve . Rather than scheduling this work as a new macrotask Flight now schedules pings in a microtask. This allows more microtasks to ping before actually doing a work flush but doesn't force the vm to spin up a new task which is quite common give n the nature of Server Components --- .../server/ReactDOMLegacyServerStreamConfig.js | 8 ++++++++ .../src/ReactNoopFlightServer.js | 3 +++ .../react-noop-renderer/src/ReactNoopServer.js | 3 +++ packages/react-server/src/ReactFlightServer.js | 3 ++- .../src/ReactServerStreamConfigBrowser.js | 15 +++++++++++++++ .../src/ReactServerStreamConfigBun.js | 2 ++ .../src/ReactServerStreamConfigEdge.js | 15 +++++++++++++++ .../src/ReactServerStreamConfigNode.js | 2 ++ .../src/forks/ReactServerStreamConfig.custom.js | 1 + ...ReactServerStreamConfig.dom-fb-experimental.js | 2 ++ .../src/forks/ReactServerStreamConfig.dom-fb.js | 4 ++++ 11 files changed, 57 insertions(+), 1 deletion(-) diff --git a/packages/react-dom-bindings/src/server/ReactDOMLegacyServerStreamConfig.js b/packages/react-dom-bindings/src/server/ReactDOMLegacyServerStreamConfig.js index 5fa0c88d13181..4b940731b99b0 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMLegacyServerStreamConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMLegacyServerStreamConfig.js @@ -20,6 +20,14 @@ export function scheduleWork(callback: () => void) { callback(); } +export function scheduleMicrotask(callback: () => void) { + // While this defies the method name the legacy builds have special + // overrides that make work scheduling sync. At the moment scheduleMicrotask + // isn't used by any legacy APIs so this is somewhat academic but if they + // did in the future we'd probably want to have this be in sync with scheduleWork + callback(); +} + export function flushBuffered(destination: Destination) {} export function beginWriting(destination: Destination) {} diff --git a/packages/react-noop-renderer/src/ReactNoopFlightServer.js b/packages/react-noop-renderer/src/ReactNoopFlightServer.js index 983ae748e0ea7..cf6f24404c3ed 100644 --- a/packages/react-noop-renderer/src/ReactNoopFlightServer.js +++ b/packages/react-noop-renderer/src/ReactNoopFlightServer.js @@ -25,6 +25,9 @@ type Destination = Array; const textEncoder = new TextEncoder(); const ReactNoopFlightServer = ReactFlightServer({ + scheduleMicrotask(callback: () => void) { + callback(); + }, scheduleWork(callback: () => void) { callback(); }, diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index 7d739d3178836..4e2832e4f2bfe 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -74,6 +74,9 @@ function write(destination: Destination, buffer: Uint8Array): void { } const ReactNoopServer = ReactFizzServer({ + scheduleMicrotask(callback: () => void) { + callback(); + }, scheduleWork(callback: () => void) { callback(); }, diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 4231cfc146784..2622b4e15cc8f 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -26,6 +26,7 @@ import {enableFlightReadableStream} from 'shared/ReactFeatureFlags'; import { scheduleWork, + scheduleMicrotask, flushBuffered, beginWriting, writeChunkAndReturn, @@ -1571,7 +1572,7 @@ function pingTask(request: Request, task: Task): void { pingedTasks.push(task); if (pingedTasks.length === 1) { request.flushScheduled = request.destination !== null; - scheduleWork(() => performWork(request)); + scheduleMicrotask(() => performWork(request)); } } diff --git a/packages/react-server/src/ReactServerStreamConfigBrowser.js b/packages/react-server/src/ReactServerStreamConfigBrowser.js index a1f8a33d43e10..2e68ca7117544 100644 --- a/packages/react-server/src/ReactServerStreamConfigBrowser.js +++ b/packages/react-server/src/ReactServerStreamConfigBrowser.js @@ -27,6 +27,21 @@ export function scheduleWork(callback: () => void) { channel.port2.postMessage(null); } +function handleErrorInNextTick(error: any) { + setTimeout(() => { + throw error; + }); +} + +const LocalPromise = Promise; + +export const scheduleMicrotask: (callback: () => void) => void = + typeof queueMicrotask === 'function' + ? queueMicrotask + : callback => { + LocalPromise.resolve(null).then(callback).catch(handleErrorInNextTick); + }; + export function flushBuffered(destination: Destination) { // WHATWG Streams do not yet have a way to flush the underlying // transform streams. https://github.com/whatwg/streams/issues/960 diff --git a/packages/react-server/src/ReactServerStreamConfigBun.js b/packages/react-server/src/ReactServerStreamConfigBun.js index 36c94570ec91c..81f86a50b7b25 100644 --- a/packages/react-server/src/ReactServerStreamConfigBun.js +++ b/packages/react-server/src/ReactServerStreamConfigBun.js @@ -25,6 +25,8 @@ export function scheduleWork(callback: () => void) { setTimeout(callback, 0); } +export const scheduleMicrotask = queueMicrotask; + export function flushBuffered(destination: Destination) { // Bun direct streams provide a flush function. // If we don't have any more data to send right now. diff --git a/packages/react-server/src/ReactServerStreamConfigEdge.js b/packages/react-server/src/ReactServerStreamConfigEdge.js index e77dc28284a18..22f165ded94c1 100644 --- a/packages/react-server/src/ReactServerStreamConfigEdge.js +++ b/packages/react-server/src/ReactServerStreamConfigEdge.js @@ -13,6 +13,21 @@ export type PrecomputedChunk = Uint8Array; export opaque type Chunk = Uint8Array; export type BinaryChunk = Uint8Array; +function handleErrorInNextTick(error: any) { + setTimeout(() => { + throw error; + }); +} + +const LocalPromise = Promise; + +export const scheduleMicrotask: (callback: () => void) => void = + typeof queueMicrotask === 'function' + ? queueMicrotask + : callback => { + LocalPromise.resolve(null).then(callback).catch(handleErrorInNextTick); + }; + export function scheduleWork(callback: () => void) { setTimeout(callback, 0); } diff --git a/packages/react-server/src/ReactServerStreamConfigNode.js b/packages/react-server/src/ReactServerStreamConfigNode.js index cbd366ab54ba3..773c998610df0 100644 --- a/packages/react-server/src/ReactServerStreamConfigNode.js +++ b/packages/react-server/src/ReactServerStreamConfigNode.js @@ -26,6 +26,8 @@ export function scheduleWork(callback: () => void) { setImmediate(callback); } +export const scheduleMicrotask = queueMicrotask; + export function flushBuffered(destination: Destination) { // If we don't have any more data to send right now. // Flush whatever is in the buffer to the wire. diff --git a/packages/react-server/src/forks/ReactServerStreamConfig.custom.js b/packages/react-server/src/forks/ReactServerStreamConfig.custom.js index 22cd6551c0b28..a9799cb7ba190 100644 --- a/packages/react-server/src/forks/ReactServerStreamConfig.custom.js +++ b/packages/react-server/src/forks/ReactServerStreamConfig.custom.js @@ -31,6 +31,7 @@ export opaque type Chunk = mixed; // eslint-disable-line no-undef export opaque type BinaryChunk = mixed; // eslint-disable-line no-undef export const scheduleWork = $$$config.scheduleWork; +export const scheduleMicrotask = $$$config.scheduleMicrotask; export const beginWriting = $$$config.beginWriting; export const writeChunk = $$$config.writeChunk; export const writeChunkAndReturn = $$$config.writeChunkAndReturn; diff --git a/packages/react-server/src/forks/ReactServerStreamConfig.dom-fb-experimental.js b/packages/react-server/src/forks/ReactServerStreamConfig.dom-fb-experimental.js index 86cd8d27712a7..2d705e2a1cdb7 100644 --- a/packages/react-server/src/forks/ReactServerStreamConfig.dom-fb-experimental.js +++ b/packages/react-server/src/forks/ReactServerStreamConfig.dom-fb-experimental.js @@ -60,6 +60,8 @@ export function scheduleWork(callback: () => void) { LocalPromise.resolve().then(callback).catch(handleErrorInNextTick); } +export const scheduleMicrotask: (callback: () => void) => void = scheduleWork; + export function beginWriting(destination: Destination) { destination.beginWriting(); } diff --git a/packages/react-server/src/forks/ReactServerStreamConfig.dom-fb.js b/packages/react-server/src/forks/ReactServerStreamConfig.dom-fb.js index e15f6808673c3..12ed6ba59852e 100644 --- a/packages/react-server/src/forks/ReactServerStreamConfig.dom-fb.js +++ b/packages/react-server/src/forks/ReactServerStreamConfig.dom-fb.js @@ -9,6 +9,10 @@ export * from '../ReactServerStreamConfigFB'; +export function scheduleMicrotask(callback: () => void) { + // We don't schedule work in this model, and instead expect performWork to always be called repeatedly. +} + export function scheduleWork(callback: () => void) { // We don't schedule work in this model, and instead expect performWork to always be called repeatedly. } From 70194be4038158f5ba8e55e27f7ffd02be13bbca Mon Sep 17 00:00:00 2001 From: XiaoPi <530257315@qq.com> Date: Fri, 7 Jun 2024 01:48:24 +0800 Subject: [PATCH 11/15] fix: reread the testfilter file if filter enabled during the watch process (#29775) Resolve #29720 In the above PR, I overlooked that we can change the filter mode during the watch process. Now it's fixed. --- compiler/packages/snap/src/runner-watch.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/packages/snap/src/runner-watch.ts b/compiler/packages/snap/src/runner-watch.ts index bebedef721600..414d99084c52e 100644 --- a/compiler/packages/snap/src/runner-watch.ts +++ b/compiler/packages/snap/src/runner-watch.ts @@ -189,7 +189,7 @@ function subscribeKeyEvents( state: RunnerState, onChange: (state: RunnerState) => void ) { - process.stdin.on("keypress", (str, key) => { + process.stdin.on("keypress", async (str, key) => { if (key.name === "u") { // u => update fixtures state.mode.action = RunnerAction.Update; @@ -197,6 +197,7 @@ function subscribeKeyEvents( process.exit(0); } else if (key.name === "f") { state.mode.filter = !state.mode.filter; + state.filter = state.mode.filter ? await readTestFilter() : null; state.mode.action = RunnerAction.Test; } else { // any other key re-runs tests From 29b12787902acff714466e5eb656a7ab0f978836 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Thu, 6 Jun 2024 13:38:54 -0400 Subject: [PATCH 12/15] [compiler] Check for __DEV__ for FastRefresh We don't always have the NODE_ENV set, so additionally check for the __DEV__ global if it has one set. ghstack-source-id: 3719a4710a5fb1b4abf511f469c815917b7dfdf4 Pull Request resolved: https://github.com/facebook/react/pull/29785 --- .../babel-plugin-react-compiler/src/Babel/BabelPlugin.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts b/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts index 0945f178c362d..64a5816048dd6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts @@ -30,13 +30,16 @@ export default function BabelPluginReactCompiler( */ Program(prog, pass): void { let opts = parsePluginOptions(pass.opts); + const isDev = + (typeof __DEV__ !== "undefined" && __DEV__ === true) || + process.env["NODE_ENV"] === "development"; if ( opts.enableReanimatedCheck === true && pipelineUsesReanimatedPlugin(pass.file.opts.plugins) ) { opts = injectReanimatedFlag(opts); } - if (process.env["NODE_ENV"] === "development") { + if (isDev) { opts = { ...opts, environment: { From 90499a730ed53c29c2321faaf19e77b4ebeeada4 Mon Sep 17 00:00:00 2001 From: Ricky Date: Thu, 6 Jun 2024 14:07:16 -0400 Subject: [PATCH 13/15] Fix RN version string in builds (#29787) The version was set for React but not the renderers --- scripts/rollup/build-all-release-channels.js | 54 +++++++++++++++++--- 1 file changed, 48 insertions(+), 6 deletions(-) diff --git a/scripts/rollup/build-all-release-channels.js b/scripts/rollup/build-all-release-channels.js index 2a6c626cf4a21..5e8cd27cf5e47 100644 --- a/scripts/rollup/build-all-release-channels.js +++ b/scripts/rollup/build-all-release-channels.js @@ -168,12 +168,19 @@ function processStable(buildDir) { ); } + const rnVersionString = + ReactVersion + '-native-fb-' + sha + '-' + dateString; if (fs.existsSync(buildDir + '/facebook-react-native')) { - const versionString = - ReactVersion + '-native-fb-' + sha + '-' + dateString; updatePlaceholderReactVersionInCompiledArtifacts( buildDir + '/facebook-react-native', - versionString + rnVersionString + ); + } + + if (fs.existsSync(buildDir + '/react-native')) { + updatePlaceholderReactVersionInCompiledArtifactsFb( + buildDir + '/react-native', + rnVersionString ); } @@ -265,17 +272,24 @@ function processExperimental(buildDir, version) { fs.writeFileSync(buildDir + '/facebook-www/VERSION_MODERN', versionString); } + const rnVersionString = ReactVersion + '-native-fb-' + sha + '-' + dateString; if (fs.existsSync(buildDir + '/facebook-react-native')) { - const versionString = ReactVersion + '-native-fb-' + sha + '-' + dateString; updatePlaceholderReactVersionInCompiledArtifacts( buildDir + '/facebook-react-native', - versionString + rnVersionString ); // Also save a file with the version number fs.writeFileSync( buildDir + '/facebook-react-native/VERSION_NATIVE_FB', - versionString + rnVersionString + ); + } + + if (fs.existsSync(buildDir + '/react-native')) { + updatePlaceholderReactVersionInCompiledArtifactsFb( + buildDir + '/react-native', + rnVersionString ); } @@ -396,6 +410,34 @@ function updatePlaceholderReactVersionInCompiledArtifacts( } } +function updatePlaceholderReactVersionInCompiledArtifactsFb( + artifactsDirectory, + newVersion +) { + // Update the version of React in the compiled artifacts by searching for + // the placeholder string and replacing it with a new one. + const artifactFilenames = String( + spawnSync('grep', [ + '-lr', + PLACEHOLDER_REACT_VERSION, + '--', + artifactsDirectory, + ]).stdout + ) + .trim() + .split('\n') + .filter(filename => filename.endsWith('.fb.js')); + + for (const artifactFilename of artifactFilenames) { + const originalText = fs.readFileSync(artifactFilename, 'utf8'); + const replacedText = originalText.replaceAll( + PLACEHOLDER_REACT_VERSION, + newVersion + ); + fs.writeFileSync(artifactFilename, replacedText); + } +} + /** * cross-platform alternative to `rsync -ar` * @param {string} source From fe5ce4e3e969aca4705b9973a6fdb5f132e03025 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 6 Jun 2024 20:01:15 +0100 Subject: [PATCH 14/15] =?UTF-8?q?fix[react-devtools/store-test]:=20fork=20?= =?UTF-8?q?the=20test=20to=20represent=20current=20be=E2=80=A6=20(#29777)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary The test started to fail after https://github.com/facebook/react/pull/29088. Fork the test and the expected store state for: - React 18.x, to represent the previous behavior - React >= 19, to represent the current RDT behavior, where error can't be connected to the fiber, because it was not yet mounted and shared with DevTools. Ideally, DevTools should start keeping track of such fibers, but also distinguish them from some that haven't mounted due to Suspense or error boundaries. --- .../src/__tests__/store-test.js | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index 565d67067850f..c6ce366df04b2 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -1915,8 +1915,12 @@ describe('Store', () => { }); }); - // @reactVersion >= 18.0 - it('from react get counted', () => { + // In React 19, JSX warnings were moved into the renderer - https://github.com/facebook/react/pull/29088 + // When the error is emitted, the source fiber of this error is not yet mounted + // So DevTools can't connect the error and the fiber + // TODO(hoxyq): update RDT to keep track of such fibers + // @reactVersion >= 19.0 + it('from react get counted [React >= 19]', () => { function Example() { return []; } @@ -1938,6 +1942,31 @@ describe('Store', () => { `); }); + // @reactVersion >= 18.0 + // @reactVersion < 19.0 + it('from react get counted [React 18.x]', () => { + function Example() { + return []; + } + function Child() { + return null; + } + + withErrorsOrWarningsIgnored( + ['Warning: Each child in a list should have a unique "key" prop'], + () => { + act(() => render()); + }, + ); + + expect(store).toMatchInlineSnapshot(` + ✕ 1, ⚠ 0 + [root] + ▾ ✕ + + `); + }); + // @reactVersion >= 18.0 it('can be cleared for the whole app', () => { function Example() { From c4b433f8cb31d6f73d4a800fcf11ed55c8689daf Mon Sep 17 00:00:00 2001 From: Josh Story Date: Thu, 6 Jun 2024 14:41:27 -0700 Subject: [PATCH 15/15] [Flight] Allow aborting during render (#29764) Stacked on #29491 Previously if you aborted during a render the currently rendering task would itself be aborted which will cause the entire model to be replaced by the aborted error rather than just the slot currently being rendered. This change updates the abort logic to mark currently rendering tasks as aborted but allowing the current render to emit a partially serialized model with an error reference in place of the current model. The intent is to support aborting from rendering synchronously, in microtasks (after an await or in a .then) and in lazy initializers. We don't specifically support aborting from things like proxies that might be triggered during serialization of props --- .../src/__tests__/ReactFlightDOM-test.js | 587 +++++++++++++++++- .../react-server/src/ReactFlightServer.js | 114 +++- scripts/error-codes/codes.json | 3 +- 3 files changed, 675 insertions(+), 29 deletions(-) diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js index 1ead6efe4b25a..3bf8e02e0f687 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -36,6 +36,7 @@ let ErrorBoundary; let JSDOM; let ReactServerScheduler; let reactServerAct; +let assertConsoleErrorDev; describe('ReactFlightDOM', () => { beforeEach(() => { @@ -70,6 +71,8 @@ describe('ReactFlightDOM', () => { __unmockReact(); jest.resetModules(); act = require('internal-test-utils').act; + assertConsoleErrorDev = + require('internal-test-utils').assertConsoleErrorDev; Stream = require('stream'); React = require('react'); use = React.use; @@ -107,6 +110,38 @@ describe('ReactFlightDOM', () => { return maybePromise; } + async function readInto( + container: Document | HTMLElement, + stream: ReadableStream, + ) { + const reader = stream.getReader(); + const decoder = new TextDecoder(); + let content = ''; + while (true) { + const {done, value} = await reader.read(); + if (done) { + content += decoder.decode(); + break; + } + content += decoder.decode(value, {stream: true}); + } + if (container.nodeType === 9 /* DOCUMENT */) { + const doc = new JSDOM(content).window.document; + container.documentElement.innerHTML = doc.documentElement.innerHTML; + while (container.documentElement.attributes.length > 0) { + container.documentElement.removeAttribute( + container.documentElement.attributes[0].name, + ); + } + const attrs = doc.documentElement.attributes; + for (let i = 0; i < attrs.length; i++) { + container.documentElement.setAttribute(attrs[i].name, attrs[i].value); + } + } else { + container.innerHTML = content; + } + } + function getTestStream() { const writable = new Stream.PassThrough(); const readable = new ReadableStream({ @@ -1633,20 +1668,8 @@ describe('ReactFlightDOM', () => { ReactDOMFizzServer.renderToPipeableStream().pipe(fizzWritable); }); - const decoder = new TextDecoder(); - const reader = fizzReadable.getReader(); - let content = ''; - while (true) { - const {done, value} = await reader.read(); - if (done) { - content += decoder.decode(); - break; - } - content += decoder.decode(value, {stream: true}); - } - - const doc = new JSDOM(content).window.document; - expect(getMeaningfulChildren(doc)).toEqual( + await readInto(document, fizzReadable); + expect(getMeaningfulChildren(document)).toEqual( @@ -1912,4 +1935,540 @@ describe('ReactFlightDOM', () => { }); expect(container.innerHTML).toBe('Hello World'); }); + + it('can abort synchronously during render', async () => { + function Sibling() { + return

sibling

; + } + + function App() { + return ( +
+ loading 1...

}> + + +
+ loading 2...

}> + +
+
+ loading 3...

}> +
+ +
+
+
+
+ ); + } + + const abortRef = {current: null}; + function ComponentThatAborts() { + abortRef.current(); + return

hello world

; + } + + const {writable: flightWritable, readable: flightReadable} = + getTestStream(); + + await serverAct(() => { + const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ); + abortRef.current = abort; + pipe(flightWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + ]); + + const response = + ReactServerDOMClient.createFromReadableStream(flightReadable); + + const {writable: fizzWritable, readable: fizzReadable} = getTestStream(); + + function ClientApp() { + return use(response); + } + + const shellErrors = []; + await serverAct(async () => { + ReactDOMFizzServer.renderToPipeableStream( + React.createElement(ClientApp), + { + onShellError(error) { + shellErrors.push(error.message); + }, + }, + ).pipe(fizzWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + ]); + + expect(shellErrors).toEqual([]); + + const container = document.createElement('div'); + await readInto(container, fizzReadable); + expect(getMeaningfulChildren(container)).toEqual( +
+

loading 1...

+

loading 2...

+
+

loading 3...

+
+
, + ); + }); + + it('can abort during render in an async tick', async () => { + async function Sibling() { + return

sibling

; + } + + function App() { + return ( +
+ loading 1...

}> + + +
+ loading 2...

}> + +
+
+ loading 3...

}> +
+ +
+
+
+
+ ); + } + + const abortRef = {current: null}; + async function ComponentThatAborts() { + await 1; + abortRef.current(); + return

hello world

; + } + + const {writable: flightWritable, readable: flightReadable} = + getTestStream(); + + await serverAct(() => { + const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ); + abortRef.current = abort; + pipe(flightWritable); + }); + + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + ]); + + const response = + ReactServerDOMClient.createFromReadableStream(flightReadable); + + const {writable: fizzWritable, readable: fizzReadable} = getTestStream(); + + function ClientApp() { + return use(response); + } + + const shellErrors = []; + await serverAct(async () => { + ReactDOMFizzServer.renderToPipeableStream( + React.createElement(ClientApp), + { + onShellError(error) { + shellErrors.push(error.message); + }, + }, + ).pipe(fizzWritable); + }); + + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + ]); + + expect(shellErrors).toEqual([]); + + const container = document.createElement('div'); + await readInto(container, fizzReadable); + expect(getMeaningfulChildren(container)).toEqual( +
+

loading 1...

+

loading 2...

+
+

loading 3...

+
+
, + ); + }); + + it('can abort during render in a lazy initializer for a component', async () => { + function Sibling() { + return

sibling

; + } + + function App() { + return ( +
+ loading 1...

}> + +
+ loading 2...

}> + +
+
+ loading 3...

}> +
+ +
+
+
+
+ ); + } + + const abortRef = {current: null}; + const LazyAbort = React.lazy(() => { + abortRef.current(); + return { + then(cb) { + cb({default: 'div'}); + }, + }; + }); + + const {writable: flightWritable, readable: flightReadable} = + getTestStream(); + + await serverAct(() => { + const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ); + abortRef.current = abort; + pipe(flightWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + ]); + + const response = + ReactServerDOMClient.createFromReadableStream(flightReadable); + + const {writable: fizzWritable, readable: fizzReadable} = getTestStream(); + + function ClientApp() { + return use(response); + } + + const shellErrors = []; + await serverAct(async () => { + ReactDOMFizzServer.renderToPipeableStream( + React.createElement(ClientApp), + { + onShellError(error) { + shellErrors.push(error.message); + }, + }, + ).pipe(fizzWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + ]); + + expect(shellErrors).toEqual([]); + + const container = document.createElement('div'); + await readInto(container, fizzReadable); + expect(getMeaningfulChildren(container)).toEqual( +
+

loading 1...

+

loading 2...

+
+

loading 3...

+
+
, + ); + }); + + it('can abort during render in a lazy initializer for an element', async () => { + function Sibling() { + return

sibling

; + } + + function App() { + return ( +
+ loading 1...

}>{lazyAbort}
+ loading 2...

}> + +
+
+ loading 3...

}> +
+ +
+
+
+
+ ); + } + + const abortRef = {current: null}; + const lazyAbort = React.lazy(() => { + abortRef.current(); + return { + then(cb) { + cb({default: 'hello world'}); + }, + }; + }); + + const {writable: flightWritable, readable: flightReadable} = + getTestStream(); + + await serverAct(() => { + const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ); + abortRef.current = abort; + pipe(flightWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + ]); + + const response = + ReactServerDOMClient.createFromReadableStream(flightReadable); + + const {writable: fizzWritable, readable: fizzReadable} = getTestStream(); + + function ClientApp() { + return use(response); + } + + const shellErrors = []; + await serverAct(async () => { + ReactDOMFizzServer.renderToPipeableStream( + React.createElement(ClientApp), + { + onShellError(error) { + shellErrors.push(error.message); + }, + }, + ).pipe(fizzWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + ]); + + expect(shellErrors).toEqual([]); + + const container = document.createElement('div'); + await readInto(container, fizzReadable); + expect(getMeaningfulChildren(container)).toEqual( +
+

loading 1...

+

loading 2...

+
+

loading 3...

+
+
, + ); + }); + + it('can abort during a synchronous thenable resolution', async () => { + function Sibling() { + return

sibling

; + } + + function App() { + return ( +
+ loading 1...

}>{thenable}
+ loading 2...

}> + +
+
+ loading 3...

}> +
+ +
+
+
+
+ ); + } + + const abortRef = {current: null}; + const thenable = { + then(cb) { + abortRef.current(); + cb(thenable.value); + }, + }; + + const {writable: flightWritable, readable: flightReadable} = + getTestStream(); + + await serverAct(() => { + const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ); + abortRef.current = abort; + pipe(flightWritable); + }); + + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + ]); + + const response = + ReactServerDOMClient.createFromReadableStream(flightReadable); + + const {writable: fizzWritable, readable: fizzReadable} = getTestStream(); + + function ClientApp() { + return use(response); + } + + const shellErrors = []; + await serverAct(async () => { + ReactDOMFizzServer.renderToPipeableStream( + React.createElement(ClientApp), + { + onShellError(error) { + shellErrors.push(error.message); + }, + }, + ).pipe(fizzWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + ]); + + expect(shellErrors).toEqual([]); + + const container = document.createElement('div'); + await readInto(container, fizzReadable); + expect(getMeaningfulChildren(container)).toEqual( +
+

loading 1...

+

loading 2...

+
+

loading 3...

+
+
, + ); + }); + + it('wont serialize thenables that were not already settled by the time an abort happens', async () => { + function App() { + return ( +
+ loading 1...

}> + +
+ loading 2...

}>{thenable1}
+
+ loading 3...

}>{thenable2}
+
+
+ ); + } + + const abortRef = {current: null}; + const thenable1 = { + then(cb) { + cb('hello world'); + }, + }; + + const thenable2 = { + then(cb) { + cb('hello world'); + }, + status: 'fulfilled', + value: 'hello world', + }; + + function ComponentThatAborts() { + abortRef.current(); + return thenable1; + } + + const {writable: flightWritable, readable: flightReadable} = + getTestStream(); + + await serverAct(() => { + const {pipe, abort} = ReactServerDOMServer.renderToPipeableStream( + , + webpackMap, + ); + abortRef.current = abort; + pipe(flightWritable); + }); + + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + ]); + + const response = + ReactServerDOMClient.createFromReadableStream(flightReadable); + + const {writable: fizzWritable, readable: fizzReadable} = getTestStream(); + + function ClientApp() { + return use(response); + } + + const shellErrors = []; + await serverAct(async () => { + ReactDOMFizzServer.renderToPipeableStream( + React.createElement(ClientApp), + { + onShellError(error) { + shellErrors.push(error.message); + }, + }, + ).pipe(fizzWritable); + }); + assertConsoleErrorDev([ + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', + ]); + + expect(shellErrors).toEqual([]); + + const container = document.createElement('div'); + await readInto(container, fizzReadable); + expect(getMeaningfulChildren(container)).toEqual( +
+

loading 1...

+

loading 2...

+
hello world
+
, + ); + }); }); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 2622b4e15cc8f..11b558c592abf 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -381,10 +381,11 @@ const PENDING = 0; const COMPLETED = 1; const ABORTED = 3; const ERRORED = 4; +const RENDERING = 5; type Task = { id: number, - status: 0 | 1 | 3 | 4, + status: 0 | 1 | 3 | 4 | 5, model: ReactClientValue, ping: () => void, toJSON: (key: string, value: ReactClientValue) => ReactJSONValue, @@ -396,7 +397,7 @@ type Task = { interface Reference {} export type Request = { - status: 0 | 1 | 2, + status: 0 | 1 | 2 | 3, flushScheduled: boolean, fatalError: mixed, destination: null | Destination, @@ -427,6 +428,8 @@ export type Request = { didWarnForKey: null | WeakSet, }; +const AbortSigil = {}; + const { TaintRegistryObjects, TaintRegistryValues, @@ -466,8 +469,9 @@ function defaultPostponeHandler(reason: string) { } const OPEN = 0; -const CLOSING = 1; -const CLOSED = 2; +const ABORTING = 1; +const CLOSING = 2; +const CLOSED = 3; export function createRequest( model: ReactClientValue, @@ -556,7 +560,6 @@ function serializeThenable( task.implicitSlot, request.abortableTasks, ); - if (__DEV__) { // If this came from Flight, forward any debug info into this new row. const debugInfo: ?ReactDebugInfo = (thenable: any)._debugInfo; @@ -590,6 +593,15 @@ function serializeThenable( return newTask.id; } default: { + if (request.status === ABORTING) { + // We can no longer accept any resolved values + newTask.status = ABORTED; + const errorId: number = (request.fatalError: any); + const model = stringify(serializeByValueID(errorId)); + emitModelChunk(request, newTask.id, model); + request.abortableTasks.delete(newTask); + return newTask.id; + } if (typeof thenable.status === 'string') { // Only instrument the thenable if the status if not defined. If // it's defined, but an unknown value, assume it's been instrumented by @@ -1046,6 +1058,14 @@ function renderFunctionComponent( const secondArg = undefined; result = Component(props, secondArg); } + + if (request.status === ABORTING) { + // If we aborted during rendering we should interrupt the render but + // we don't need to provide an error because the renderer will encode + // the abort error as the reason. + throw AbortSigil; + } + if ( typeof result === 'object' && result !== null && @@ -1523,6 +1543,12 @@ function renderElement( const init = type._init; wrappedType = init(payload); } + if (request.status === ABORTING) { + // lazy initializers are user code and could abort during render + // we don't wan to return any value resolved from the lazy initializer + // if it aborts so we interrupt rendering here + throw AbortSigil; + } return renderElement( request, task, @@ -1942,6 +1968,15 @@ function renderModel( try { return renderModelDestructive(request, task, parent, key, value); } catch (thrownValue) { + // If the suspended/errored value was an element or lazy it can be reduced + // to a lazy reference, so that it doesn't error the parent. + const model = task.model; + const wasReactNode = + typeof model === 'object' && + model !== null && + ((model: any).$$typeof === REACT_ELEMENT_TYPE || + (model: any).$$typeof === REACT_LAZY_TYPE); + const x = thrownValue === SuspenseException ? // This is a special type of exception used for Suspense. For historical @@ -1951,17 +1986,18 @@ function renderModel( // later, once we deprecate the old API in favor of `use`. getSuspendedThenable() : thrownValue; - // If the suspended/errored value was an element or lazy it can be reduced - // to a lazy reference, so that it doesn't error the parent. - const model = task.model; - const wasReactNode = - typeof model === 'object' && - model !== null && - ((model: any).$$typeof === REACT_ELEMENT_TYPE || - (model: any).$$typeof === REACT_LAZY_TYPE); + if (typeof x === 'object' && x !== null) { // $FlowFixMe[method-unbinding] if (typeof x.then === 'function') { + if (request.status === ABORTING) { + task.status = ABORTED; + const errorId: number = (request.fatalError: any); + if (wasReactNode) { + return serializeLazyID(errorId); + } + return serializeByValueID(errorId); + } // Something suspended, we'll need to create a new task and resolve it later. const newTask = createTask( request, @@ -2004,6 +2040,15 @@ function renderModel( } } + if (thrownValue === AbortSigil) { + task.status = ABORTED; + const errorId: number = (request.fatalError: any); + if (wasReactNode) { + return serializeLazyID(errorId); + } + return serializeByValueID(errorId); + } + // Restore the context. We assume that this will be restored by the inner // functions in case nothing throws so we don't use "finally" here. task.keyPath = prevKeyPath; @@ -2147,6 +2192,12 @@ function renderModelDestructive( const init = lazy._init; resolvedModel = init(payload); } + if (request.status === ABORTING) { + // lazy initializers are user code and could abort during render + // we don't wan to return any value resolved from the lazy initializer + // if it aborts so we interrupt rendering here + throw AbortSigil; + } if (__DEV__) { const debugInfo: ?ReactDebugInfo = lazy._debugInfo; if (debugInfo) { @@ -3262,6 +3313,7 @@ function retryTask(request: Request, task: Task): void { } const prevDebugID = debugID; + task.status = RENDERING; try { // Track the root so we know that we have to emit this object even though it @@ -3328,10 +3380,19 @@ function retryTask(request: Request, task: Task): void { if (typeof x === 'object' && x !== null) { // $FlowFixMe[method-unbinding] if (typeof x.then === 'function') { + if (request.status === ABORTING) { + request.abortableTasks.delete(task); + task.status = ABORTED; + const errorId: number = (request.fatalError: any); + const model = stringify(serializeByValueID(errorId)); + emitModelChunk(request, task.id, model); + return; + } // Something suspended again, let's pick it back up later. + task.status = PENDING; + task.thenableState = getThenableStateAfterSuspending(); const ping = task.ping; x.then(ping, ping); - task.thenableState = getThenableStateAfterSuspending(); return; } else if (enablePostpone && x.$$typeof === REACT_POSTPONE_TYPE) { request.abortableTasks.delete(task); @@ -3342,6 +3403,16 @@ function retryTask(request: Request, task: Task): void { return; } } + + if (x === AbortSigil) { + request.abortableTasks.delete(task); + task.status = ABORTED; + const errorId: number = (request.fatalError: any); + const model = stringify(serializeByValueID(errorId)); + emitModelChunk(request, task.id, model); + return; + } + request.abortableTasks.delete(task); task.status = ERRORED; const digest = logRecoverableError(request, x); @@ -3399,6 +3470,10 @@ function performWork(request: Request): void { } function abortTask(task: Task, request: Request, errorId: number): void { + if (task.status === RENDERING) { + // This task will be aborted by the render + return; + } task.status = ABORTED; // Instead of emitting an error per task.id, we emit a model that only // has a single value referencing the error. @@ -3484,6 +3559,7 @@ function flushCompletedChunks( if (enableTaint) { cleanupTaintQueue(request); } + request.status = CLOSED; close(destination); request.destination = null; } @@ -3547,12 +3623,14 @@ export function stopFlowing(request: Request): void { // This is called to early terminate a request. It creates an error at all pending tasks. export function abort(request: Request, reason: mixed): void { try { + request.status = ABORTING; const abortableTasks = request.abortableTasks; // We have tasks to abort. We'll emit one error row and then emit a reference // to that row from every row that's still remaining. if (abortableTasks.size > 0) { request.pendingChunks++; const errorId = request.nextChunkId++; + request.fatalError = errorId; if ( enablePostpone && typeof reason === 'object' && @@ -3568,6 +3646,10 @@ export function abort(request: Request, reason: mixed): void { ? new Error( 'The render was aborted by the server without a reason.', ) + : typeof reason === 'object' && + reason !== null && + typeof reason.then === 'function' + ? new Error('The render was aborted by the server with a promise.') : reason; const digest = logRecoverableError(request, error); emitErrorChunk(request, errorId, digest, error); @@ -3594,6 +3676,10 @@ export function abort(request: Request, reason: mixed): void { ? new Error( 'The render was aborted by the server without a reason.', ) + : typeof reason === 'object' && + reason !== null && + typeof reason.then === 'function' + ? new Error('The render was aborted by the server with a promise.') : reason; } abortListeners.forEach(callback => callback(error)); diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index b157b6eaef7d2..ef4ae75a6d634 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -514,5 +514,6 @@ "526": "Could not reference an opaque temporary reference. This is likely due to misconfiguring the temporaryReferences options on the server.", "527": "Incompatible React versions: The \"react\" and \"react-dom\" packages must have the exact same version. Instead got:\n - react: %s\n - react-dom: %s\nLearn more: https://react.dev/warnings/version-mismatch", "528": "Expected not to update to be updated to a stylesheet with precedence. Check the `rel`, `href`, and `precedence` props of this component. Alternatively, check whether two different components render in the same slot or share the same key.%s", - "529": "Expected stylesheet with precedence to not be updated to a different kind of . Check the `rel`, `href`, and `precedence` props of this component. Alternatively, check whether two different components render in the same slot or share the same key.%s" + "529": "Expected stylesheet with precedence to not be updated to a different kind of . Check the `rel`, `href`, and `precedence` props of this component. Alternatively, check whether two different components render in the same slot or share the same key.%s", + "530": "The render was aborted by the server with a promise." }