Skip to content

Commit b053396

Browse files
committed
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.
1 parent 5b2a0fb commit b053396

File tree

8 files changed

+102
-78
lines changed

8 files changed

+102
-78
lines changed

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

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
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);
@@ -22,29 +24,56 @@ export function precacheFiberNode(hostInst, node) {
2224
* Given a DOM node, return the closest ReactDOMComponent or
2325
* ReactDOMTextComponent instance ancestor.
2426
*/
25-
export function getClosestInstanceFromNode(node) {
26-
let inst = node[internalInstanceKey];
27-
if (inst) {
28-
return inst;
27+
export function getClosestInstanceFromNode(targetNode) {
28+
let targetInst = targetNode[internalInstanceKey];
29+
if (targetInst) {
30+
return targetInst;
2931
}
32+
// If the direct event target isn't a React owned DOM node, we need to look
33+
// to see if one of its parents is a React owned DOM node.
34+
let parentNode = targetNode.parentNode;
35+
while (parentNode) {
36+
targetInst = parentNode[internalInstanceKey];
37+
if (targetInst) {
38+
// Since this wasn't the direct target of the event, we might have
39+
// stepped past dehydrated DOM nodes to get here. However they could
40+
// also have been non-React nodes. We need to answer which one.
3041

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;
42+
// If we the instance doesn't have any children, then there can't be
43+
// a nested suspense boundary within it. So we can use this as a fast
44+
// bailout. Most of the time, when people add non-React children to
45+
// the tree, it is using a ref to a child-less DOM node.
46+
// We only need to check one of the fibers because if it has ever
47+
// gone from having children to deleting them or vice versa it would
48+
// have deleted the dehydrated boundary nested inside already.
49+
if (targetInst.child !== null) {
50+
// Next we need to figure out if the node that skipped past is
51+
// nested within a dehydrated boundary and if so, which one.
52+
let suspenseInstance = getParentSuspenseInstance(targetNode);
53+
if (suspenseInstance !== null) {
54+
// We found a suspense instance. That means that we haven't
55+
// hydrated it yet. Even though we leave the comments in the
56+
// DOM after hydrating, and there are boundaries in the DOM
57+
// that could already be hydrated, we wouldn't have found them
58+
// through this pass since if the target is hydrated it would
59+
// have had an internalInstanceKey on it.
60+
// Let's get the fiber associated with the SuspenseComponent
61+
// as the deepest instance.
62+
let targetSuspenseInst = suspenseInstance[internalInstanceKey];
63+
if (targetSuspenseInst) {
64+
return targetSuspenseInst;
65+
}
66+
// If we don't find a Fiber on the comment, it might be because
67+
// we haven't gotten to hydrate it yet. That should mean that
68+
// the parent component also hasn't hydrated yet but we can
69+
// just return that since it will bail out on the isMounted
70+
// check.
71+
}
72+
}
73+
return targetInst;
3974
}
40-
} while (!inst);
41-
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;
75+
targetNode = parentNode;
76+
parentNode = targetNode.parentNode;
4877
}
4978
return null;
5079
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,13 @@ export function hydrateTextInstance(
673673
return diffHydratedText(textInstance, text);
674674
}
675675

676+
export function hydrateSuspenseInstance(
677+
suspenseInstance: SuspenseInstance,
678+
internalInstanceHandle: Object,
679+
) {
680+
precacheFiberNode(internalInstanceHandle, suspenseInstance);
681+
}
682+
676683
export function getNextHydratableInstanceAfterSuspenseInstance(
677684
suspenseInstance: SuspenseInstance,
678685
): null | HydratableInstance {

packages/react-dom/src/events/ReactDOMEventListener.js

Lines changed: 18 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {
2323
import {runExtractedPluginEventsInBatch} from 'legacy-events/EventPluginHub';
2424
import {dispatchEventForResponderEventSystem} from '../events/DOMEventResponderSystem';
2525
import {isFiberMounted} from 'react-reconciler/reflection';
26-
import {HostRoot} from 'shared/ReactWorkTags';
26+
import {HostRoot, SuspenseComponent} from 'shared/ReactWorkTags';
2727
import {
2828
type EventSystemFlags,
2929
PLUGIN_EVENT_SYSTEM,
@@ -39,14 +39,10 @@ import {
3939
addEventCaptureListenerWithPassiveFlag,
4040
} from './EventListener';
4141
import getEventTarget from './getEventTarget';
42-
import {
43-
getClosestInstanceFromNode,
44-
getInstanceFromNode,
45-
} from '../client/ReactDOMComponentTree';
42+
import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree';
4643
import SimpleEventPlugin from './SimpleEventPlugin';
4744
import {getRawEventName} from './DOMTopLevelEventTypes';
4845
import {passiveBrowserEventsSupported} from './checkPassiveEvents';
49-
import {getParentSuspenseInstance} from '../client/ReactDOMHostConfig';
5046

5147
import {
5248
enableFlareAPI,
@@ -313,61 +309,26 @@ export function dispatchEvent(
313309
return;
314310
}
315311
const nativeEventTarget = getEventTarget(nativeEvent);
316-
317-
// This is effectively an inlined version of getClosestInstanceFromNode with
318-
// some special semantics for dehydrated Suspense boundaries.
319-
let targetInst = getInstanceFromNode(nativeEventTarget);
320-
if (targetInst === null) {
321-
// If the direct event target isn't a React owned DOM node, we need to look
322-
// to see if one of its parents is a React owned DOM node.
323-
let targetNode = nativeEventTarget;
324-
let parentNode = targetNode.parentNode;
325-
while (parentNode) {
326-
targetInst = getInstanceFromNode(parentNode);
327-
if (targetInst !== null) {
328-
// Since this wasn't the direct target of the event, we might have
329-
// stepped past dehydrated DOM nodes to get here. However they could
330-
// also have been non-React nodes. We need to answer which one.
331-
332-
// If we the instance doesn't have any children, then there can't be
333-
// a nested suspense boundary within it. So we can use this as a fast
334-
// bailout. Most of the time, when people add non-React children to
335-
// the tree, it is using a ref to a child-less DOM node.
336-
// We only need to check one of the fibers because if it has ever
337-
// gone from having children to deleting them or vice versa it would
338-
// have deleted the dehydrated boundary nested inside already.
339-
if (targetInst.child !== null) {
340-
// Next we need to figure out if the node that skipped past is
341-
// nested within a dehydrated boundary and if so, which one.
342-
let suspenseInstance = getParentSuspenseInstance(targetNode);
343-
if (suspenseInstance !== null) {
344-
// We found a suspense instance. That means that we haven't
345-
// hydrated it yet. Even though we leave the comments in the
346-
// DOM after hydrating, and there are boundaries in the DOM
347-
// that could already be hydrated, we wouldn't have found them
348-
// through this pass since if the target is hydrated it would
349-
// have had an internalInstanceKey on it.
350-
// We're going to proceed as if there was no target yet.
351-
// TODO: This is a good opportunity to schedule a replay of
352-
// the event instead once this boundary has been hydrated.
353-
targetInst = null;
354-
}
355-
}
356-
break;
312+
let targetInst = getClosestInstanceFromNode(nativeEventTarget);
313+
314+
if (targetInst !== null) {
315+
if (isFiberMounted(targetInst)) {
316+
if (targetInst.tag === SuspenseComponent) {
317+
// TODO: This is a good opportunity to schedule a replay of
318+
// the event instead once this boundary has been hydrated.
319+
// For now we're going to just ignore this event as if it's
320+
// not mounted.
321+
targetInst = null;
357322
}
358-
targetNode = parentNode;
359-
parentNode = targetNode.parent;
323+
} else {
324+
// If we get an event (ex: img onload) before committing that
325+
// component's mount, ignore it for now (that is, treat it as if it was an
326+
// event on a non-React tree). We might also consider queueing events and
327+
// dispatching them after the mount.
328+
targetInst = null;
360329
}
361330
}
362331

363-
if (targetInst !== null && !isFiberMounted(targetInst)) {
364-
// If we get an event (ex: img onload) before committing that
365-
// component's mount, ignore it for now (that is, treat it as if it was an
366-
// event on a non-React tree). We might also consider queueing events and
367-
// dispatching them after the mount.
368-
targetInst = null;
369-
}
370-
371332
if (enableFlareAPI) {
372333
if (eventSystemFlags === PLUGIN_EVENT_SYSTEM) {
373334
dispatchEventForPluginEventSystem(

packages/react-reconciler/src/ReactFiberCompleteWork.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ import {popProvider} from './ReactFiberNewContext';
103103
import {
104104
prepareToHydrateHostInstance,
105105
prepareToHydrateHostTextInstance,
106+
prepareToHydrateHostSuspenseInstance,
106107
popHydrationState,
107108
resetHydrationState,
108109
} from './ReactFiberHydrationContext';
@@ -813,6 +814,7 @@ function completeWork(
813814
'A dehydrated suspense component was completed without a hydrated node. ' +
814815
'This is probably a bug in React.',
815816
);
817+
prepareToHydrateHostSuspenseInstance(workInProgress);
816818
if (enableSchedulerTracing) {
817819
markSpawnedWork(Never);
818820
}

packages/react-reconciler/src/ReactFiberHydrationContext.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import {
4141
getFirstHydratableChild,
4242
hydrateInstance,
4343
hydrateTextInstance,
44+
hydrateSuspenseInstance,
4445
getNextHydratableInstanceAfterSuspenseInstance,
4546
didNotMatchHydratedContainerTextInstance,
4647
didNotMatchHydratedTextInstance,
@@ -370,6 +371,26 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean {
370371
return shouldUpdate;
371372
}
372373

374+
function prepareToHydrateHostSuspenseInstance(fiber: Fiber): void {
375+
if (!supportsHydration) {
376+
invariant(
377+
false,
378+
'Expected prepareToHydrateHostSuspenseInstance() to never be called. ' +
379+
'This error is likely caused by a bug in React. Please file an issue.',
380+
);
381+
}
382+
383+
let suspenseState: null | SuspenseState = fiber.memoizedState;
384+
let suspenseInstance: null | SuspenseInstance =
385+
suspenseState !== null ? suspenseState.dehydrated : null;
386+
invariant(
387+
suspenseInstance,
388+
'Expected to have a hydrated suspense instance. ' +
389+
'This error is likely caused by a bug in React. Please file an issue.',
390+
);
391+
hydrateSuspenseInstance(suspenseInstance, fiber);
392+
}
393+
373394
function skipPastDehydratedSuspenseInstance(
374395
fiber: Fiber,
375396
): null | HydratableInstance {
@@ -471,5 +492,6 @@ export {
471492
tryToClaimNextHydratableInstance,
472493
prepareToHydrateHostInstance,
473494
prepareToHydrateHostTextInstance,
495+
prepareToHydrateHostSuspenseInstance,
474496
popHydrationState,
475497
};

packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ export const getNextHydratableSibling = $$$hostConfig.getNextHydratableSibling;
126126
export const getFirstHydratableChild = $$$hostConfig.getFirstHydratableChild;
127127
export const hydrateInstance = $$$hostConfig.hydrateInstance;
128128
export const hydrateTextInstance = $$$hostConfig.hydrateTextInstance;
129+
export const hydrateSuspenseInstance = $$$hostConfig.hydrateSuspenseInstance;
129130
export const getNextHydratableInstanceAfterSuspenseInstance =
130131
$$$hostConfig.getNextHydratableInstanceAfterSuspenseInstance;
131132
export const clearSuspenseBoundary = $$$hostConfig.clearSuspenseBoundary;

packages/shared/HostConfigWithNoHydration.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export const getNextHydratableSibling = shim;
3434
export const getFirstHydratableChild = shim;
3535
export const hydrateInstance = shim;
3636
export const hydrateTextInstance = shim;
37+
export const hydrateSuspenseInstance = shim;
3738
export const getNextHydratableInstanceAfterSuspenseInstance = shim;
3839
export const clearSuspenseBoundary = shim;
3940
export const clearSuspenseBoundaryFromContainer = shim;

scripts/error-codes/codes.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,5 +340,6 @@
340340
"339": "An invalid value was used as an event listener. Expect one or many event listeners created via React.unstable_useResponder().",
341341
"340": "Threw in newly mounted dehydrated component. This is likely a bug in React. Please file an issue.",
342342
"341": "We just came from a parent so we must have had a parent. This is a bug in React.",
343-
"342": "A React component suspended while rendering, but no fallback UI was specified.\n\nAdd a <Suspense fallback=...> component higher in the tree to provide a loading indicator or placeholder to display."
343+
"342": "A React component suspended while rendering, but no fallback UI was specified.\n\nAdd a <Suspense fallback=...> component higher in the tree to provide a loading indicator or placeholder to display.",
344+
"343": "Expected prepareToHydrateHostSuspenseInstance() to never be called. This error is likely caused by a bug in React. Please file an issue."
344345
}

0 commit comments

Comments
 (0)