Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

useMutableSource hydration support #18771

Merged
merged 4 commits into from
May 21, 2020
Merged

useMutableSource hydration support #18771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d85a30c
useMutableSource hydration support
May 8, 2020
6d7ea6b
Remove unnecessary ReactMutableSource fork
May 8, 2020
eb3e705
Replaced root.registerMutableSourceForHydration() with mutableSources…
May 8, 2020
ec01142
Response to PR feedback:
May 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 23 additions & 61 deletions packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(<App show={false} />);
expect(Scheduler).toHaveYielded([]);
Expand All @@ -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__
? [
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',
]
: [
'Child with ID',
'Child with ID',
'Child with ID',
'Child One',
'Child Two',
],
);
]);

expect(child1Ref.current).toBe(null);
expect(childWithIDRef.current).toEqual(
Expand All @@ -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();

Expand Down Expand Up @@ -1606,9 +1596,7 @@ describe('ReactDOMServerHooks', () => {
ReactDOM.unstable_createRoot(container, {hydrate: true}).render(
<App />,
);
expect(() =>
expect(() => Scheduler.unstable_flushAll()).toThrow(),
).toErrorDev([
expect(() => Scheduler.unstable_flushAll()).toErrorDev([
'Warning: Expected server HTML to contain a matching <div> in <div>.',
]);
});
Expand Down Expand Up @@ -1694,14 +1682,12 @@ describe('ReactDOMServerHooks', () => {
ReactDOM.unstable_createRoot(container, {hydrate: true}).render(
<App />,
);
expect(() =>
expect(() => Scheduler.unstable_flushAll()).toThrow(),
).toErrorDev([
expect(() => Scheduler.unstable_flushAll()).toErrorDev([
'Warning: Expected server HTML to contain a matching <div> in <div>.',
]);
});

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 <div aria-labelledby={appId + ''} />;
}
Expand All @@ -1718,12 +1704,7 @@ describe('ReactDOMServerHooks', () => {
ReactDOM.unstable_createRoot(container, {hydrate: true}).render(
<App />,
);
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 <span> in <div>.',
Expand All @@ -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 <div aria-labelledby={appId + ''} />;
}
Expand All @@ -1749,12 +1730,7 @@ describe('ReactDOMServerHooks', () => {
ReactDOM.unstable_createRoot(container, {hydrate: true}).render(
<App />,
);
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 <span> in <div>.',
Expand All @@ -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 <div aria-labelledby={appId + ''} />;
}
Expand All @@ -1779,12 +1755,7 @@ describe('ReactDOMServerHooks', () => {
ReactDOM.unstable_createRoot(container, {hydrate: true}).render(
<App />,
);
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 <div> in <div>.',
Expand All @@ -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 <div aria-labelledby={id + ''} />;
Expand All @@ -1806,12 +1777,7 @@ describe('ReactDOMServerHooks', () => {
ReactDOM.unstable_createRoot(container, {hydrate: true}).render(
<App />,
);
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 <div> in <div>.',
Expand All @@ -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 <div aria-labelledby={appId + ''} />;
}
Expand All @@ -1842,16 +1808,14 @@ describe('ReactDOMServerHooks', () => {
<App />,
);

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. ' +
Expand All @@ -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 <div aria-labelledby={+appId} />;
}
Expand All @@ -1888,16 +1852,14 @@ describe('ReactDOMServerHooks', () => {
<App />,
);

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. ' +
Expand Down
19 changes: 17 additions & 2 deletions packages/react-dom/src/client/ReactDOMRoot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -25,6 +24,7 @@ export type RootOptions = {
hydrationOptions?: {
onHydrated?: (suspenseNode: Comment) => void,
onDeleted?: (suspenseNode: Comment) => void,
mutableSources?: Array<MutableSource<any>>,
...
bvaughn marked this conversation as resolved.
Show resolved Hide resolved
},
...
Expand All @@ -47,6 +47,8 @@ import {ensureListeningTo} from './ReactDOMComponent';
import {
createContainer,
updateContainer,
findHostInstanceWithNoPortals,
registerMutableSourceForHydration,
} from 'react-reconciler/src/ReactFiberReconciler';
import invariant from 'shared/invariant';
import {
Expand Down Expand Up @@ -124,6 +126,11 @@ function createRootImpl(
const hydrate = options != null && options.hydrate === true;
const hydrationCallbacks =
(options != null && options.hydrationOptions) || 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 All @@ -143,6 +150,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;
}

Expand Down
28 changes: 28 additions & 0 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -126,6 +127,7 @@ import {
isSuspenseInstancePending,
isSuspenseInstanceFallback,
registerSuspenseInstanceRetry,
supportsHydration,
} from './ReactFiberHostConfig';
import type {SuspenseInstance} from './ReactFiberHostConfig';
import {shouldSuspend} from './ReactFiberReconciler';
Expand Down Expand Up @@ -193,8 +195,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';

Expand Down Expand Up @@ -1071,6 +1077,20 @@ function updateHostRoot(current, workInProgress, renderLanes) {
// be any children to hydrate which is effectively the same thing as
// not hydrating.

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(
workInProgress,
null,
Expand Down Expand Up @@ -2253,6 +2273,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,
Expand Down
Loading