@samitouri / QOS-React-1 / commits / 2e3e6a9b1c

Unify ReactFiberCurrentOwner and ReactCurrentFiber (#29038)

We previously had two slightly different concepts for "current fiber". There's the "owner" which is set inside of class components in prod if string refs are enabled, and sometimes inside function components in DEV but not other contexts. Then we have the "current fiber" which is only set in DEV for various warnings but is enabled in a bunch of contexts. This unifies them into a single "current fiber". The concept of string refs shouldn't really exist so this should really be a DEV only concept. In the meantime, this sets the current fiber inside class render only in prod, however, in DEV it's now enabled in more contexts which can affect the string refs. That was already the case that a string ref in a Function component was only connecting to the owner in prod. Any string ref associated with any non-class won't work regardless so that's not an issue. The practical change here is that an element with a string ref created inside a life-cycle associated with a class will work in DEV but not in prod. Since we need the current fiber to be available in more contexts in DEV for the debugging purposes. That wouldn't affect any old code since it would have a broken ref anyway. New code shouldn't use string refs anyway. The other implication is that "owner" doesn't necessarily mean "rendering" since we need the "owner" to track other debug information like stacks - in other contexts like useEffect, life cycles, etc. Internally we have a separate `isRendering` flag that actually means we're rendering but even that is a very overloaded concept. So anything that uses "owner" to imply rendering might be wrong with this change. This is a first step to a larger refactor for tracking current rendering information. --------- Co-authored-by: Sebastian Silbermann <silbermann.sebastian@gmail.com>

Sebastian Markbåge committed May 23, 2024 at 12:25 UTC 2e3e6a9b1cc97ec91248be74565e7ccbf6946067
13 files changed +56 -66
packages/react-devtools-shared/src/__tests__/treeContext-test.js
+6 -6
@@ -2586,14 +2586,14 @@ describe('TreeListContext', () => {
2586 utils.act(() => TestRenderer.create(<Contexts />));
2587
2588 expect(store).toMatchInlineSnapshot(`
2589 - ✕ 2, ⚠ 0
2589 + ✕ 1, ⚠ 0
2590 [root]
2591 <ErrorBoundary> ✕
2592 `);
2593
2594 selectNextErrorOrWarning();
2595 expect(state).toMatchInlineSnapshot(`
2596 - ✕ 2, ⚠ 0
2596 + ✕ 1, ⚠ 0
2597 [root]
2598 → <ErrorBoundary> ✕
2599 `);
@@ -2648,14 +2648,14 @@ describe('TreeListContext', () => {
2648 utils.act(() => TestRenderer.create(<Contexts />));
2649
2650 expect(store).toMatchInlineSnapshot(`
2651 - ✕ 2, ⚠ 0
2651 + ✕ 1, ⚠ 0
2652 [root]
2653 <ErrorBoundary> ✕
2654 `);
2655
2656 selectNextErrorOrWarning();
2657 expect(state).toMatchInlineSnapshot(`
2658 - ✕ 2, ⚠ 0
2658 + ✕ 1, ⚠ 0
2659 [root]
2660 → <ErrorBoundary> ✕
2661 `);
@@ -2705,7 +2705,7 @@ describe('TreeListContext', () => {
2705 utils.act(() => TestRenderer.create(<Contexts />));
2706
2707 expect(store).toMatchInlineSnapshot(`
2708 - ✕ 3, ⚠ 0
2708 + ✕ 2, ⚠ 0
2709 [root]
2710 ▾ <ErrorBoundary> ✕
2711 <Child> ✕
@@ -2713,7 +2713,7 @@ describe('TreeListContext', () => {
2713
2714 selectNextErrorOrWarning();
2715 expect(state).toMatchInlineSnapshot(`
2716 - ✕ 3, ⚠ 0
2716 + ✕ 2, ⚠ 0
2717 [root]
2718 → ▾ <ErrorBoundary> ✕
2719 <Child> ✕
packages/react-dom/src/__tests__/refs-test.js
+2 -2
@@ -499,8 +499,8 @@ describe('creating element with string ref in constructor', () => {
499 }
500 }
501
502 - // @gate !disableStringRefs
503 - it('throws an error', async () => {
502 + // @gate !disableStringRefs && !__DEV__
503 + it('throws an error in prod', async () => {
504 await expect(async function () {
505 const container = document.createElement('div');
506 const root = ReactDOMClient.createRoot(container);
packages/react-dom/src/client/ReactDOMRootFB.js
+5 -2
@@ -61,7 +61,10 @@ import {LegacyRoot} from 'react-reconciler/src/ReactRootTags';
61 import getComponentNameFromType from 'shared/getComponentNameFromType';
62 import {has as hasInstance} from 'shared/ReactInstanceMap';
63
64 -import {currentOwner} from 'react-reconciler/src/ReactFiberCurrentOwner';
64 +import {
65 + current as currentOwner,
66 + isRendering,
67 +} from 'react-reconciler/src/ReactCurrentFiber';
68
69 import assign from 'shared/assign';
70
@@ -343,7 +346,7 @@ export function findDOMNode(
346 ): null | Element | Text {
347 if (__DEV__) {
348 const owner = currentOwner;
346 - if (owner !== null && owner.stateNode !== null) {
349 + if (owner !== null && isRendering && owner.stateNode !== null) {
350 const warnedAboutRefsInRender = owner.stateNode._warnedAboutRefsInRender;
351 if (!warnedAboutRefsInRender) {
352 console.error(
packages/react-native-renderer/src/ReactNativePublicCompat.js
+5 -2
@@ -25,14 +25,17 @@ import {
25 } from 'react-reconciler/src/ReactFiberReconciler';
26 import {doesFiberContain} from 'react-reconciler/src/ReactFiberTreeReflection';
27 import getComponentNameFromType from 'shared/getComponentNameFromType';
28 -import {currentOwner} from 'react-reconciler/src/ReactFiberCurrentOwner';
28 +import {
29 + current as currentOwner,
30 + isRendering,
31 +} from 'react-reconciler/src/ReactCurrentFiber';
32
33 export function findHostInstance_DEPRECATED<TElementType: ElementType>(
34 componentOrHandle: ?(ElementRef<TElementType> | number),
35 ): ?ElementRef<HostComponent<mixed>> {
36 if (__DEV__) {
37 const owner = currentOwner;
35 - if (owner !== null && owner.stateNode !== null) {
38 + if (owner !== null && isRendering && owner.stateNode !== null) {
39 if (!owner.stateNode._warnedAboutRefsInRender) {
40 console.error(
41 '%s is accessing findNodeHandle inside its render(). ' +
packages/react-reconciler/src/ReactCurrentFiber.js
+14 -2
@@ -41,21 +41,33 @@ function getCurrentFiberStackInDev(): string {
41 return '';
42 }
43
44 +export function resetCurrentDebugFiberInDEV() {
45 + if (__DEV__) {
46 + resetCurrentFiber();
47 + }
48 +}
49 +
50 +export function setCurrentDebugFiberInDEV(fiber: Fiber | null) {
51 + if (__DEV__) {
52 + setCurrentFiber(fiber);
53 + }
54 +}
55 +
56 export function resetCurrentFiber() {
57 if (__DEV__) {
58 ReactSharedInternals.getCurrentStack = null;
47 - current = null;
59 isRendering = false;
60 }
61 + current = null;
62 }
63
64 export function setCurrentFiber(fiber: Fiber | null) {
65 if (__DEV__) {
66 ReactSharedInternals.getCurrentStack =
67 fiber === null ? null : getCurrentFiberStackInDev;
56 - current = fiber;
68 isRendering = false;
69 }
70 + current = fiber;
71 }
72
73 export function getCurrentFiber(): Fiber | null {
packages/react-reconciler/src/ReactFiberAsyncDispatcher.js
+1 -1
@@ -16,7 +16,7 @@ import {CacheContext} from './ReactFiberCacheComponent';
16
17 import {disableStringRefs} from 'shared/ReactFeatureFlags';
18
19 -import {currentOwner} from './ReactFiberCurrentOwner';
19 +import {current as currentOwner} from './ReactCurrentFiber';
20
21 function getCacheForType<T>(resourceType: () => T): T {
22 if (!enableCache) {
packages/react-reconciler/src/ReactFiberBeginWork.js
+2 -5
@@ -125,6 +125,7 @@ import {
125 import {
126 getCurrentFiberOwnerNameInDevOrNull,
127 setIsRendering,
128 + setCurrentFiber,
129 } from './ReactCurrentFiber';
130 import {
131 resolveFunctionForHotReloading,
@@ -296,7 +297,6 @@ import {
297 pushRootMarkerInstance,
298 TransitionTracingMarker,
299 } from './ReactFiberTracingMarkerComponent';
299 -import {setCurrentOwner} from './ReactFiberCurrentOwner';
300
301 // A special exception that's used to unwind the stack when an update flows
302 // into a dehydrated boundary.
@@ -432,7 +432,6 @@ function updateForwardRef(
432 markComponentRenderStarted(workInProgress);
433 }
434 if (__DEV__) {
435 - setCurrentOwner(workInProgress);
435 setIsRendering(true);
436 nextChildren = renderWithHooks(
437 current,
@@ -1150,7 +1149,6 @@ function updateFunctionComponent(
1149 markComponentRenderStarted(workInProgress);
1150 }
1151 if (__DEV__) {
1153 - setCurrentOwner(workInProgress);
1152 setIsRendering(true);
1153 nextChildren = renderWithHooks(
1154 current,
@@ -1373,7 +1371,7 @@ function finishClassComponent(
1371
1372 // Rerender
1373 if (__DEV__ || !disableStringRefs) {
1376 - setCurrentOwner(workInProgress);
1374 + setCurrentFiber(workInProgress);
1375 }
1376 let nextChildren;
1377 if (
@@ -3419,7 +3417,6 @@ function updateContextConsumer(
3417 }
3418 let newChildren;
3419 if (__DEV__) {
3422 - setCurrentOwner(workInProgress);
3420 setIsRendering(true);
3421 newChildren = render(newValue);
3422 setIsRendering(false);
packages/react-reconciler/src/ReactFiberCommitWork.js
+5 -3
@@ -101,8 +101,8 @@ import {
101 } from './ReactFiberFlags';
102 import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
103 import {
104 - resetCurrentFiber as resetCurrentDebugFiberInDEV,
105 - setCurrentFiber as setCurrentDebugFiberInDEV,
104 + resetCurrentDebugFiberInDEV,
105 + setCurrentDebugFiberInDEV,
106 getCurrentFiber as getCurrentDebugFiberInDEV,
107 } from './ReactCurrentFiber';
108 import {resolveClassComponentProps} from './ReactFiberClassComponent';
@@ -2486,7 +2486,7 @@ export function commitMutationEffects(
2486
2487 setCurrentDebugFiberInDEV(finishedWork);
2488 commitMutationEffectsOnFiber(finishedWork, root, committedLanes);
2489 - setCurrentDebugFiberInDEV(finishedWork);
2489 + resetCurrentDebugFiberInDEV();
2490
2491 inProgressLanes = null;
2492 inProgressRoot = null;
@@ -3125,8 +3125,10 @@ export function commitLayoutEffects(
3125 inProgressLanes = committedLanes;
3126 inProgressRoot = root;
3127
3128 + setCurrentDebugFiberInDEV(finishedWork);
3129 const current = finishedWork.alternate;
3130 commitLayoutEffectOnFiber(root, current, finishedWork, committedLanes);
3131 + resetCurrentDebugFiberInDEV();
3132
3133 inProgressLanes = null;
3134 inProgressRoot = null;
packages/react-reconciler/src/ReactFiberCurrentOwner.js deleted
-16
@@ -1,16 +0,0 @@
1 -/**
2 - * Copyright (c) Meta Platforms, Inc. and affiliates.
3 - *
4 - * This source code is licensed under the MIT license found in the
5 - * LICENSE file in the root directory of this source tree.
6 - *
7 - * @flow
8 - */
9 -
10 -import type {Fiber} from './ReactInternalTypes';
11 -
12 -export let currentOwner: Fiber | null = null;
13 -
14 -export function setCurrentOwner(fiber: null | Fiber) {
15 - currentOwner = fiber;
16 -}
packages/react-reconciler/src/ReactFiberReconciler.js
+2 -2
@@ -78,8 +78,8 @@ import {
78 import {
79 isRendering as ReactCurrentFiberIsRendering,
80 current as ReactCurrentFiberCurrent,
81 - resetCurrentFiber as resetCurrentDebugFiberInDEV,
82 - setCurrentFiber as setCurrentDebugFiberInDEV,
81 + resetCurrentDebugFiberInDEV,
82 + setCurrentDebugFiberInDEV,
83 } from './ReactCurrentFiber';
84 import {StrictLegacyMode} from './ReactTypeOfMode';
85 import {
packages/react-reconciler/src/ReactFiberTreeReflection.js
+2 -2
@@ -24,7 +24,7 @@ import {
24 SuspenseComponent,
25 } from './ReactWorkTags';
26 import {NoFlags, Placement, Hydrating} from './ReactFiberFlags';
27 -import {currentOwner} from './ReactFiberCurrentOwner';
27 +import {current as currentOwner, isRendering} from './ReactCurrentFiber';
28
29 export function getNearestMountedFiber(fiber: Fiber): null | Fiber {
30 let node = fiber;
@@ -90,7 +90,7 @@ export function isFiberMounted(fiber: Fiber): boolean {
90 export function isMounted(component: React$Component<any, any>): boolean {
91 if (__DEV__) {
92 const owner = currentOwner;
93 - if (owner !== null && owner.tag === ClassComponent) {
93 + if (owner !== null && isRendering && owner.tag === ClassComponent) {
94 const ownerFiber: Fiber = owner;
95 const instance = ownerFiber.stateNode;
96 if (!instance._warnedAboutRefsInRender) {
packages/react-reconciler/src/ReactFiberWorkLoop.js
+10 -21
@@ -206,7 +206,6 @@ import {
206 ContextOnlyDispatcher,
207 } from './ReactFiberHooks';
208 import {DefaultAsyncDispatcher} from './ReactFiberAsyncDispatcher';
209 -import {setCurrentOwner} from './ReactFiberCurrentOwner';
209 import {
210 createCapturedValueAtFiber,
211 type CapturedValue,
@@ -232,8 +231,9 @@ import ReactStrictModeWarnings from './ReactStrictModeWarnings';
231 import {
232 isRendering as ReactCurrentDebugFiberIsRenderingInDEV,
233 current as ReactCurrentFiberCurrent,
235 - resetCurrentFiber as resetCurrentDebugFiberInDEV,
236 - setCurrentFiber as setCurrentDebugFiberInDEV,
234 + resetCurrentDebugFiberInDEV,
235 + setCurrentDebugFiberInDEV,
236 + resetCurrentFiber,
237 } from './ReactCurrentFiber';
238 import {
239 isDevToolsPresent,
@@ -1686,9 +1686,8 @@ function handleThrow(root: FiberRoot, thrownValue: any): void {
1686 // These should be reset immediately because they're only supposed to be set
1687 // when React is executing user code.
1688 resetHooksAfterThrow();
1689 - resetCurrentDebugFiberInDEV();
1689 if (__DEV__ || !disableStringRefs) {
1691 - setCurrentOwner(null);
1690 + resetCurrentFiber();
1691 }
1692
1693 if (thrownValue === SuspenseException) {
@@ -2386,7 +2385,9 @@ function performUnitOfWork(unitOfWork: Fiber): void {
2385 next = beginWork(current, unitOfWork, entangledRenderLanes);
2386 }
2387
2389 - resetCurrentDebugFiberInDEV();
2388 + if (__DEV__ || !disableStringRefs) {
2389 + resetCurrentFiber();
2390 + }
2391 unitOfWork.memoizedProps = unitOfWork.pendingProps;
2392 if (next === null) {
2393 // If this doesn't spawn new work, complete the current work.
@@ -2394,10 +2395,6 @@ function performUnitOfWork(unitOfWork: Fiber): void {
2395 } else {
2396 workInProgress = next;
2397 }
2397 -
2398 - if (__DEV__ || !disableStringRefs) {
2399 - setCurrentOwner(null);
2400 - }
2398 }
2399
2400 function replaySuspendedUnitOfWork(unitOfWork: Fiber): void {
@@ -2408,7 +2405,6 @@ function replaySuspendedUnitOfWork(unitOfWork: Fiber): void {
2405 setCurrentDebugFiberInDEV(unitOfWork);
2406
2407 let next;
2411 - setCurrentDebugFiberInDEV(unitOfWork);
2408 const isProfilingMode =
2409 enableProfilerTimer && (unitOfWork.mode & ProfileMode) !== NoMode;
2410 if (isProfilingMode) {
@@ -2501,7 +2497,9 @@ function replaySuspendedUnitOfWork(unitOfWork: Fiber): void {
2497 // The begin phase finished successfully without suspending. Return to the
2498 // normal work loop.
2499
2504 - resetCurrentDebugFiberInDEV();
2500 + if (__DEV__ || !disableStringRefs) {
2501 + resetCurrentFiber();
2502 + }
2503 unitOfWork.memoizedProps = unitOfWork.pendingProps;
2504 if (next === null) {
2505 // If this doesn't spawn new work, complete the current work.
@@ -2509,10 +2507,6 @@ function replaySuspendedUnitOfWork(unitOfWork: Fiber): void {
2507 } else {
2508 workInProgress = next;
2509 }
2512 -
2513 - if (__DEV__ || !disableStringRefs) {
2514 - setCurrentOwner(null);
2515 - }
2510 }
2511
2512 function throwAndUnwindWorkLoop(
@@ -2902,11 +2896,6 @@ function commitRootImpl(
2896 const prevExecutionContext = executionContext;
2897 executionContext |= CommitContext;
2898
2905 - // Reset this to null before calling lifecycles
2906 - if (__DEV__ || !disableStringRefs) {
2907 - setCurrentOwner(null);
2908 - }
2909 -
2899 // The commit phase is broken into several sub-phases. We do a separate pass
2900 // of the effect list for each phase: all mutation effects come before all
2901 // layout effects, and so on.
packages/react-reconciler/src/ReactStrictModeWarnings.js
+2 -2
@@ -10,8 +10,8 @@
10 import type {Fiber} from './ReactInternalTypes';
11
12 import {
13 - resetCurrentFiber as resetCurrentDebugFiberInDEV,
14 - setCurrentFiber as setCurrentDebugFiberInDEV,
13 + resetCurrentDebugFiberInDEV,
14 + setCurrentDebugFiberInDEV,
15 } from './ReactCurrentFiber';
16 import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
17 import {StrictLegacyMode} from './ReactTypeOfMode';