@samitouri / QOS-React / commits / 6b5d9fd316

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.

Sebastian Markbåge committed Mar 14, 2025 at 17:38 UTC 6b5d9fd3166eb58b469fb23f7b96972b184c0218
2 files changed +43 -45
packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
+7 -44
@@ -34,7 +34,6 @@ import {getCurrentRootHostContainer} from 'react-reconciler/src/ReactFiberHostCo
34 import hasOwnProperty from 'shared/hasOwnProperty';
35 import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
36 import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
37 -import {OffscreenComponent} from 'react-reconciler/src/ReactWorkTags';
37
38 export {
39 setCurrentUpdatePriority,
@@ -54,6 +53,8 @@ import {
53 markNodeAsHoistable,
54 isOwnedInstance,
55 } from './ReactDOMComponentTree';
56 +import {traverseFragmentInstance} from 'react-reconciler/src/ReactFiberTreeReflection';
57 +
58 export {detachDeletedInstance};
59 import {hasRole} from './DOMAccessibilityRoles';
60 import {
@@ -2221,9 +2222,8 @@ FragmentInstance.prototype.addEventListener = function (
2222 indexOfEventListener(listeners, type, listener, optionsOrUseCapture) === -1;
2223 if (isNewEventListener) {
2224 listeners.push({type, listener, optionsOrUseCapture});
2224 - traverseFragmentInstanceChildren(
2225 - this,
2226 - this._fragmentFiber.child,
2225 + traverseFragmentInstance(
2226 + this._fragmentFiber,
2227 addEventListenerToChild,
2228 type,
2229 listener,
@@ -2253,9 +2253,8 @@ FragmentInstance.prototype.removeEventListener = function (
2253 return;
2254 }
2255 if (typeof listeners !== 'undefined' && listeners.length > 0) {
2256 - traverseFragmentInstanceChildren(
2257 - this,
2258 - this._fragmentFiber.child,
2256 + traverseFragmentInstance(
2257 + this._fragmentFiber,
2258 removeEventListenerFromChild,
2259 type,
2260 listener,
@@ -2283,45 +2282,9 @@ function removeEventListenerFromChild(
2282 }
2283 // $FlowFixMe[prop-missing]
2284 FragmentInstance.prototype.focus = function (this: FragmentInstanceType) {
2286 - traverseFragmentInstanceChildren(
2287 - this,
2288 - this._fragmentFiber.child,
2289 - setFocusIfFocusable,
2290 - );
2285 + traverseFragmentInstance(this._fragmentFiber, setFocusIfFocusable);
2286 };
2287
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 -
2288 function normalizeListenerOptions(
2289 opts: ?EventListenerOptionsOrUseCapture,
2290 ): string {
packages/react-reconciler/src/ReactFiberTreeReflection.js
+36 -1
@@ -8,7 +8,7 @@
8 */
9
10 import type {Fiber} from './ReactInternalTypes';
11 -import type {Container, SuspenseInstance} from './ReactFiberConfig';
11 +import type {Container, SuspenseInstance, Instance} from './ReactFiberConfig';
12 import type {SuspenseState} from './ReactFiberSuspenseComponent';
13
14 import {
@@ -19,6 +19,7 @@ import {
19 HostPortal,
20 HostText,
21 SuspenseComponent,
22 + OffscreenComponent,
23 } from './ReactWorkTags';
24 import {NoFlags, Placement, Hydrating} from './ReactFiberFlags';
25
@@ -317,3 +318,37 @@ export function doesFiberContain(
318 }
319 return false;
320 }
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 +}