From d85a30c7c9a58e7dcf2fdb891bb9f4f034d1ac0a Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 May 2020 11:37:45 -0700 Subject: [PATCH 1/4] useMutableSource hydration support --- .../ReactDOMServerIntegrationHooks-test.js | 98 ++--- packages/react-dom/src/client/ReactDOMRoot.js | 12 +- .../src/ReactFiberBeginWork.new.js | 23 ++ .../src/ReactFiberBeginWork.old.js | 23 ++ .../src/ReactFiberRoot.new.js | 3 +- .../src/ReactFiberRoot.old.js | 1 + .../src/ReactFiberWorkLoop.new.js | 38 +- .../src/ReactFiberWorkLoop.old.js | 38 +- .../src/ReactInternalTypes.js | 6 + .../src/ReactMutableSource.js | 24 ++ .../src/ReactMutableSource.new.js | 17 + .../src/ReactMutableSource.old.js | 16 + .../useMutableSourceHydration-test.js | 376 ++++++++++++++++++ scripts/jest/setupHostConfigs.js | 8 + scripts/rollup/forks.js | 20 + 15 files changed, 618 insertions(+), 85 deletions(-) create mode 100644 packages/react-reconciler/src/ReactMutableSource.js create mode 100644 packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js index c69ea90721046..ea5dfc1be7618 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js @@ -1445,8 +1445,6 @@ describe('ReactDOMServerHooks', () => { .getAttribute('id'); expect(serverId).not.toBeNull(); - const childOneSpan = container.getElementsByTagName('span')[0]; - const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); root.render(); expect(Scheduler).toHaveYielded([]); @@ -1462,25 +1460,15 @@ describe('ReactDOMServerHooks', () => { // State update should trigger the ID to update, which changes the props // of ChildWithID. This should cause ChildWithID to hydrate before Children - expect(Scheduler).toFlushAndYieldThrough( - __DEV__ - ? [ - 'Child with ID', - // Fallbacks are immediately committed in TestUtils version - // of act - // 'Child with ID', - // 'Child with ID', - 'Child One', - 'Child Two', - ] - : [ - 'Child with ID', - 'Child with ID', - 'Child with ID', - 'Child One', - 'Child Two', - ], - ); + expect(Scheduler).toFlushAndYieldThrough([ + 'Child with ID', + // Fallbacks are immediately committed in TestUtils version + // of act + // 'Child with ID', + // 'Child with ID', + 'Child One', + 'Child Two', + ]); expect(child1Ref.current).toBe(null); expect(childWithIDRef.current).toEqual( @@ -1500,7 +1488,9 @@ describe('ReactDOMServerHooks', () => { }); // Children hydrates after ChildWithID - expect(child1Ref.current).toBe(childOneSpan); + expect(child1Ref.current).toBe( + container.getElementsByTagName('span')[0], + ); Scheduler.unstable_flushAll(); @@ -1606,9 +1596,7 @@ describe('ReactDOMServerHooks', () => { ReactDOM.unstable_createRoot(container, {hydrate: true}).render( , ); - expect(() => - expect(() => Scheduler.unstable_flushAll()).toThrow(), - ).toErrorDev([ + expect(() => Scheduler.unstable_flushAll()).toErrorDev([ 'Warning: Expected server HTML to contain a matching
in
.', ]); }); @@ -1694,14 +1682,12 @@ describe('ReactDOMServerHooks', () => { ReactDOM.unstable_createRoot(container, {hydrate: true}).render( , ); - expect(() => - expect(() => Scheduler.unstable_flushAll()).toThrow(), - ).toErrorDev([ + expect(() => Scheduler.unstable_flushAll()).toErrorDev([ 'Warning: Expected server HTML to contain a matching
in
.', ]); }); - it('useOpaqueIdentifier throws when there is a hydration error and we are using ID as a string', async () => { + it('useOpaqueIdentifier warns when there is a hydration error and we are using ID as a string', async () => { function Child({appId}) { return
; } @@ -1718,12 +1704,7 @@ describe('ReactDOMServerHooks', () => { ReactDOM.unstable_createRoot(container, {hydrate: true}).render( , ); - expect(() => - expect(() => Scheduler.unstable_flushAll()).toThrow( - 'The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. ' + - 'Do not read the value directly.', - ), - ).toErrorDev( + expect(() => Scheduler.unstable_flushAll()).toErrorDev( [ 'Warning: The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. Do not read the value directly.', 'Warning: Did not expect server HTML to contain a in
.', @@ -1732,7 +1713,7 @@ describe('ReactDOMServerHooks', () => { ); }); - it('useOpaqueIdentifier throws when there is a hydration error and we are using ID as a string', async () => { + it('useOpaqueIdentifier warns when there is a hydration error and we are using ID as a string', async () => { function Child({appId}) { return
; } @@ -1749,12 +1730,7 @@ describe('ReactDOMServerHooks', () => { ReactDOM.unstable_createRoot(container, {hydrate: true}).render( , ); - expect(() => - expect(() => Scheduler.unstable_flushAll()).toThrow( - 'The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. ' + - 'Do not read the value directly.', - ), - ).toErrorDev( + expect(() => Scheduler.unstable_flushAll()).toErrorDev( [ 'Warning: The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. Do not read the value directly.', 'Warning: Did not expect server HTML to contain a in
.', @@ -1763,7 +1739,7 @@ describe('ReactDOMServerHooks', () => { ); }); - it('useOpaqueIdentifier throws if you try to use the result as a string in a child component', async () => { + it('useOpaqueIdentifier warns if you try to use the result as a string in a child component', async () => { function Child({appId}) { return
; } @@ -1779,12 +1755,7 @@ describe('ReactDOMServerHooks', () => { ReactDOM.unstable_createRoot(container, {hydrate: true}).render( , ); - expect(() => - expect(() => Scheduler.unstable_flushAll()).toThrow( - 'The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. ' + - 'Do not read the value directly.', - ), - ).toErrorDev( + expect(() => Scheduler.unstable_flushAll()).toErrorDev( [ 'Warning: The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. Do not read the value directly.', 'Warning: Did not expect server HTML to contain a
in
.', @@ -1793,7 +1764,7 @@ describe('ReactDOMServerHooks', () => { ); }); - it('useOpaqueIdentifier throws if you try to use the result as a string', async () => { + it('useOpaqueIdentifier warns if you try to use the result as a string', async () => { function App() { const id = useOpaqueIdentifier(); return
; @@ -1806,12 +1777,7 @@ describe('ReactDOMServerHooks', () => { ReactDOM.unstable_createRoot(container, {hydrate: true}).render( , ); - expect(() => - expect(() => Scheduler.unstable_flushAll()).toThrow( - 'The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. ' + - 'Do not read the value directly.', - ), - ).toErrorDev( + expect(() => Scheduler.unstable_flushAll()).toErrorDev( [ 'Warning: The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. Do not read the value directly.', 'Warning: Did not expect server HTML to contain a
in
.', @@ -1820,7 +1786,7 @@ describe('ReactDOMServerHooks', () => { ); }); - it('useOpaqueIdentifier throws if you try to use the result as a string in a child component wrapped in a Suspense', async () => { + it('useOpaqueIdentifier warns if you try to use the result as a string in a child component wrapped in a Suspense', async () => { function Child({appId}) { return
; } @@ -1842,16 +1808,14 @@ describe('ReactDOMServerHooks', () => { , ); - if ( - gate(flags => flags.new && flags.deferRenderPhaseUpdateToNextBatch) - ) { + if (gate(flags => !flags.new)) { expect(() => Scheduler.unstable_flushAll()).toErrorDev([ 'The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. ' + 'Do not read the value directly.', ]); } else { - // In the old reconciler, the error isn't surfaced to the user. That - // part isn't important, as long as It warns. + // This error isn't surfaced to the user; only the warning is. + // The error is just the mechanism that restarts the render. expect(() => expect(() => Scheduler.unstable_flushAll()).toThrow( 'The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. ' + @@ -1864,7 +1828,7 @@ describe('ReactDOMServerHooks', () => { } }); - it('useOpaqueIdentifier throws if you try to add the result as a number in a child component wrapped in a Suspense', async () => { + it('useOpaqueIdentifier warns if you try to add the result as a number in a child component wrapped in a Suspense', async () => { function Child({appId}) { return
; } @@ -1888,16 +1852,14 @@ describe('ReactDOMServerHooks', () => { , ); - if ( - gate(flags => flags.new && flags.deferRenderPhaseUpdateToNextBatch) - ) { + if (gate(flags => !flags.new)) { expect(() => Scheduler.unstable_flushAll()).toErrorDev([ 'The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. ' + 'Do not read the value directly.', ]); } else { - // In the old reconciler, the error isn't surfaced to the user. That - // part isn't important, as long as It warns. + // This error isn't surfaced to the user; only the warning is. + // The error is just the mechanism that restarts the render. expect(() => expect(() => Scheduler.unstable_flushAll()).toThrow( 'The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. ' + diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index 3c29c2e86a66e..34a3223a0cd6e 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -9,9 +9,8 @@ import type {Container} from './ReactDOMHostConfig'; import type {RootTag} from 'react-reconciler/src/ReactRootTags'; -import type {ReactNodeList} from 'shared/ReactTypes'; +import type {MutableSource, ReactNodeList} from 'shared/ReactTypes'; import type {FiberRoot} from 'react-reconciler/src/ReactInternalTypes'; -import {findHostInstanceWithNoPortals} from 'react-reconciler/src/ReactFiberReconciler'; export type RootType = { render(children: ReactNodeList): void, @@ -30,6 +29,8 @@ export type RootOptions = { ... }; +import {findHostInstanceWithNoPortals} from 'react-reconciler/src/ReactFiberReconciler'; +import {registerMutableSourceForHydration} from 'react-reconciler/src/ReactMutableSource'; import { isContainerMarkedAsRoot, markContainerAsRoot, @@ -115,6 +116,13 @@ ReactDOMRoot.prototype.unmount = ReactDOMBlockingRoot.prototype.unmount = functi }); }; +ReactDOMRoot.prototype.registerMutableSourceForHydration = ReactDOMBlockingRoot.prototype.registerMutableSourceForHydration = function( + mutableSource: MutableSource, +): void { + const root = this._internalRoot; + registerMutableSourceForHydration(root, mutableSource); +}; + function createRootImpl( container: Container, tag: RootTag, diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 76838eda06e91..0d5eae224b2c6 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -13,6 +13,7 @@ import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy'; import type {Fiber} from './ReactInternalTypes'; import type {FiberRoot} from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane'; +import type {MutableSource} from 'shared/ReactTypes'; import type { SuspenseState, SuspenseListRenderState, @@ -193,8 +194,12 @@ import { markSkippedUpdateLanes, getWorkInProgressRoot, pushRenderLanes, + getExecutionContext, + RetryAfterError, + NoContext, } from './ReactFiberWorkLoop.new'; import {unstable_wrap as Schedule_tracing_wrap} from 'scheduler/tracing'; +import {setWorkInProgressVersion} from './ReactMutableSource.new'; import {disableLogs, reenableLogs} from 'shared/ConsolePatchingDev'; @@ -1071,6 +1076,16 @@ function updateHostRoot(current, workInProgress, renderLanes) { // be any children to hydrate which is effectively the same thing as // not hydrating. + const mutableSourceEagerHydrationData = + root.mutableSourceEagerHydrationData; + for (let i = 0; i < mutableSourceEagerHydrationData.length; i += 2) { + const mutableSource = ((mutableSourceEagerHydrationData[ + i + ]: any): MutableSource); + const version = mutableSourceEagerHydrationData[i + 1]; + setWorkInProgressVersion(mutableSource, version); + } + const child = mountChildFibers( workInProgress, null, @@ -2253,6 +2268,14 @@ function updateDehydratedSuspenseComponent( // but after we've already committed once. warnIfHydrating(); + if ((getExecutionContext() & RetryAfterError) !== NoContext) { + return retrySuspenseComponentWithoutHydrating( + current, + workInProgress, + renderLanes, + ); + } + if ((workInProgress.mode & BlockingMode) === NoMode) { return retrySuspenseComponentWithoutHydrating( current, diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 0a3a4e5a47317..f7179fe353d0a 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -13,6 +13,7 @@ import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy'; import type {Fiber} from './ReactInternalTypes'; import type {FiberRoot} from './ReactInternalTypes'; import type {ExpirationTime} from './ReactFiberExpirationTime.old'; +import type {MutableSource} from 'shared/ReactTypes'; import type { SuspenseState, SuspenseListRenderState, @@ -179,8 +180,12 @@ import { renderDidSuspendDelayIfPossible, markUnprocessedUpdateTime, getWorkInProgressRoot, + getExecutionContext, + RetryAfterError, + NoContext, } from './ReactFiberWorkLoop.old'; import {unstable_wrap as Schedule_tracing_wrap} from 'scheduler/tracing'; +import {setWorkInProgressVersion} from './ReactMutableSource.old'; import {disableLogs, reenableLogs} from 'shared/ConsolePatchingDev'; @@ -1038,6 +1043,16 @@ function updateHostRoot(current, workInProgress, renderExpirationTime) { // be any children to hydrate which is effectively the same thing as // not hydrating. + const mutableSourceEagerHydrationData = + root.mutableSourceEagerHydrationData; + for (let i = 0; i < mutableSourceEagerHydrationData.length; i += 2) { + const mutableSource = ((mutableSourceEagerHydrationData[ + i + ]: any): MutableSource); + const version = mutableSourceEagerHydrationData[i + 1]; + setWorkInProgressVersion(mutableSource, version); + } + const child = mountChildFibers( workInProgress, null, @@ -2239,6 +2254,14 @@ function updateDehydratedSuspenseComponent( // but after we've already committed once. warnIfHydrating(); + if ((getExecutionContext() & RetryAfterError) !== NoContext) { + return retrySuspenseComponentWithoutHydrating( + current, + workInProgress, + renderExpirationTime, + ); + } + if ((workInProgress.mode & BlockingMode) === NoMode) { return retrySuspenseComponentWithoutHydrating( current, diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js index 96f795cbdf279..a8d49bf725360 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.new.js +++ b/packages/react-reconciler/src/ReactFiberRoot.new.js @@ -46,12 +46,13 @@ function FiberRootNode(containerInfo, tag, hydrate) { this.pingedLanes = NoLanes; this.expiredLanes = NoLanes; this.mutableReadLanes = NoLanes; - this.finishedLanes = NoLanes; this.entangledLanes = NoLanes; this.entanglements = createLaneMap(NoLanes); + this.mutableSourceEagerHydrationData = []; + if (enableSchedulerTracing) { this.interactionThreadID = unstable_getThreadID(); this.memoizedInteractions = new Set(); diff --git a/packages/react-reconciler/src/ReactFiberRoot.old.js b/packages/react-reconciler/src/ReactFiberRoot.old.js index 72d03a596925d..cece9e9409bd1 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.old.js +++ b/packages/react-reconciler/src/ReactFiberRoot.old.js @@ -45,6 +45,7 @@ function FiberRootNode(containerInfo, tag, hydrate) { this.lastPingedTime = NoWork; this.lastExpiredTime = NoWork; this.mutableSourceLastPendingUpdateTime = NoWork; + this.mutableSourceEagerHydrationData = []; if (enableSchedulerTracing) { this.interactionThreadID = unstable_getThreadID(); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 53ac747aa4627..312b74f75b2c3 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -62,6 +62,7 @@ import { warnsIfNotActing, beforeActiveInstanceBlur, afterActiveInstanceBlur, + clearContainer, } from './ReactFiberHostConfig'; import { @@ -215,13 +216,14 @@ const { type ExecutionContext = number; -const NoContext = /* */ 0b000000; -const BatchedContext = /* */ 0b000001; -const EventContext = /* */ 0b000010; -const DiscreteEventContext = /* */ 0b000100; -const LegacyUnbatchedContext = /* */ 0b001000; -const RenderContext = /* */ 0b010000; -const CommitContext = /* */ 0b100000; +export const NoContext = /* */ 0b0000000; +const BatchedContext = /* */ 0b0000001; +const EventContext = /* */ 0b0000010; +const DiscreteEventContext = /* */ 0b0000100; +const LegacyUnbatchedContext = /* */ 0b0001000; +const RenderContext = /* */ 0b0010000; +const CommitContext = /* */ 0b0100000; +export const RetryAfterError = /* */ 0b1000000; type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5; const RootIncomplete = 0; @@ -724,6 +726,15 @@ function performConcurrentWorkOnRoot(root, didTimeout) { prepareFreshStack(root, NoLanes); } else if (exitStatus !== RootIncomplete) { if (exitStatus === RootErrored) { + executionContext |= RetryAfterError; + + // If an error occurred during hydration, + // discard server response and fall back to client side render. + if (root.hydrate) { + root.hydrate = false; + clearContainer(root.containerInfo); + } + // If something threw an error, try rendering one more time. We'll render // synchronously to block concurrent data mutations, and we'll includes // all pending updates are included. If it still fails after the second @@ -976,6 +987,15 @@ function performSyncWorkOnRoot(root) { } if (root.tag !== LegacyRoot && exitStatus === RootErrored) { + executionContext |= RetryAfterError; + + // If an error occurred during hydration, + // discard server response and fall back to client side render. + if (root.hydrate) { + root.hydrate = false; + clearContainer(root.containerInfo); + } + // If something threw an error, try rendering one more time. We'll render // synchronously to block concurrent data mutations, and we'll includes // all pending updates are included. If it still fails after the second @@ -1016,6 +1036,10 @@ export function flushRoot(root: FiberRoot, lanes: Lanes) { } } +export function getExecutionContext(): ExecutionContext { + return executionContext; +} + export function flushDiscreteUpdates() { // TODO: Should be able to flush inside batchedUpdates, but not inside `act`. // However, `act` uses `batchedUpdates`, so there's no way to distinguish diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index b984a3fa66b96..f8dd5185f1581 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -74,6 +74,7 @@ import { warnsIfNotActing, beforeActiveInstanceBlur, afterActiveInstanceBlur, + clearContainer, } from './ReactFiberHostConfig'; import { @@ -207,13 +208,14 @@ const { type ExecutionContext = number; -const NoContext = /* */ 0b000000; -const BatchedContext = /* */ 0b000001; -const EventContext = /* */ 0b000010; -const DiscreteEventContext = /* */ 0b000100; -const LegacyUnbatchedContext = /* */ 0b001000; -const RenderContext = /* */ 0b010000; -const CommitContext = /* */ 0b100000; +export const NoContext = /* */ 0b0000000; +const BatchedContext = /* */ 0b0000001; +const EventContext = /* */ 0b0000010; +const DiscreteEventContext = /* */ 0b0000100; +const LegacyUnbatchedContext = /* */ 0b0001000; +const RenderContext = /* */ 0b0010000; +const CommitContext = /* */ 0b0100000; +export const RetryAfterError = /* */ 0b1000000; type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5; const RootIncomplete = 0; @@ -728,6 +730,15 @@ function performConcurrentWorkOnRoot(root, didTimeout) { if (exitStatus !== RootIncomplete) { if (exitStatus === RootErrored) { + executionContext |= RetryAfterError; + + // If an error occurred during hydration, + // discard server response and fall back to client side render. + if (root.hydrate) { + root.hydrate = false; + clearContainer(root.containerInfo); + } + // If something threw an error, try rendering one more time. We'll // render synchronously to block concurrent data mutations, and we'll // render at Idle (or lower) so that all pending updates are included. @@ -1011,6 +1022,15 @@ function performSyncWorkOnRoot(root) { let exitStatus = renderRootSync(root, expirationTime); if (root.tag !== LegacyRoot && exitStatus === RootErrored) { + executionContext |= RetryAfterError; + + // If an error occurred during hydration, + // discard server response and fall back to client side render. + if (root.hydrate) { + root.hydrate = false; + clearContainer(root.containerInfo); + } + // If something threw an error, try rendering one more time. We'll // render synchronously to block concurrent data mutations, and we'll // render at Idle (or lower) so that all pending updates are included. @@ -1051,6 +1071,10 @@ export function flushRoot(root: FiberRoot, expirationTime: ExpirationTime) { } } +export function getExecutionContext(): ExecutionContext { + return executionContext; +} + export function flushDiscreteUpdates() { // TODO: Should be able to flush inside batchedUpdates, but not inside `act`. // However, `act` uses `batchedUpdates`, so there's no way to distinguish diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 54b3825fa3e4b..1c9878f05cc55 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -16,6 +16,7 @@ import type { ReactContext, MutableSourceSubscribeFn, MutableSourceGetSnapshotFn, + MutableSourceVersion, MutableSource, } from 'shared/ReactTypes'; import type {SuspenseInstance} from './ReactFiberHostConfig'; @@ -247,6 +248,11 @@ type BaseFiberRootProperties = {| // when external, mutable sources are read from during render. mutableSourceLastPendingUpdateTime: ExpirationTime, + // Used by useMutableSource hook to avoid tearing during hydrtaion. + mutableSourceEagerHydrationData: Array< + MutableSource | MutableSourceVersion, + >, + // Only used by new reconciler // Represents the next task that the root should work on, or the current one diff --git a/packages/react-reconciler/src/ReactMutableSource.js b/packages/react-reconciler/src/ReactMutableSource.js new file mode 100644 index 0000000000000..0b48c6183c3fb --- /dev/null +++ b/packages/react-reconciler/src/ReactMutableSource.js @@ -0,0 +1,24 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import {enableNewReconciler} from 'shared/ReactFeatureFlags'; + +// The entry file imports either the old or new version of mutable source. +// This is necessary since ReactDOMRoot imports this module directly. +// Note that it's not possible to export all of the API methods, +// as the new and old implementations fork slightly (due to the lanes refactor). +// It's only necessary to export the subset of the API required by ReactDOMRoot. + +import {registerMutableSourceForHydration as registerMutableSourceForHydration_old} from './ReactMutableSource.old'; + +import {registerMutableSourceForHydration as registerMutableSourceForHydration_new} from './ReactMutableSource.new'; + +export const registerMutableSourceForHydration = enableNewReconciler + ? registerMutableSourceForHydration_new + : registerMutableSourceForHydration_old; diff --git a/packages/react-reconciler/src/ReactMutableSource.new.js b/packages/react-reconciler/src/ReactMutableSource.new.js index dd21b95781c59..10ca424cdd4be 100644 --- a/packages/react-reconciler/src/ReactMutableSource.new.js +++ b/packages/react-reconciler/src/ReactMutableSource.new.js @@ -8,6 +8,7 @@ */ import type {MutableSource, MutableSourceVersion} from 'shared/ReactTypes'; +import type {FiberRoot} from './ReactInternalTypes'; import {isPrimaryRenderer} from './ReactFiberHostConfig'; @@ -85,3 +86,19 @@ export function warnAboutMultipleRenderersDEV( } } } + +// Eager reads the version of a mutable source and stores it on the root. +// This ensures that the version used for server rendering matches the one +// that is eventually read during hydration. +// If they don't match there's a potential tear and a full deopt render is required. +export function registerMutableSourceForHydration( + root: FiberRoot, + mutableSource: MutableSource, +): void { + const getVersion = mutableSource._getVersion; + const version = getVersion(mutableSource._source); + + // TODO Clear this data once all pending hydration work is finished. + // Retaining it forever may interfere with GC. + root.mutableSourceEagerHydrationData.push(mutableSource, version); +} diff --git a/packages/react-reconciler/src/ReactMutableSource.old.js b/packages/react-reconciler/src/ReactMutableSource.old.js index 6d88953128ecc..0d8ea3d01c6dd 100644 --- a/packages/react-reconciler/src/ReactMutableSource.old.js +++ b/packages/react-reconciler/src/ReactMutableSource.old.js @@ -116,3 +116,19 @@ export function warnAboutMultipleRenderersDEV( } } } + +// Eager reads the version of a mutable source and stores it on the root. +// This ensures that the version used for server rendering matches the one +// that is eventually read during hydration. +// If they don't match there's a potential tear and a full deopt render is required. +export function registerMutableSourceForHydration( + root: FiberRoot, + mutableSource: MutableSource, +): void { + const getVersion = mutableSource._getVersion; + const version = getVersion(mutableSource._source); + + // TODO Clear this data once all pending hydration work is finished. + // Retaining it forever may interfere with GC. + root.mutableSourceEagerHydrationData.push(mutableSource, version); +} diff --git a/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js b/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js new file mode 100644 index 0000000000000..229282ad4434c --- /dev/null +++ b/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js @@ -0,0 +1,376 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +let React; +let ReactDOM; +let ReactDOMServer; +let Scheduler; +let act; +let useMutableSource; + +describe('useMutableSourceHydration', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactDOM = require('react-dom'); + ReactDOMServer = require('react-dom/server'); + Scheduler = require('scheduler'); + + useMutableSource = React.useMutableSource; + act = require('react-dom/test-utils').act; + }); + + const defaultGetSnapshot = source => source.value; + const defaultSubscribe = (source, callback) => source.subscribe(callback); + + function createComplexSource(initialValueA, initialValueB) { + const callbacksA = []; + const callbacksB = []; + let revision = 0; + let valueA = initialValueA; + let valueB = initialValueB; + + const subscribeHelper = (callbacks, callback) => { + if (callbacks.indexOf(callback) < 0) { + callbacks.push(callback); + } + return () => { + const index = callbacks.indexOf(callback); + if (index >= 0) { + callbacks.splice(index, 1); + } + }; + }; + + return { + subscribeA(callback) { + return subscribeHelper(callbacksA, callback); + }, + subscribeB(callback) { + return subscribeHelper(callbacksB, callback); + }, + + get listenerCountA() { + return callbacksA.length; + }, + get listenerCountB() { + return callbacksB.length; + }, + + set valueA(newValue) { + revision++; + valueA = newValue; + callbacksA.forEach(callback => callback()); + }, + get valueA() { + return valueA; + }, + + set valueB(newValue) { + revision++; + valueB = newValue; + callbacksB.forEach(callback => callback()); + }, + get valueB() { + return valueB; + }, + + get version() { + return revision; + }, + }; + } + + function createSource(initialValue) { + const callbacks = []; + let revision = 0; + let value = initialValue; + return { + subscribe(callback) { + if (callbacks.indexOf(callback) < 0) { + callbacks.push(callback); + } + return () => { + const index = callbacks.indexOf(callback); + if (index >= 0) { + callbacks.splice(index, 1); + } + }; + }, + get listenerCount() { + return callbacks.length; + }, + set value(newValue) { + revision++; + value = newValue; + callbacks.forEach(callback => callback()); + }, + get value() { + return value; + }, + get version() { + return revision; + }, + }; + } + + function createMutableSource(source) { + return React.createMutableSource(source, param => param.version); + } + + function Component({getSnapshot, label, mutableSource, subscribe}) { + const snapshot = useMutableSource(mutableSource, getSnapshot, subscribe); + Scheduler.unstable_yieldValue(`${label}:${snapshot}`); + return
{`${label}:${snapshot}`}
; + } + + // @gate experimental + it('should render and hydrate', () => { + const source = createSource('one'); + const mutableSource = createMutableSource(source); + + function TestComponent() { + return ( + + ); + } + + const container = document.createElement('div'); + document.body.appendChild(container); + + const htmlString = ReactDOMServer.renderToString(); + container.innerHTML = htmlString; + expect(Scheduler).toHaveYielded(['only:one']); + expect(source.listenerCount).toBe(0); + + const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + act(() => { + root.registerMutableSourceForHydration(mutableSource); + root.render(); + }); + expect(Scheduler).toHaveYielded(['only:one']); + expect(source.listenerCount).toBe(1); + }); + + // @gate experimental + it('should detect a tear before hydrating a component', () => { + const source = createSource('one'); + const mutableSource = createMutableSource(source); + + function TestComponent() { + return ( + + ); + } + + const container = document.createElement('div'); + document.body.appendChild(container); + + const htmlString = ReactDOMServer.renderToString(); + container.innerHTML = htmlString; + expect(Scheduler).toHaveYielded(['only:one']); + expect(source.listenerCount).toBe(0); + + const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + expect(() => { + act(() => { + root.registerMutableSourceForHydration(mutableSource); + root.render(); + + source.value = 'two'; + }); + }).toErrorDev( + 'Warning: Did not expect server HTML to contain a
in
.', + {withoutStack: true}, + ); + expect(Scheduler).toHaveYielded(['only:two']); + expect(source.listenerCount).toBe(1); + }); + + // @gate experimental + it('should detect a tear between hydrating components', () => { + const source = createSource('one'); + const mutableSource = createMutableSource(source); + + function TestComponent() { + return ( + <> + + + + ); + } + + const container = document.createElement('div'); + document.body.appendChild(container); + + const htmlString = ReactDOMServer.renderToString(); + container.innerHTML = htmlString; + expect(Scheduler).toHaveYielded(['a:one', 'b:one']); + expect(source.listenerCount).toBe(0); + + const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + expect(() => { + act(() => { + root.registerMutableSourceForHydration(mutableSource); + root.render(); + expect(Scheduler).toFlushAndYieldThrough(['a:one']); + source.value = 'two'; + }); + }).toErrorDev( + 'Warning: Did not expect server HTML to contain a
in
.', + {withoutStack: true}, + ); + expect(Scheduler).toHaveYielded(['a:two', 'b:two']); + expect(source.listenerCount).toBe(2); + }); + + // @gate experimental + it('should detect a tear between hydrating components reading from different parts of a source', () => { + const source = createComplexSource('a:one', 'b:one'); + const mutableSource = createMutableSource(source); + + // Subscribe to part of the store. + const getSnapshotA = s => s.valueA; + const subscribeA = (s, callback) => s.subscribeA(callback); + const getSnapshotB = s => s.valueB; + const subscribeB = (s, callback) => s.subscribeB(callback); + + const container = document.createElement('div'); + document.body.appendChild(container); + + const htmlString = ReactDOMServer.renderToString( + <> + + + , + ); + container.innerHTML = htmlString; + expect(Scheduler).toHaveYielded(['0:a:one', '1:b:one']); + + const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + expect(() => { + act(() => { + root.registerMutableSourceForHydration(mutableSource); + root.render( + <> + + + , + ); + expect(Scheduler).toFlushAndYieldThrough(['0:a:one']); + source.valueB = 'b:two'; + }); + }).toErrorDev( + 'Warning: Did not expect server HTML to contain a
in
.', + {withoutStack: true}, + ); + expect(Scheduler).toHaveYielded(['0:a:one', '1:b:two']); + }); + + // @gate experimental + it('should detect a tear during a higher priority interruption', () => { + const source = createSource('one'); + const mutableSource = createMutableSource(source); + + function Unrelated({flag}) { + Scheduler.unstable_yieldValue(flag); + return flag; + } + + function TestComponent({flag}) { + return ( + <> + + + + ); + } + + const container = document.createElement('div'); + document.body.appendChild(container); + + const htmlString = ReactDOMServer.renderToString( + , + ); + container.innerHTML = htmlString; + expect(Scheduler).toHaveYielded([1, 'a:one']); + expect(source.listenerCount).toBe(0); + + const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + expect(() => { + act(() => { + root.registerMutableSourceForHydration(mutableSource); + root.render(); + expect(Scheduler).toFlushAndYieldThrough([1]); + + // Render an update which will be higher priority than the hydration. + Scheduler.unstable_runWithPriority( + Scheduler.unstable_UserBlockingPriority, + () => root.render(), + ); + expect(Scheduler).toFlushAndYieldThrough([2]); + + source.value = 'two'; + }); + }).toErrorDev( + 'Warning: Text content did not match. Server: "1" Client: "2"', + ); + expect(Scheduler).toHaveYielded([2, 'a:two']); + expect(source.listenerCount).toBe(1); + }); +}); diff --git a/scripts/jest/setupHostConfigs.js b/scripts/jest/setupHostConfigs.js index 9a4af65cf0c59..7f525dee4891e 100644 --- a/scripts/jest/setupHostConfigs.js +++ b/scripts/jest/setupHostConfigs.js @@ -10,6 +10,14 @@ jest.mock('react-reconciler/src/ReactFiberReconciler', () => { ); }); +jest.mock('react-reconciler/src/ReactMutableSource', () => { + return require.requireActual( + __VARIANT__ + ? 'react-reconciler/src/ReactMutableSource.new' + : 'react-reconciler/src/ReactMutableSource.old' + ); +}); + // When testing the custom renderer code path through `react-reconciler`, // turn the export into a function, and use the argument as host config. const shimHostConfigPath = 'react-reconciler/src/ReactFiberHostConfig'; diff --git a/scripts/rollup/forks.js b/scripts/rollup/forks.js index b0319333ea80d..93e12fe04dbd8 100644 --- a/scripts/rollup/forks.js +++ b/scripts/rollup/forks.js @@ -280,6 +280,26 @@ const forks = Object.freeze({ return 'react-reconciler/src/ReactFiberReconciler.old.js'; }, + 'react-reconciler/src/ReactMutableSource': ( + bundleType, + entry, + dependencies, + moduleType, + bundle + ) => { + if (bundle.enableNewReconciler) { + switch (bundleType) { + case FB_WWW_DEV: + case FB_WWW_PROD: + case FB_WWW_PROFILING: + // Use the forked version of the reconciler + return 'react-reconciler/src/ReactMutableSource.new.js'; + } + } + // Otherwise, use the non-forked version. + return 'react-reconciler/src/ReactMutableSource.old.js'; + }, + 'react-reconciler/src/ReactFiberHotReloading': ( bundleType, entry, From 6d7ea6baeb2006f782188abe596827fb043604dc Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 May 2020 11:38:01 -0700 Subject: [PATCH 2/4] Remove unnecessary ReactMutableSource fork --- packages/react-dom/src/client/ReactDOMRoot.js | 4 ++-- .../src/ReactFiberReconciler.js | 6 +++++ .../src/ReactFiberReconciler.new.js | 1 + .../src/ReactFiberReconciler.old.js | 1 + .../src/ReactMutableSource.js | 24 ------------------- scripts/jest/setupHostConfigs.js | 8 ------- scripts/rollup/forks.js | 20 ---------------- 7 files changed, 10 insertions(+), 54 deletions(-) delete mode 100644 packages/react-reconciler/src/ReactMutableSource.js diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index 34a3223a0cd6e..928cfd3543ed0 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -29,8 +29,6 @@ export type RootOptions = { ... }; -import {findHostInstanceWithNoPortals} from 'react-reconciler/src/ReactFiberReconciler'; -import {registerMutableSourceForHydration} from 'react-reconciler/src/ReactMutableSource'; import { isContainerMarkedAsRoot, markContainerAsRoot, @@ -48,6 +46,8 @@ import {ensureListeningTo} from './ReactDOMComponent'; import { createContainer, updateContainer, + findHostInstanceWithNoPortals, + registerMutableSourceForHydration, } from 'react-reconciler/src/ReactFiberReconciler'; import invariant from 'shared/invariant'; import { diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 78992d9d7837c..6cc795b2ed6e3 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -49,6 +49,7 @@ import { findBoundingRects as findBoundingRects_old, focusWithin as focusWithin_old, observeVisibleRects as observeVisibleRects_old, + registerMutableSourceForHydration as registerMutableSourceForHydration_old, } from './ReactFiberReconciler.old'; import { @@ -86,6 +87,7 @@ import { findBoundingRects as findBoundingRects_new, focusWithin as focusWithin_new, observeVisibleRects as observeVisibleRects_new, + registerMutableSourceForHydration as registerMutableSourceForHydration_new, } from './ReactFiberReconciler.new'; export const createContainer = enableNewReconciler @@ -186,3 +188,7 @@ export const focusWithin = enableNewReconciler export const observeVisibleRects = enableNewReconciler ? observeVisibleRects_new : observeVisibleRects_old; + +export const registerMutableSourceForHydration = enableNewReconciler + ? registerMutableSourceForHydration_new + : registerMutableSourceForHydration_old; diff --git a/packages/react-reconciler/src/ReactFiberReconciler.new.js b/packages/react-reconciler/src/ReactFiberReconciler.new.js index eb3eb6a760246..3d508dec78aea 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.new.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.new.js @@ -88,6 +88,7 @@ import { findHostInstancesForRefresh, } from './ReactFiberHotReloading.new'; +export {registerMutableSourceForHydration} from './ReactMutableSource.new'; export {createPortal} from './ReactPortal'; export { createComponentSelector, diff --git a/packages/react-reconciler/src/ReactFiberReconciler.old.js b/packages/react-reconciler/src/ReactFiberReconciler.old.js index 8646598564c30..68d3fbd206c55 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.old.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.old.js @@ -85,6 +85,7 @@ import { findHostInstancesForRefresh, } from './ReactFiberHotReloading.old'; +export {registerMutableSourceForHydration} from './ReactMutableSource.old'; export {createPortal} from './ReactPortal'; export { createComponentSelector, diff --git a/packages/react-reconciler/src/ReactMutableSource.js b/packages/react-reconciler/src/ReactMutableSource.js deleted file mode 100644 index 0b48c6183c3fb..0000000000000 --- a/packages/react-reconciler/src/ReactMutableSource.js +++ /dev/null @@ -1,24 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -import {enableNewReconciler} from 'shared/ReactFeatureFlags'; - -// The entry file imports either the old or new version of mutable source. -// This is necessary since ReactDOMRoot imports this module directly. -// Note that it's not possible to export all of the API methods, -// as the new and old implementations fork slightly (due to the lanes refactor). -// It's only necessary to export the subset of the API required by ReactDOMRoot. - -import {registerMutableSourceForHydration as registerMutableSourceForHydration_old} from './ReactMutableSource.old'; - -import {registerMutableSourceForHydration as registerMutableSourceForHydration_new} from './ReactMutableSource.new'; - -export const registerMutableSourceForHydration = enableNewReconciler - ? registerMutableSourceForHydration_new - : registerMutableSourceForHydration_old; diff --git a/scripts/jest/setupHostConfigs.js b/scripts/jest/setupHostConfigs.js index 7f525dee4891e..9a4af65cf0c59 100644 --- a/scripts/jest/setupHostConfigs.js +++ b/scripts/jest/setupHostConfigs.js @@ -10,14 +10,6 @@ jest.mock('react-reconciler/src/ReactFiberReconciler', () => { ); }); -jest.mock('react-reconciler/src/ReactMutableSource', () => { - return require.requireActual( - __VARIANT__ - ? 'react-reconciler/src/ReactMutableSource.new' - : 'react-reconciler/src/ReactMutableSource.old' - ); -}); - // When testing the custom renderer code path through `react-reconciler`, // turn the export into a function, and use the argument as host config. const shimHostConfigPath = 'react-reconciler/src/ReactFiberHostConfig'; diff --git a/scripts/rollup/forks.js b/scripts/rollup/forks.js index 93e12fe04dbd8..b0319333ea80d 100644 --- a/scripts/rollup/forks.js +++ b/scripts/rollup/forks.js @@ -280,26 +280,6 @@ const forks = Object.freeze({ return 'react-reconciler/src/ReactFiberReconciler.old.js'; }, - 'react-reconciler/src/ReactMutableSource': ( - bundleType, - entry, - dependencies, - moduleType, - bundle - ) => { - if (bundle.enableNewReconciler) { - switch (bundleType) { - case FB_WWW_DEV: - case FB_WWW_PROD: - case FB_WWW_PROFILING: - // Use the forked version of the reconciler - return 'react-reconciler/src/ReactMutableSource.new.js'; - } - } - // Otherwise, use the non-forked version. - return 'react-reconciler/src/ReactMutableSource.old.js'; - }, - 'react-reconciler/src/ReactFiberHotReloading': ( bundleType, entry, From eb3e7059be6f1372eb8c8e8251302390237e756d Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 May 2020 12:53:28 -0700 Subject: [PATCH 3/4] Replaced root.registerMutableSourceForHydration() with mutableSources option --- packages/react-dom/src/client/ReactDOMRoot.js | 17 ++++++----- .../useMutableSourceHydration-test.js | 30 ++++++++++++------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index 928cfd3543ed0..a5ff7999f8840 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -26,6 +26,7 @@ export type RootOptions = { onDeleted?: (suspenseNode: Comment) => void, ... }, + mutableSources?: Array>, ... }; @@ -116,13 +117,6 @@ ReactDOMRoot.prototype.unmount = ReactDOMBlockingRoot.prototype.unmount = functi }); }; -ReactDOMRoot.prototype.registerMutableSourceForHydration = ReactDOMBlockingRoot.prototype.registerMutableSourceForHydration = function( - mutableSource: MutableSource, -): void { - const root = this._internalRoot; - registerMutableSourceForHydration(root, mutableSource); -}; - function createRootImpl( container: Container, tag: RootTag, @@ -132,6 +126,7 @@ function createRootImpl( const hydrate = options != null && options.hydrate === true; const hydrationCallbacks = (options != null && options.hydrationOptions) || null; + const mutableSources = (options != null && options.mutableSources) || null; const root = createContainer(container, tag, hydrate, hydrationCallbacks); markContainerAsRoot(root.current, container); const containerNodeType = container.nodeType; @@ -151,6 +146,14 @@ function createRootImpl( ) { ensureListeningTo(container, 'onMouseEnter'); } + + if (mutableSources) { + for (let i = 0; i < mutableSources.length; i++) { + const mutableSource = mutableSources[i]; + registerMutableSourceForHydration(root, mutableSource); + } + } + return root; } diff --git a/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js b/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js index 229282ad4434c..6868cd9b49cd4 100644 --- a/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js +++ b/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js @@ -157,9 +157,11 @@ describe('useMutableSourceHydration', () => { expect(Scheduler).toHaveYielded(['only:one']); expect(source.listenerCount).toBe(0); - const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + const root = ReactDOM.unstable_createRoot(container, { + hydrate: true, + mutableSources: [mutableSource], + }); act(() => { - root.registerMutableSourceForHydration(mutableSource); root.render(); }); expect(Scheduler).toHaveYielded(['only:one']); @@ -190,10 +192,12 @@ describe('useMutableSourceHydration', () => { expect(Scheduler).toHaveYielded(['only:one']); expect(source.listenerCount).toBe(0); - const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + const root = ReactDOM.unstable_createRoot(container, { + hydrate: true, + mutableSources: [mutableSource], + }); expect(() => { act(() => { - root.registerMutableSourceForHydration(mutableSource); root.render(); source.value = 'two'; @@ -238,10 +242,12 @@ describe('useMutableSourceHydration', () => { expect(Scheduler).toHaveYielded(['a:one', 'b:one']); expect(source.listenerCount).toBe(0); - const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + const root = ReactDOM.unstable_createRoot(container, { + hydrate: true, + mutableSources: [mutableSource], + }); expect(() => { act(() => { - root.registerMutableSourceForHydration(mutableSource); root.render(); expect(Scheduler).toFlushAndYieldThrough(['a:one']); source.value = 'two'; @@ -287,10 +293,12 @@ describe('useMutableSourceHydration', () => { container.innerHTML = htmlString; expect(Scheduler).toHaveYielded(['0:a:one', '1:b:one']); - const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + const root = ReactDOM.unstable_createRoot(container, { + hydrate: true, + mutableSources: [mutableSource], + }); expect(() => { act(() => { - root.registerMutableSourceForHydration(mutableSource); root.render( <> { expect(Scheduler).toHaveYielded([1, 'a:one']); expect(source.listenerCount).toBe(0); - const root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + const root = ReactDOM.unstable_createRoot(container, { + hydrate: true, + mutableSources: [mutableSource], + }); expect(() => { act(() => { - root.registerMutableSourceForHydration(mutableSource); root.render(); expect(Scheduler).toFlushAndYieldThrough([1]); From ec01142def6f9034ca08ab2bd0311f61a2cb5340 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 18 May 2020 16:40:55 -0700 Subject: [PATCH 4/4] Response to PR feedback: 1. Moved mutableSources root option to hydrationOptions object 2. Only initialize root mutableSourceEagerHydrationData if supportsHydration config is true 3. Lazily initialize mutableSourceEagerHydrationData on root object --- packages/react-dom/src/client/ReactDOMRoot.js | 8 +++++-- .../src/ReactFiberBeginWork.new.js | 21 ++++++++++++------- .../src/ReactFiberBeginWork.old.js | 21 ++++++++++++------- .../src/ReactFiberRoot.new.js | 6 ++++-- .../src/ReactFiberRoot.old.js | 7 +++++-- .../src/ReactInternalTypes.js | 4 ++-- .../src/ReactMutableSource.new.js | 6 +++++- .../src/ReactMutableSource.old.js | 6 +++++- .../useMutableSourceHydration-test.js | 20 +++++++++++++----- 9 files changed, 68 insertions(+), 31 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index a5ff7999f8840..549f7c3e60042 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -24,9 +24,9 @@ export type RootOptions = { hydrationOptions?: { onHydrated?: (suspenseNode: Comment) => void, onDeleted?: (suspenseNode: Comment) => void, + mutableSources?: Array>, ... }, - mutableSources?: Array>, ... }; @@ -126,7 +126,11 @@ function createRootImpl( const hydrate = options != null && options.hydrate === true; const hydrationCallbacks = (options != null && options.hydrationOptions) || null; - const mutableSources = (options != null && options.mutableSources) || null; + const mutableSources = + (options != null && + options.hydrationOptions != null && + options.hydrationOptions.mutableSources) || + null; const root = createContainer(container, tag, hydrate, hydrationCallbacks); markContainerAsRoot(root.current, container); const containerNodeType = container.nodeType; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 0d5eae224b2c6..38cbec836fe3c 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -127,6 +127,7 @@ import { isSuspenseInstancePending, isSuspenseInstanceFallback, registerSuspenseInstanceRetry, + supportsHydration, } from './ReactFiberHostConfig'; import type {SuspenseInstance} from './ReactFiberHostConfig'; import {shouldSuspend} from './ReactFiberReconciler'; @@ -1076,14 +1077,18 @@ function updateHostRoot(current, workInProgress, renderLanes) { // be any children to hydrate which is effectively the same thing as // not hydrating. - const mutableSourceEagerHydrationData = - root.mutableSourceEagerHydrationData; - for (let i = 0; i < mutableSourceEagerHydrationData.length; i += 2) { - const mutableSource = ((mutableSourceEagerHydrationData[ - i - ]: any): MutableSource); - const version = mutableSourceEagerHydrationData[i + 1]; - setWorkInProgressVersion(mutableSource, version); + if (supportsHydration) { + const mutableSourceEagerHydrationData = + root.mutableSourceEagerHydrationData; + if (mutableSourceEagerHydrationData != null) { + for (let i = 0; i < mutableSourceEagerHydrationData.length; i += 2) { + const mutableSource = ((mutableSourceEagerHydrationData[ + i + ]: any): MutableSource); + const version = mutableSourceEagerHydrationData[i + 1]; + setWorkInProgressVersion(mutableSource, version); + } + } } const child = mountChildFibers( diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index f7179fe353d0a..2d2916ac77767 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -115,6 +115,7 @@ import { isSuspenseInstancePending, isSuspenseInstanceFallback, registerSuspenseInstanceRetry, + supportsHydration, } from './ReactFiberHostConfig'; import type {SuspenseInstance} from './ReactFiberHostConfig'; import {shouldSuspend} from './ReactFiberReconciler'; @@ -1043,14 +1044,18 @@ function updateHostRoot(current, workInProgress, renderExpirationTime) { // be any children to hydrate which is effectively the same thing as // not hydrating. - const mutableSourceEagerHydrationData = - root.mutableSourceEagerHydrationData; - for (let i = 0; i < mutableSourceEagerHydrationData.length; i += 2) { - const mutableSource = ((mutableSourceEagerHydrationData[ - i - ]: any): MutableSource); - const version = mutableSourceEagerHydrationData[i + 1]; - setWorkInProgressVersion(mutableSource, version); + if (supportsHydration) { + const mutableSourceEagerHydrationData = + root.mutableSourceEagerHydrationData; + if (mutableSourceEagerHydrationData != null) { + for (let i = 0; i < mutableSourceEagerHydrationData.length; i += 2) { + const mutableSource = ((mutableSourceEagerHydrationData[ + i + ]: any): MutableSource); + const version = mutableSourceEagerHydrationData[i + 1]; + setWorkInProgressVersion(mutableSource, version); + } + } } const child = mountChildFibers( diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js index a8d49bf725360..d1e481414b35f 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.new.js +++ b/packages/react-reconciler/src/ReactFiberRoot.new.js @@ -10,7 +10,7 @@ import type {FiberRoot, SuspenseHydrationCallbacks} from './ReactInternalTypes'; import type {RootTag} from './ReactRootTags'; -import {noTimeout} from './ReactFiberHostConfig'; +import {noTimeout, supportsHydration} from './ReactFiberHostConfig'; import {createHostRootFiber} from './ReactFiber.new'; import { NoLanes, @@ -51,7 +51,9 @@ function FiberRootNode(containerInfo, tag, hydrate) { this.entangledLanes = NoLanes; this.entanglements = createLaneMap(NoLanes); - this.mutableSourceEagerHydrationData = []; + if (supportsHydration) { + this.mutableSourceEagerHydrationData = null; + } if (enableSchedulerTracing) { this.interactionThreadID = unstable_getThreadID(); diff --git a/packages/react-reconciler/src/ReactFiberRoot.old.js b/packages/react-reconciler/src/ReactFiberRoot.old.js index cece9e9409bd1..fab9d3def6557 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.old.js +++ b/packages/react-reconciler/src/ReactFiberRoot.old.js @@ -11,7 +11,7 @@ import type {FiberRoot, SuspenseHydrationCallbacks} from './ReactInternalTypes'; import type {ExpirationTime} from './ReactFiberExpirationTime.old'; import type {RootTag} from './ReactRootTags'; -import {noTimeout} from './ReactFiberHostConfig'; +import {noTimeout, supportsHydration} from './ReactFiberHostConfig'; import {createHostRootFiber} from './ReactFiber.old'; import {NoWork} from './ReactFiberExpirationTime.old'; import { @@ -45,7 +45,10 @@ function FiberRootNode(containerInfo, tag, hydrate) { this.lastPingedTime = NoWork; this.lastExpiredTime = NoWork; this.mutableSourceLastPendingUpdateTime = NoWork; - this.mutableSourceEagerHydrationData = []; + + if (supportsHydration) { + this.mutableSourceEagerHydrationData = null; + } if (enableSchedulerTracing) { this.interactionThreadID = unstable_getThreadID(); diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 1c9878f05cc55..b4374af8e2883 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -249,9 +249,9 @@ type BaseFiberRootProperties = {| mutableSourceLastPendingUpdateTime: ExpirationTime, // Used by useMutableSource hook to avoid tearing during hydrtaion. - mutableSourceEagerHydrationData: Array< + mutableSourceEagerHydrationData?: Array< MutableSource | MutableSourceVersion, - >, + > | null, // Only used by new reconciler diff --git a/packages/react-reconciler/src/ReactMutableSource.new.js b/packages/react-reconciler/src/ReactMutableSource.new.js index 10ca424cdd4be..61809d33f800d 100644 --- a/packages/react-reconciler/src/ReactMutableSource.new.js +++ b/packages/react-reconciler/src/ReactMutableSource.new.js @@ -100,5 +100,9 @@ export function registerMutableSourceForHydration( // TODO Clear this data once all pending hydration work is finished. // Retaining it forever may interfere with GC. - root.mutableSourceEagerHydrationData.push(mutableSource, version); + if (root.mutableSourceEagerHydrationData == null) { + root.mutableSourceEagerHydrationData = [mutableSource, version]; + } else { + root.mutableSourceEagerHydrationData.push(mutableSource, version); + } } diff --git a/packages/react-reconciler/src/ReactMutableSource.old.js b/packages/react-reconciler/src/ReactMutableSource.old.js index 0d8ea3d01c6dd..09af3f760f091 100644 --- a/packages/react-reconciler/src/ReactMutableSource.old.js +++ b/packages/react-reconciler/src/ReactMutableSource.old.js @@ -130,5 +130,9 @@ export function registerMutableSourceForHydration( // TODO Clear this data once all pending hydration work is finished. // Retaining it forever may interfere with GC. - root.mutableSourceEagerHydrationData.push(mutableSource, version); + if (root.mutableSourceEagerHydrationData == null) { + root.mutableSourceEagerHydrationData = [mutableSource, version]; + } else { + root.mutableSourceEagerHydrationData.push(mutableSource, version); + } } diff --git a/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js b/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js index 6868cd9b49cd4..e0563e0435043 100644 --- a/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js +++ b/packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js @@ -159,7 +159,9 @@ describe('useMutableSourceHydration', () => { const root = ReactDOM.unstable_createRoot(container, { hydrate: true, - mutableSources: [mutableSource], + hydrationOptions: { + mutableSources: [mutableSource], + }, }); act(() => { root.render(); @@ -194,7 +196,9 @@ describe('useMutableSourceHydration', () => { const root = ReactDOM.unstable_createRoot(container, { hydrate: true, - mutableSources: [mutableSource], + hydrationOptions: { + mutableSources: [mutableSource], + }, }); expect(() => { act(() => { @@ -244,7 +248,9 @@ describe('useMutableSourceHydration', () => { const root = ReactDOM.unstable_createRoot(container, { hydrate: true, - mutableSources: [mutableSource], + hydrationOptions: { + mutableSources: [mutableSource], + }, }); expect(() => { act(() => { @@ -295,7 +301,9 @@ describe('useMutableSourceHydration', () => { const root = ReactDOM.unstable_createRoot(container, { hydrate: true, - mutableSources: [mutableSource], + hydrationOptions: { + mutableSources: [mutableSource], + }, }); expect(() => { act(() => { @@ -361,7 +369,9 @@ describe('useMutableSourceHydration', () => { const root = ReactDOM.unstable_createRoot(container, { hydrate: true, - mutableSources: [mutableSource], + hydrationOptions: { + mutableSources: [mutableSource], + }, }); expect(() => { act(() => {