Skip to content

Commit 8d7c733

Browse files
authored
[Partial Hydration] Don't invoke listeners on parent of dehydrated event target (#16591)
* Don't invoke listeners on parent of dehydrated event target * Move Suspense boundary check to getClosestInstanceFromNode Now getClosestInstanceFromNode can return either a host component, host text component or suspense component when the suspense component is dehydrated. We then use that to ignore events on a suspense component. * Attach the HostRoot fiber to the DOM container This lets us detect if an event happens on this root's subtree before it has rendered something. * Add todo The approach of checking isFiberMounted answers if we might be in an in-progress hydration but it doesn't answer which root or boundary might be in-progress so we don't know what to wait for. This needs some refactoring. * Refactor isFiberMountedImpl to getNearestMountedFiber We'll need the nearest boundary for event replaying so this prepares for that. This surfaced an issue that we attach Hydrating tag on the root but normally this (and Placement) is attached on the child. This surfaced an issue that this can lead to both Placement and Hydrating effects which is not supported so we need to ensure that we only ever use one or the other. * Add todo for bug I spotted * Cache tags * Check the ContainerInstanceKey before the InstanceKey The container is inside the instance, so we must find it before the instance, since otherwise we'll miss it.
1 parent 9ce8711 commit 8d7c733

14 files changed

+444
-65
lines changed

packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1895,4 +1895,166 @@ describe('ReactDOMServerPartialHydration', () => {
18951895

18961896
document.body.removeChild(container);
18971897
});
1898+
1899+
it('does not invoke the parent of dehydrated boundary event', async () => {
1900+
let suspend = false;
1901+
let resolve;
1902+
let promise = new Promise(resolvePromise => (resolve = resolvePromise));
1903+
1904+
let clicksOnParent = 0;
1905+
let clicksOnChild = 0;
1906+
1907+
function Child({text}) {
1908+
if (suspend) {
1909+
throw promise;
1910+
} else {
1911+
return (
1912+
<span
1913+
onClick={e => {
1914+
// The stopPropagation is showing an example why invoking
1915+
// the event on only a parent might not be correct.
1916+
e.stopPropagation();
1917+
clicksOnChild++;
1918+
}}>
1919+
Hello
1920+
</span>
1921+
);
1922+
}
1923+
}
1924+
1925+
function App() {
1926+
return (
1927+
<div onClick={() => clicksOnParent++}>
1928+
<Suspense fallback="Loading...">
1929+
<Child />
1930+
</Suspense>
1931+
</div>
1932+
);
1933+
}
1934+
1935+
suspend = false;
1936+
let finalHTML = ReactDOMServer.renderToString(<App />);
1937+
let container = document.createElement('div');
1938+
container.innerHTML = finalHTML;
1939+
1940+
// We need this to be in the document since we'll dispatch events on it.
1941+
document.body.appendChild(container);
1942+
1943+
let span = container.getElementsByTagName('span')[0];
1944+
1945+
// On the client we don't have all data yet but we want to start
1946+
// hydrating anyway.
1947+
suspend = true;
1948+
let root = ReactDOM.unstable_createRoot(container, {hydrate: true});
1949+
root.render(<App />);
1950+
Scheduler.unstable_flushAll();
1951+
jest.runAllTimers();
1952+
1953+
// We're now partially hydrated.
1954+
span.click();
1955+
expect(clicksOnChild).toBe(0);
1956+
expect(clicksOnParent).toBe(0);
1957+
1958+
// Resolving the promise so that rendering can complete.
1959+
suspend = false;
1960+
resolve();
1961+
await promise;
1962+
1963+
Scheduler.unstable_flushAll();
1964+
jest.runAllTimers();
1965+
1966+
// TODO: With selective hydration the event should've been replayed
1967+
// but for now we'll have to issue it again.
1968+
act(() => {
1969+
span.click();
1970+
});
1971+
1972+
expect(clicksOnChild).toBe(1);
1973+
// This will be zero due to the stopPropagation.
1974+
expect(clicksOnParent).toBe(0);
1975+
1976+
document.body.removeChild(container);
1977+
});
1978+
1979+
it('does not invoke an event on a parent tree when a subtree is dehydrated', async () => {
1980+
let suspend = false;
1981+
let resolve;
1982+
let promise = new Promise(resolvePromise => (resolve = resolvePromise));
1983+
1984+
let clicks = 0;
1985+
let childSlotRef = React.createRef();
1986+
1987+
function Parent() {
1988+
return <div onClick={() => clicks++} ref={childSlotRef} />;
1989+
}
1990+
1991+
function Child({text}) {
1992+
if (suspend) {
1993+
throw promise;
1994+
} else {
1995+
return <a>Click me</a>;
1996+
}
1997+
}
1998+
1999+
function App() {
2000+
// The root is a Suspense boundary.
2001+
return (
2002+
<Suspense fallback="Loading...">
2003+
<Child />
2004+
</Suspense>
2005+
);
2006+
}
2007+
2008+
suspend = false;
2009+
let finalHTML = ReactDOMServer.renderToString(<App />);
2010+
2011+
let parentContainer = document.createElement('div');
2012+
let childContainer = document.createElement('div');
2013+
2014+
// We need this to be in the document since we'll dispatch events on it.
2015+
document.body.appendChild(parentContainer);
2016+
2017+
// We're going to use a different root as a parent.
2018+
// This lets us detect whether an event goes through React's event system.
2019+
let parentRoot = ReactDOM.unstable_createRoot(parentContainer);
2020+
parentRoot.render(<Parent />);
2021+
Scheduler.unstable_flushAll();
2022+
2023+
childSlotRef.current.appendChild(childContainer);
2024+
2025+
childContainer.innerHTML = finalHTML;
2026+
2027+
let a = childContainer.getElementsByTagName('a')[0];
2028+
2029+
suspend = true;
2030+
2031+
// Hydrate asynchronously.
2032+
let root = ReactDOM.unstable_createRoot(childContainer, {hydrate: true});
2033+
root.render(<App />);
2034+
jest.runAllTimers();
2035+
Scheduler.unstable_flushAll();
2036+
2037+
// The Suspense boundary is not yet hydrated.
2038+
a.click();
2039+
expect(clicks).toBe(0);
2040+
2041+
// Resolving the promise so that rendering can complete.
2042+
suspend = false;
2043+
resolve();
2044+
await promise;
2045+
2046+
Scheduler.unstable_flushAll();
2047+
jest.runAllTimers();
2048+
2049+
// We're now full hydrated.
2050+
// TODO: With selective hydration the event should've been replayed
2051+
// but for now we'll have to issue it again.
2052+
act(() => {
2053+
a.click();
2054+
});
2055+
2056+
expect(clicks).toBe(1);
2057+
2058+
document.body.removeChild(parentContainer);
2059+
});
18982060
});

packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,4 +586,62 @@ describe('ReactDOMServerHydration', () => {
586586

587587
document.body.removeChild(container);
588588
});
589+
590+
it('does not invoke an event on a parent tree when a subtree is hydrating', () => {
591+
let clicks = 0;
592+
let childSlotRef = React.createRef();
593+
594+
function Parent() {
595+
return <div onClick={() => clicks++} ref={childSlotRef} />;
596+
}
597+
598+
function App() {
599+
return (
600+
<div>
601+
<a>Click me</a>
602+
</div>
603+
);
604+
}
605+
606+
let finalHTML = ReactDOMServer.renderToString(<App />);
607+
608+
let parentContainer = document.createElement('div');
609+
let childContainer = document.createElement('div');
610+
611+
// We need this to be in the document since we'll dispatch events on it.
612+
document.body.appendChild(parentContainer);
613+
614+
// We're going to use a different root as a parent.
615+
// This lets us detect whether an event goes through React's event system.
616+
let parentRoot = ReactDOM.unstable_createRoot(parentContainer);
617+
parentRoot.render(<Parent />);
618+
Scheduler.unstable_flushAll();
619+
620+
childSlotRef.current.appendChild(childContainer);
621+
622+
childContainer.innerHTML = finalHTML;
623+
624+
let a = childContainer.getElementsByTagName('a')[0];
625+
626+
// Hydrate asynchronously.
627+
let root = ReactDOM.unstable_createRoot(childContainer, {hydrate: true});
628+
root.render(<App />);
629+
// Nothing has rendered so far.
630+
631+
a.click();
632+
expect(clicks).toBe(0);
633+
634+
Scheduler.unstable_flushAll();
635+
636+
// We're now full hydrated.
637+
// TODO: With selective hydration the event should've been replayed
638+
// but for now we'll have to issue it again.
639+
act(() => {
640+
a.click();
641+
});
642+
643+
expect(clicks).toBe(1);
644+
645+
document.body.removeChild(parentContainer);
646+
});
589647
});

packages/react-dom/src/client/ReactDOM.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ import {
7070
getNodeFromInstance,
7171
getFiberCurrentPropsFromNode,
7272
getClosestInstanceFromNode,
73+
markContainerAsRoot,
7374
} from './ReactDOMComponentTree';
7475
import {restoreControlledState} from './ReactDOMComponent';
7576
import {dispatchEvent} from '../events/ReactDOMEventListener';
@@ -375,6 +376,7 @@ function ReactSyncRoot(
375376
(options != null && options.hydrationOptions) || null;
376377
const root = createContainer(container, tag, hydrate, hydrationCallbacks);
377378
this._internalRoot = root;
379+
markContainerAsRoot(root.current, container);
378380
}
379381

380382
function ReactRoot(container: DOMContainer, options: void | RootOptions) {
@@ -388,6 +390,7 @@ function ReactRoot(container: DOMContainer, options: void | RootOptions) {
388390
hydrationCallbacks,
389391
);
390392
this._internalRoot = root;
393+
markContainerAsRoot(root.current, container);
391394
}
392395

