Skip to content

Commit 6b5d9fd

Browse files
authored
Move traverseFragmentInstanceChildren to internal ReactFiberTreeReflection (#32613)
This is a nit but a Config should not have to know anything about the internals of Fibers. Ideally it shouldn't even access them but we have some cases where we need pointers back in like for this fragment. The way we've typically abstracted this is using the `ReactFiberTreeReflection` helper that's in the `react-reconciler`. Such as in the event system. https://github.com/facebook/react/blob/f3c956006a90dc68210bd3e19497d10fb9b028d3/packages/react-dom-bindings/src/events/ReactDOMEventListener.js#L22-L26 We sometimes cheat but we really should clean this up such that a `Fiber` is actually an opaque type to the Configs and it can never dot into it without using a helper. So this just moves `traverseFragmentInstanceChildren` to ReactFiberTreeReflection so that the ConfigDOM doesn't ever dot into its fields itself. It just passes the Fiber through back into the react-reconciler. I had to add a wrapper to read the `.child` to avoid that being assumed too. I also noticed that FragmentInstanceType is not actually passed through so that argument is unnecessary.
1 parent 2c56037 commit 6b5d9fd

File tree

2 files changed

+43
-45
lines changed

2 files changed

+43
-45
lines changed

Diff for: packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

+7-44
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import {getCurrentRootHostContainer} from 'react-reconciler/src/ReactFiberHostCo
3434
import hasOwnProperty from 'shared/hasOwnProperty';
3535
import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
3636
import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
37-
import {OffscreenComponent} from 'react-reconciler/src/ReactWorkTags';
3837

3938
export {
4039
setCurrentUpdatePriority,
@@ -54,6 +53,8 @@ import {
5453
markNodeAsHoistable,
5554
isOwnedInstance,
5655
} from './ReactDOMComponentTree';
56+
import {traverseFragmentInstance} from 'react-reconciler/src/ReactFiberTreeReflection';
57+
5758
export {detachDeletedInstance};
5859
import {hasRole} from './DOMAccessibilityRoles';
5960
import {
@@ -2221,9 +2222,8 @@ FragmentInstance.prototype.addEventListener = function (
22212222
indexOfEventListener(listeners, type, listener, optionsOrUseCapture) === -1;
22222223
if (isNewEventListener) {
22232224
listeners.push({type, listener, optionsOrUseCapture});
2224-
traverseFragmentInstanceChildren(
2225-
this,
2226-
this._fragmentFiber.child,
2225+
traverseFragmentInstance(
2226+
this._fragmentFiber,
22272227
addEventListenerToChild,
22282228
type,
22292229
listener,
@@ -2253,9 +2253,8 @@ FragmentInstance.prototype.removeEventListener = function (
22532253
return;
22542254
}
22552255
if (typeof listeners !== 'undefined' && listeners.length > 0) {
2256-
traverseFragmentInstanceChildren(
2257-
this,
2258-
this._fragmentFiber.child,
2256+
traverseFragmentInstance(
2257+
this._fragmentFiber,
22592258
removeEventListenerFromChild,
22602259
type,
22612260
listener,
@@ -2283,45 +2282,9 @@ function removeEventListenerFromChild(
22832282
}
22842283
// $FlowFixMe[prop-missing]
22852284
FragmentInstance.prototype.focus = function (this: FragmentInstanceType) {
2286-
traverseFragmentInstanceChildren(
2287-
this,
2288-
this._fragmentFiber.child,
2289-
setFocusIfFocusable,
2290-
);
2285+
traverseFragmentInstance(this._fragmentFiber, setFocusIfFocusable);
22912286
};
22922287

2293-
function traverseFragmentInstanceChildren<A, B, C>(
2294-
fragmentInstance: FragmentInstanceType,
2295-
child: Fiber | null,
2296-
fn: (Instance, A, B, C) => boolean,
2297-
a: A,
2298-
b: B,
2299-
c: C,
2300-
): void {
2301-
while (child !== null) {
2302-
if (child.tag === HostComponent) {
2303-
if (fn(child.stateNode, a, b, c)) {
2304-
return;
2305-
}
2306-
} else if (
2307-
child.tag === OffscreenComponent &&
2308-
child.memoizedState !== null
2309-
) {
2310-
// Skip hidden subtrees
2311-
} else {
2312-
traverseFragmentInstanceChildren(
2313-
fragmentInstance,
2314-
child.child,
2315-
fn,
2316-
a,
2317-
b,
2318-
c,
2319-
);
2320-
}
2321-
child = child.sibling;
2322-
}
2323-
}
2324-
23252288
function normalizeListenerOptions(
23262289
opts: ?EventListenerOptionsOrUseCapture,
23272290
): string {

Diff for: packages/react-reconciler/src/ReactFiberTreeReflection.js

+36-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99

1010
import type {Fiber} from './ReactInternalTypes';
11-
import type {Container, SuspenseInstance} from './ReactFiberConfig';
11+
import type {Container, SuspenseInstance, Instance} from './ReactFiberConfig';
1212
import type {SuspenseState} from './ReactFiberSuspenseComponent';
1313

1414
import {
@@ -19,6 +19,7 @@ import {
1919
HostPortal,
2020
HostText,
2121
SuspenseComponent,
22+
OffscreenComponent,
2223
} from './ReactWorkTags';
2324
import {NoFlags, Placement, Hydrating} from './ReactFiberFlags';
2425

@@ -317,3 +318,37 @@ export function doesFiberContain(
317318
}
318319
return false;
319320
}
321+
322+
export function traverseFragmentInstance<A, B, C>(
323+
fragmentFiber: Fiber,
324+
fn: (Instance, A, B, C) => boolean,
325+
a: A,
326+
b: B,
327+
c: C,
328+
): void {
329+
return traverseFragmentInstanceChildren(fragmentFiber.child, fn, a, b, c);
330+
}
331+
332+
function traverseFragmentInstanceChildren<A, B, C>(
333+
child: Fiber | null,
334+
fn: (Instance, A, B, C) => boolean,
335+
a: A,
336+
b: B,
337+
c: C,
338+
): void {
339+
while (child !== null) {
340+
if (child.tag === HostComponent) {
341+
if (fn(child.stateNode, a, b, c)) {
342+
return;
343+
}
344+
} else if (
345+
child.tag === OffscreenComponent &&
346+
child.memoizedState !== null
347+
) {
348+
// Skip hidden subtrees
349+
} else {
350+
traverseFragmentInstanceChildren(child.child, fn, a, b, c);
351+
}
352+
child = child.sibling;
353+
}
354+
}

0 commit comments

Comments
 (0)