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.

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

Allow updating dehydrated root at lower priority without forcing client render #24082

Merged
merged 4 commits into from
Mar 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion packages/react-art/src/ReactART.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class Surface extends React.Component {
this._mountNode = createContainer(
this._surface,
LegacyRoot,
false,
null,
false,
false,
Expand Down
9 changes: 7 additions & 2 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2704,9 +2704,14 @@ export function attach(
// TODO: relying on this seems a bit fishy.
const wasMounted =
alternate.memoizedState != null &&
alternate.memoizedState.element != null;
alternate.memoizedState.element != null &&
// A dehydrated root is not considered mounted
alternate.memoizedState.isDehydrated !== true;
const isMounted =
current.memoizedState != null && current.memoizedState.element != null;
current.memoizedState != null &&
current.memoizedState.element != null &&
// A dehydrated root is not considered mounted
current.memoizedState.isDehydrated !== true;
if (!wasMounted && isMounted) {
// Mount a new root.
setRootPseudoKey(currentRootID, current);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

let JSDOM;
let React;
let startTransition;
let ReactDOMClient;
let Scheduler;
let clientAct;
Expand All @@ -33,6 +34,8 @@ describe('ReactDOMFizzShellHydration', () => {
ReactDOMFizzServer = require('react-dom/server');
Stream = require('stream');

startTransition = React.startTransition;

textCache = new Map();

// Test Environment
Expand Down Expand Up @@ -214,7 +217,36 @@ describe('ReactDOMFizzShellHydration', () => {
expect(container.textContent).toBe('Shell');
});

test('updating the root before the shell hydrates forces a client render', async () => {
test(
'updating the root at lower priority than initial hydration does not ' +
'force a client render',
async () => {
function App() {
return <Text text="Initial" />;
}

// Server render
await resolveText('Initial');
await serverAct(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
pipe(writable);
});
expect(Scheduler).toHaveYielded(['Initial']);

await clientAct(async () => {
const root = ReactDOMClient.hydrateRoot(container, <App />);
// This has lower priority than the initial hydration, so the update
// won't be processed until after hydration finishes.
startTransition(() => {
root.render(<Text text="Updated" />);
});
});
expect(Scheduler).toHaveYielded(['Initial', 'Updated']);
expect(container.textContent).toBe('Updated');
},
);

test('updating the root while the shell is suspended forces a client render', async () => {
function App() {
return <AsyncText text="Shell" />;
}
Expand Down Expand Up @@ -245,9 +277,9 @@ describe('ReactDOMFizzShellHydration', () => {
root.render(<Text text="New screen" />);
});
expect(Scheduler).toHaveYielded([
'New screen',
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These switched order because the recoverable error gets logged during the commit phase instead of immediately in the event handler.

'This root received an early update, before anything was able ' +
'hydrate. Switched the entire root to client rendering.',
'New screen',
]);
expect(container.textContent).toBe('New screen');
});
Expand Down
9 changes: 9 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMRoot-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,15 @@ describe('ReactDOMRoot', () => {
);
});

it('callback passed to legacy hydrate() API', () => {
container.innerHTML = '<div>Hi</div>';
ReactDOM.hydrate(<div>Hi</div>, container, () => {
Scheduler.unstable_yieldValue('callback');
});
expect(container.textContent).toEqual('Hi');
expect(Scheduler).toHaveYielded(['callback']);
});

it('warns when unmounting with legacy API (no previous content)', () => {
const root = ReactDOMClient.createRoot(container);
root.render(<div>Hi</div>);
Expand Down
119 changes: 79 additions & 40 deletions packages/react-dom/src/client/ReactDOMLegacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {

import {
createContainer,
createHydrationContainer,
findHostInstanceWithNoPortals,
updateContainer,
flushSync,
Expand Down Expand Up @@ -109,34 +110,81 @@ function noopOnRecoverableError() {

function legacyCreateRootFromDOMContainer(
container: Container,
forceHydrate: boolean,
initialChildren: ReactNodeList,
parentComponent: ?React$Component<any, any>,
callback: ?Function,
isHydrationContainer: boolean,
): FiberRoot {
// First clear any existing content.
if (!forceHydrate) {
if (isHydrationContainer) {
if (typeof callback === 'function') {
const originalCallback = callback;
callback = function() {
const instance = getPublicRootInstance(root);
originalCallback.call(instance);
};
}

const root = createHydrationContainer(
initialChildren,
callback,
container,
LegacyRoot,
null, // hydrationCallbacks
false, // isStrictMode
false, // concurrentUpdatesByDefaultOverride,
'', // identifierPrefix
noopOnRecoverableError,
// TODO(luna) Support hydration later
null,
);
container._reactRootContainer = root;
markContainerAsRoot(root.current, container);

const rootContainerElement =
container.nodeType === COMMENT_NODE ? container.parentNode : container;
listenToAllSupportedEvents(rootContainerElement);

flushSync();
return root;
} else {
// First clear any existing content.
let rootSibling;
while ((rootSibling = container.lastChild)) {
container.removeChild(rootSibling);
}
}

const root = createContainer(
container,
LegacyRoot,
forceHydrate,
null, // hydrationCallbacks
false, // isStrictMode
false, // concurrentUpdatesByDefaultOverride,
'', // identifierPrefix
noopOnRecoverableError, // onRecoverableError
null, // transitionCallbacks
);
markContainerAsRoot(root.current, container);
if (typeof callback === 'function') {
const originalCallback = callback;
callback = function() {
const instance = getPublicRootInstance(root);
originalCallback.call(instance);
};
}

const root = createContainer(
container,
LegacyRoot,
null, // hydrationCallbacks
false, // isStrictMode
false, // concurrentUpdatesByDefaultOverride,
'', // identifierPrefix
noopOnRecoverableError, // onRecoverableError
null, // transitionCallbacks
);
container._reactRootContainer = root;
markContainerAsRoot(root.current, container);

const rootContainerElement =
container.nodeType === COMMENT_NODE ? container.parentNode : container;
listenToAllSupportedEvents(rootContainerElement);

const rootContainerElement =
container.nodeType === COMMENT_NODE ? container.parentNode : container;
listenToAllSupportedEvents(rootContainerElement);
// Initial mount should not be batched.
flushSync(() => {
updateContainer(initialChildren, root, parentComponent, callback);
});

return root;
return root;
}
}

function warnOnInvalidCallback(callback: mixed, callerName: string): void {
Expand Down Expand Up @@ -164,39 +212,30 @@ function legacyRenderSubtreeIntoContainer(
warnOnInvalidCallback(callback === undefined ? null : callback, 'render');
}

let root = container._reactRootContainer;
let fiberRoot: FiberRoot;
if (!root) {
const maybeRoot = container._reactRootContainer;
let root: FiberRoot;
if (!maybeRoot) {
// Initial mount
root = container._reactRootContainer = legacyCreateRootFromDOMContainer(
root = legacyCreateRootFromDOMContainer(
container,
children,
parentComponent,
callback,
forceHydrate,
);
fiberRoot = root;
if (typeof callback === 'function') {
const originalCallback = callback;
callback = function() {
const instance = getPublicRootInstance(fiberRoot);
originalCallback.call(instance);
};
}
// Initial mount should not be batched.
flushSync(() => {
updateContainer(children, fiberRoot, parentComponent, callback);
});
} else {
fiberRoot = root;
root = maybeRoot;
if (typeof callback === 'function') {
const originalCallback = callback;
callback = function() {
const instance = getPublicRootInstance(fiberRoot);
const instance = getPublicRootInstance(root);
originalCallback.call(instance);
};
}
// Update
updateContainer(children, fiberRoot, parentComponent, callback);
updateContainer(children, root, parentComponent, callback);
}
return getPublicRootInstance(fiberRoot);
return getPublicRootInstance(root);
}

export function findDOMNode(
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/client/ReactDOMRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ export function createRoot(
const root = createContainer(
container,
ConcurrentRoot,
false,
null,
isStrictMode,
concurrentUpdatesByDefaultOverride,
Expand Down Expand Up @@ -303,6 +302,7 @@ export function hydrateRoot(

const root = createHydrationContainer(
initialChildren,
null,
container,
ConcurrentRoot,
hydrationCallbacks,
Expand Down
3 changes: 2 additions & 1 deletion packages/react-dom/src/events/ReactDOMEventListener.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import {
setCurrentUpdatePriority,
} from 'react-reconciler/src/ReactEventPriorities';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import {isRootDehydrated} from 'react-reconciler/src/ReactFiberShellHydration';

const {ReactCurrentBatchConfig} = ReactSharedInternals;

Expand Down Expand Up @@ -386,7 +387,7 @@ export function findInstanceBlockingEvent(
targetInst = null;
} else if (tag === HostRoot) {
const root: FiberRoot = nearestMounted.stateNode;
if (root.isDehydrated) {
if (isRootDehydrated(root)) {
// If this happens during a replay something went wrong and it might block
// the whole system.
return getContainerFromFiber(nearestMounted);
Expand Down
3 changes: 2 additions & 1 deletion packages/react-dom/src/events/ReactDOMEventReplaying.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
} from '../client/ReactDOMComponentTree';
import {HostRoot, SuspenseComponent} from 'react-reconciler/src/ReactWorkTags';
import {isHigherEventPriority} from 'react-reconciler/src/ReactEventPriorities';
import {isRootDehydrated} from 'react-reconciler/src/ReactFiberShellHydration';

let _attemptSynchronousHydration: (fiber: Object) => void;

Expand Down Expand Up @@ -414,7 +415,7 @@ function attemptExplicitHydrationTarget(
}
} else if (tag === HostRoot) {
const root: FiberRoot = nearestMounted.stateNode;
if (root.isDehydrated) {
if (isRootDehydrated(root)) {
queuedTarget.blockedOn = getContainerFromFiber(nearestMounted);
// We don't currently have a way to increase the priority of
// a root other than sync.
Expand Down
1 change: 0 additions & 1 deletion packages/react-native-renderer/src/ReactFabric.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@ function render(
root = createContainer(
containerTag,
concurrentRoot ? ConcurrentRoot : LegacyRoot,
false,
null,
false,
null,
Expand Down
1 change: 0 additions & 1 deletion packages/react-native-renderer/src/ReactNativeRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,6 @@ function render(
root = createContainer(
containerTag,
LegacyRoot,
false,
null,
false,
null,
Expand Down
3 changes: 0 additions & 3 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -974,7 +974,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
root = NoopRenderer.createContainer(
container,
tag,
false,
null,
null,
false,
Expand All @@ -996,7 +995,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
const fiberRoot = NoopRenderer.createContainer(
container,
ConcurrentRoot,
false,
null,
null,
false,
Expand Down Expand Up @@ -1029,7 +1027,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
const fiberRoot = NoopRenderer.createContainer(
container,
LegacyRoot,
false,
null,
null,
false,
Expand Down
Loading