393396
ReactRoot.prototype.render = ReactSyncRoot.prototype.render = function(

packages/react-dom/src/client/ReactDOMComponentTree.js

Lines changed: 74 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,43 +8,93 @@
88
import {HostComponent, HostText} from 'shared/ReactWorkTags';
99
import invariant from 'shared/invariant';
1010

11+
import {getParentSuspenseInstance} from './ReactDOMHostConfig';
12+
1113
const randomKey = Math.random()
1214
.toString(36)
1315
.slice(2);
1416
const internalInstanceKey = '__reactInternalInstance$' + randomKey;
1517
const internalEventHandlersKey = '__reactEventHandlers$' + randomKey;
18+
const internalContainerInstanceKey = '__reactContainere$' + randomKey;
1619

1720
export function precacheFiberNode(hostInst, node) {
1821
node[internalInstanceKey] = hostInst;
1922
}
2023

21-
/**
22-
* Given a DOM node, return the closest ReactDOMComponent or
23-
* ReactDOMTextComponent instance ancestor.
24-
*/
25-
export function getClosestInstanceFromNode(node) {
26-
let inst = node[internalInstanceKey];
27-
if (inst) {
28-
return inst;
29-
}
24+
export function markContainerAsRoot(hostRoot, node) {
25+
node[internalContainerInstanceKey] = hostRoot;
26+
}
3027

31-
do {
32-
node = node.parentNode;
33-
if (node) {
34-
inst = node[internalInstanceKey];
35-
} else {
36-
// Top of the tree. This node must not be part of a React tree (or is
37-
// unmounted, potentially).
38-
return null;
28+
// Given a DOM node, return the closest HostComponent or HostText fiber ancestor.
29+
// If the target node is part of a hydrated or not yet rendered subtree, then
30+
// this may also return a SuspenseComponent or HostRoot to indicate that.
31+
// Conceptually the HostRoot fiber is a child of the Container node. So if you
32+
// pass the Container node as the targetNode, you wiill not actually get the
33+
// HostRoot back. To get to the HostRoot, you need to pass a child of it.
34+
// The same thing applies to Suspense boundaries.
35+
export function getClosestInstanceFromNode(targetNode) {
36+
let targetInst = targetNode[internalInstanceKey];
37+
if (targetInst) {
38+
// Don't return HostRoot or SuspenseComponent here.
39+
return targetInst;
40+
}
41+
// If the direct event target isn't a React owned DOM node, we need to look
42+
// to see if one of its parents is a React owned DOM node.
43+
let parentNode = targetNode.parentNode;
44+
while (parentNode) {
45+
// We'll check if this is a container root that could include
46+
// React nodes in the future. We need to check this first because
47+
// if we're a child of a dehydrated container, we need to first
48+
// find that inner container before moving on to finding the parent
49+
// instance. Note that we don't check this field on the targetNode
50+
// itself because the fibers are conceptually between the container
51+
// node and the first child. It isn't surrounding the container node.
52+
targetInst = parentNode[internalContainerInstanceKey];
53+
if (targetInst) {
54+
// If so, we return the HostRoot Fiber.
55+
return targetInst;
3956
}
40-
} while (!inst);
57+
targetInst = parentNode[internalInstanceKey];
58+
if (targetInst) {
59+
// Since this wasn't the direct target of the event, we might have
60+
// stepped past dehydrated DOM nodes to get here. However they could
61+
// also have been non-React nodes. We need to answer which one.
4162

42-
let tag = inst.tag;
43-
switch (tag) {
44-
case HostComponent:
45-
case HostText:
46-
// In Fiber, this will always be the deepest root.
47-
return inst;
63+
// If we the instance doesn't have any children, then there can't be
64+
// a nested suspense boundary within it. So we can use this as a fast
65+
// bailout. Most of the time, when people add non-React children to
66+
// the tree, it is using a ref to a child-less DOM node.
67+
// We only need to check one of the fibers because if it has ever
68+
// gone from having children to deleting them or vice versa it would
69+
// have deleted the dehydrated boundary nested inside already.
70+
if (targetInst.child !== null) {
71+
// Next we need to figure out if the node that skipped past is
72+
// nested within a dehydrated boundary and if so, which one.
73+
let suspenseInstance = getParentSuspenseInstance(targetNode);
74+
if (suspenseInstance !== null) {
75+
// We found a suspense instance. That means that we haven't
76+
// hydrated it yet. Even though we leave the comments in the
77+
// DOM after hydrating, and there are boundaries in the DOM
78+
// that could already be hydrated, we wouldn't have found them
79+
// through this pass since if the target is hydrated it would
80+
// have had an internalInstanceKey on it.
81+
// Let's get the fiber associated with the SuspenseComponent
82+
// as the deepest instance.
83+
let targetSuspenseInst = suspenseInstance[internalInstanceKey];
84+
if (targetSuspenseInst) {
85+
return targetSuspenseInst;
86+
}
87+
// If we don't find a Fiber on the comment, it might be because
88+
// we haven't gotten to hydrate it yet. That should mean that
89+
// the parent component also hasn't hydrated yet but we can
90+
// just return that since it will bail out on the isMounted
91+
// check.
92+
}
93+
}
94+
return targetInst;
95+
}
96+
targetNode = parentNode;
97+
parentNode = targetNode.parentNode;
4898
}
4999
return null;
50100
}

0 commit comments

Comments
 (0)