Skip to content

Commit

Permalink
Response to PR feedback:
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
Brian Vaughn committed May 18, 2020
1 parent eb3e705 commit ec01142
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 31 deletions.
8 changes: 6 additions & 2 deletions packages/react-dom/src/client/ReactDOMRoot.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ export type RootOptions = {
hydrationOptions?: {
onHydrated?: (suspenseNode: Comment) => void,
onDeleted?: (suspenseNode: Comment) => void,
mutableSources?: Array<MutableSource<any>>,
...
},
mutableSources?: Array<MutableSource<any>>,
...
};

Expand Down Expand Up @@ -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;
Expand Down
21 changes: 13 additions & 8 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ import {
isSuspenseInstancePending,
isSuspenseInstanceFallback,
registerSuspenseInstanceRetry,
supportsHydration,
} from './ReactFiberHostConfig';
import type {SuspenseInstance} from './ReactFiberHostConfig';
import {shouldSuspend} from './ReactFiberReconciler';
Expand Down Expand Up @@ -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<any>);
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<any>);
const version = mutableSourceEagerHydrationData[i + 1];
setWorkInProgressVersion(mutableSource, version);
}
}
}

const child = mountChildFibers(
Expand Down
21 changes: 13 additions & 8 deletions packages/react-reconciler/src/ReactFiberBeginWork.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ import {
isSuspenseInstancePending,
isSuspenseInstanceFallback,
registerSuspenseInstanceRetry,
supportsHydration,
} from './ReactFiberHostConfig';
import type {SuspenseInstance} from './ReactFiberHostConfig';
import {shouldSuspend} from './ReactFiberReconciler';
Expand Down Expand Up @@ -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<any>);
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<any>);
const version = mutableSourceEagerHydrationData[i + 1];
setWorkInProgressVersion(mutableSource, version);
}
}
}

const child = mountChildFibers(
Expand Down
6 changes: 4 additions & 2 deletions packages/react-reconciler/src/ReactFiberRoot.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down
7 changes: 5 additions & 2 deletions packages/react-reconciler/src/ReactFiberRoot.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactInternalTypes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,9 @@ type BaseFiberRootProperties = {|
mutableSourceLastPendingUpdateTime: ExpirationTime,

// Used by useMutableSource hook to avoid tearing during hydrtaion.
mutableSourceEagerHydrationData: Array<
mutableSourceEagerHydrationData?: Array<
MutableSource<any> | MutableSourceVersion,
>,
> | null,

// Only used by new reconciler

Expand Down
6 changes: 5 additions & 1 deletion packages/react-reconciler/src/ReactMutableSource.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

This comment has been minimized.

Sign in to view

Copy link
@sebmarkbage

sebmarkbage May 21, 2020

Collaborator

This should also be gated on supportsHydration. Just so that the code can be stripped out.
root.mutableSourceEagerHydrationData = [mutableSource, version];
} else {
root.mutableSourceEagerHydrationData.push(mutableSource, version);
}
}
6 changes: 5 additions & 1 deletion packages/react-reconciler/src/ReactMutableSource.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
20 changes: 15 additions & 5 deletions packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,9 @@ describe('useMutableSourceHydration', () => {

const root = ReactDOM.unstable_createRoot(container, {
hydrate: true,
mutableSources: [mutableSource],
hydrationOptions: {
mutableSources: [mutableSource],
},
});
act(() => {
root.render(<TestComponent />);
Expand Down Expand Up @@ -194,7 +196,9 @@ describe('useMutableSourceHydration', () => {

const root = ReactDOM.unstable_createRoot(container, {
hydrate: true,
mutableSources: [mutableSource],
hydrationOptions: {
mutableSources: [mutableSource],
},
});
expect(() => {
act(() => {
Expand Down Expand Up @@ -244,7 +248,9 @@ describe('useMutableSourceHydration', () => {

const root = ReactDOM.unstable_createRoot(container, {
hydrate: true,
mutableSources: [mutableSource],
hydrationOptions: {
mutableSources: [mutableSource],
},
});
expect(() => {
act(() => {
Expand Down Expand Up @@ -295,7 +301,9 @@ describe('useMutableSourceHydration', () => {

const root = ReactDOM.unstable_createRoot(container, {
hydrate: true,
mutableSources: [mutableSource],
hydrationOptions: {
mutableSources: [mutableSource],
},
});
expect(() => {
act(() => {
Expand Down Expand Up @@ -361,7 +369,9 @@ describe('useMutableSourceHydration', () => {

const root = ReactDOM.unstable_createRoot(container, {
hydrate: true,
mutableSources: [mutableSource],
hydrationOptions: {
mutableSources: [mutableSource],
},
});
expect(() => {
act(() => {
Expand Down

0 comments on commit ec01142

Please sign in to comment.