From c1533e57a5872ff857098ad18a48f82fbc010bc8 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 9 May 2024 19:05:16 -0400 Subject: [PATCH 1/2] Unify ReactFiberCurrentOwner and ReactCurrentFiber --- .../src/__tests__/treeContext-test.js | 12 +++---- packages/react-dom/src/__tests__/refs-test.js | 4 +-- .../react-dom/src/client/ReactDOMRootFB.js | 7 +++-- .../src/ReactNativePublicCompat.js | 7 +++-- .../react-reconciler/src/ReactCurrentFiber.js | 16 ++++++++-- .../src/ReactFiberAsyncDispatcher.js | 2 +- .../src/ReactFiberBeginWork.js | 7 ++--- .../src/ReactFiberCommitWork.js | 8 +++-- .../src/ReactFiberCurrentOwner.js | 16 ---------- .../src/ReactFiberReconciler.js | 4 +-- .../src/ReactFiberTreeReflection.js | 4 +-- .../src/ReactFiberWorkLoop.js | 31 ++++++------------- .../src/ReactStrictModeWarnings.js | 4 +-- 13 files changed, 56 insertions(+), 66 deletions(-) delete mode 100644 packages/react-reconciler/src/ReactFiberCurrentOwner.js diff --git a/packages/react-devtools-shared/src/__tests__/treeContext-test.js b/packages/react-devtools-shared/src/__tests__/treeContext-test.js index f22b08f4d9cc..185e11461bfe 100644 --- a/packages/react-devtools-shared/src/__tests__/treeContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/treeContext-test.js @@ -2586,14 +2586,14 @@ describe('TreeListContext', () => { utils.act(() => TestRenderer.create()); expect(store).toMatchInlineSnapshot(` - ✕ 2, ⚠ 0 + ✕ 1, ⚠ 0 [root] ✕ `); selectNextErrorOrWarning(); expect(state).toMatchInlineSnapshot(` - ✕ 2, ⚠ 0 + ✕ 1, ⚠ 0 [root] → ✕ `); @@ -2648,14 +2648,14 @@ describe('TreeListContext', () => { utils.act(() => TestRenderer.create()); expect(store).toMatchInlineSnapshot(` - ✕ 2, ⚠ 0 + ✕ 1, ⚠ 0 [root] ✕ `); selectNextErrorOrWarning(); expect(state).toMatchInlineSnapshot(` - ✕ 2, ⚠ 0 + ✕ 1, ⚠ 0 [root] → ✕ `); @@ -2705,7 +2705,7 @@ describe('TreeListContext', () => { utils.act(() => TestRenderer.create()); expect(store).toMatchInlineSnapshot(` - ✕ 3, ⚠ 0 + ✕ 2, ⚠ 0 [root] ▾ ✕ ✕ @@ -2713,7 +2713,7 @@ describe('TreeListContext', () => { selectNextErrorOrWarning(); expect(state).toMatchInlineSnapshot(` - ✕ 3, ⚠ 0 + ✕ 2, ⚠ 0 [root] → ▾ ✕ ✕ diff --git a/packages/react-dom/src/__tests__/refs-test.js b/packages/react-dom/src/__tests__/refs-test.js index 9dcaed18c237..a678f955e5fc 100644 --- a/packages/react-dom/src/__tests__/refs-test.js +++ b/packages/react-dom/src/__tests__/refs-test.js @@ -499,8 +499,8 @@ describe('creating element with string ref in constructor', () => { } } - // @gate !disableStringRefs - it('throws an error', async () => { + // @gate !disableStringRefs && !__DEV__ + it('throws an error in prod', async () => { await expect(async function () { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); diff --git a/packages/react-dom/src/client/ReactDOMRootFB.js b/packages/react-dom/src/client/ReactDOMRootFB.js index 0aa2306665f4..64a1cf12ac73 100644 --- a/packages/react-dom/src/client/ReactDOMRootFB.js +++ b/packages/react-dom/src/client/ReactDOMRootFB.js @@ -61,7 +61,10 @@ import {LegacyRoot} from 'react-reconciler/src/ReactRootTags'; import getComponentNameFromType from 'shared/getComponentNameFromType'; import {has as hasInstance} from 'shared/ReactInstanceMap'; -import {currentOwner} from 'react-reconciler/src/ReactFiberCurrentOwner'; +import { + current as currentOwner, + isRendering, +} from 'react-reconciler/src/ReactCurrentFiber'; import assign from 'shared/assign'; @@ -343,7 +346,7 @@ export function findDOMNode( ): null | Element | Text { if (__DEV__) { const owner = currentOwner; - if (owner !== null && owner.stateNode !== null) { + if (owner !== null && isRendering && owner.stateNode !== null) { const warnedAboutRefsInRender = owner.stateNode._warnedAboutRefsInRender; if (!warnedAboutRefsInRender) { console.error( diff --git a/packages/react-native-renderer/src/ReactNativePublicCompat.js b/packages/react-native-renderer/src/ReactNativePublicCompat.js index 081ea6f7eb57..7d6bb4974164 100644 --- a/packages/react-native-renderer/src/ReactNativePublicCompat.js +++ b/packages/react-native-renderer/src/ReactNativePublicCompat.js @@ -25,14 +25,17 @@ import { } from 'react-reconciler/src/ReactFiberReconciler'; import {doesFiberContain} from 'react-reconciler/src/ReactFiberTreeReflection'; import getComponentNameFromType from 'shared/getComponentNameFromType'; -import {currentOwner} from 'react-reconciler/src/ReactFiberCurrentOwner'; +import { + current as currentOwner, + isRendering, +} from 'react-reconciler/src/ReactCurrentFiber'; export function findHostInstance_DEPRECATED( componentOrHandle: ?(ElementRef | number), ): ?ElementRef> { if (__DEV__) { const owner = currentOwner; - if (owner !== null && owner.stateNode !== null) { + if (owner !== null && isRendering && owner.stateNode !== null) { if (!owner.stateNode._warnedAboutRefsInRender) { console.error( '%s is accessing findNodeHandle inside its render(). ' + diff --git a/packages/react-reconciler/src/ReactCurrentFiber.js b/packages/react-reconciler/src/ReactCurrentFiber.js index 5baa696b28b5..c0ff5d16df4c 100644 --- a/packages/react-reconciler/src/ReactCurrentFiber.js +++ b/packages/react-reconciler/src/ReactCurrentFiber.js @@ -41,21 +41,33 @@ function getCurrentFiberStackInDev(): string { return ''; } +export function resetCurrentDebugFiberInDEV() { + if (__DEV__) { + resetCurrentFiber(); + } +} + +export function setCurrentDebugFiberInDEV(fiber: Fiber | null) { + if (__DEV__) { + setCurrentFiber(fiber); + } +} + export function resetCurrentFiber() { if (__DEV__) { ReactSharedInternals.getCurrentStack = null; - current = null; isRendering = false; } + current = null; } export function setCurrentFiber(fiber: Fiber | null) { if (__DEV__) { ReactSharedInternals.getCurrentStack = fiber === null ? null : getCurrentFiberStackInDev; - current = fiber; isRendering = false; } + current = fiber; } export function getCurrentFiber(): Fiber | null { diff --git a/packages/react-reconciler/src/ReactFiberAsyncDispatcher.js b/packages/react-reconciler/src/ReactFiberAsyncDispatcher.js index f1c251965512..27d3866d5d6d 100644 --- a/packages/react-reconciler/src/ReactFiberAsyncDispatcher.js +++ b/packages/react-reconciler/src/ReactFiberAsyncDispatcher.js @@ -16,7 +16,7 @@ import {CacheContext} from './ReactFiberCacheComponent'; import {disableStringRefs} from 'shared/ReactFeatureFlags'; -import {currentOwner} from './ReactFiberCurrentOwner'; +import {current as currentOwner} from 'react-reconciler/src/ReactCurrentFiber'; function getCacheForType(resourceType: () => T): T { if (!enableCache) { diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index b1d639583df7..e638580bb0da 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -125,6 +125,7 @@ import { import { getCurrentFiberOwnerNameInDevOrNull, setIsRendering, + setCurrentFiber, } from './ReactCurrentFiber'; import { resolveFunctionForHotReloading, @@ -296,7 +297,6 @@ import { pushRootMarkerInstance, TransitionTracingMarker, } from './ReactFiberTracingMarkerComponent'; -import {setCurrentOwner} from './ReactFiberCurrentOwner'; // A special exception that's used to unwind the stack when an update flows // into a dehydrated boundary. @@ -432,7 +432,6 @@ function updateForwardRef( markComponentRenderStarted(workInProgress); } if (__DEV__) { - setCurrentOwner(workInProgress); setIsRendering(true); nextChildren = renderWithHooks( current, @@ -1150,7 +1149,6 @@ function updateFunctionComponent( markComponentRenderStarted(workInProgress); } if (__DEV__) { - setCurrentOwner(workInProgress); setIsRendering(true); nextChildren = renderWithHooks( current, @@ -1373,7 +1371,7 @@ function finishClassComponent( // Rerender if (__DEV__ || !disableStringRefs) { - setCurrentOwner(workInProgress); + setCurrentFiber(workInProgress); } let nextChildren; if ( @@ -3419,7 +3417,6 @@ function updateContextConsumer( } let newChildren; if (__DEV__) { - setCurrentOwner(workInProgress); setIsRendering(true); newChildren = render(newValue); setIsRendering(false); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index f757b7ca81b1..5c7f845b9679 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -101,8 +101,8 @@ import { } from './ReactFiberFlags'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import { - resetCurrentFiber as resetCurrentDebugFiberInDEV, - setCurrentFiber as setCurrentDebugFiberInDEV, + resetCurrentDebugFiberInDEV, + setCurrentDebugFiberInDEV, getCurrentFiber as getCurrentDebugFiberInDEV, } from './ReactCurrentFiber'; import {resolveClassComponentProps} from './ReactFiberClassComponent'; @@ -2486,7 +2486,7 @@ export function commitMutationEffects( setCurrentDebugFiberInDEV(finishedWork); commitMutationEffectsOnFiber(finishedWork, root, committedLanes); - setCurrentDebugFiberInDEV(finishedWork); + resetCurrentDebugFiberInDEV(); inProgressLanes = null; inProgressRoot = null; @@ -3125,8 +3125,10 @@ export function commitLayoutEffects( inProgressLanes = committedLanes; inProgressRoot = root; + setCurrentDebugFiberInDEV(finishedWork); const current = finishedWork.alternate; commitLayoutEffectOnFiber(root, current, finishedWork, committedLanes); + resetCurrentDebugFiberInDEV(); inProgressLanes = null; inProgressRoot = null; diff --git a/packages/react-reconciler/src/ReactFiberCurrentOwner.js b/packages/react-reconciler/src/ReactFiberCurrentOwner.js deleted file mode 100644 index 0c1f3d6e3130..000000000000 --- a/packages/react-reconciler/src/ReactFiberCurrentOwner.js +++ /dev/null @@ -1,16 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -import type {Fiber} from './ReactInternalTypes'; - -export let currentOwner: Fiber | null = null; - -export function setCurrentOwner(fiber: null | Fiber) { - currentOwner = fiber; -} diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 641f229b6dea..eb1bfbb65a6c 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -78,8 +78,8 @@ import { import { isRendering as ReactCurrentFiberIsRendering, current as ReactCurrentFiberCurrent, - resetCurrentFiber as resetCurrentDebugFiberInDEV, - setCurrentFiber as setCurrentDebugFiberInDEV, + resetCurrentDebugFiberInDEV, + setCurrentDebugFiberInDEV, } from './ReactCurrentFiber'; import {StrictLegacyMode} from './ReactTypeOfMode'; import { diff --git a/packages/react-reconciler/src/ReactFiberTreeReflection.js b/packages/react-reconciler/src/ReactFiberTreeReflection.js index 57640bd7f3b1..e0199108693e 100644 --- a/packages/react-reconciler/src/ReactFiberTreeReflection.js +++ b/packages/react-reconciler/src/ReactFiberTreeReflection.js @@ -24,7 +24,7 @@ import { SuspenseComponent, } from './ReactWorkTags'; import {NoFlags, Placement, Hydrating} from './ReactFiberFlags'; -import {currentOwner} from './ReactFiberCurrentOwner'; +import {current as currentOwner, isRendering} from './ReactCurrentFiber'; export function getNearestMountedFiber(fiber: Fiber): null | Fiber { let node = fiber; @@ -90,7 +90,7 @@ export function isFiberMounted(fiber: Fiber): boolean { export function isMounted(component: React$Component): boolean { if (__DEV__) { const owner = currentOwner; - if (owner !== null && owner.tag === ClassComponent) { + if (owner !== null && isRendering && owner.tag === ClassComponent) { const ownerFiber: Fiber = owner; const instance = ownerFiber.stateNode; if (!instance._warnedAboutRefsInRender) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 00391bb354ee..e46927c1bc49 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -204,7 +204,6 @@ import { ContextOnlyDispatcher, } from './ReactFiberHooks'; import {DefaultAsyncDispatcher} from './ReactFiberAsyncDispatcher'; -import {setCurrentOwner} from './ReactFiberCurrentOwner'; import { createCapturedValueAtFiber, type CapturedValue, @@ -230,8 +229,9 @@ import ReactStrictModeWarnings from './ReactStrictModeWarnings'; import { isRendering as ReactCurrentDebugFiberIsRenderingInDEV, current as ReactCurrentFiberCurrent, - resetCurrentFiber as resetCurrentDebugFiberInDEV, - setCurrentFiber as setCurrentDebugFiberInDEV, + resetCurrentDebugFiberInDEV, + setCurrentDebugFiberInDEV, + resetCurrentFiber, } from './ReactCurrentFiber'; import { isDevToolsPresent, @@ -1683,9 +1683,8 @@ function handleThrow(root: FiberRoot, thrownValue: any): void { // These should be reset immediately because they're only supposed to be set // when React is executing user code. resetHooksAfterThrow(); - resetCurrentDebugFiberInDEV(); if (__DEV__ || !disableStringRefs) { - setCurrentOwner(null); + resetCurrentFiber(); } if (thrownValue === SuspenseException) { @@ -2377,7 +2376,9 @@ function performUnitOfWork(unitOfWork: Fiber): void { next = beginWork(current, unitOfWork, entangledRenderLanes); } - resetCurrentDebugFiberInDEV(); + if (__DEV__ || !disableStringRefs) { + resetCurrentFiber(); + } unitOfWork.memoizedProps = unitOfWork.pendingProps; if (next === null) { // If this doesn't spawn new work, complete the current work. @@ -2385,10 +2386,6 @@ function performUnitOfWork(unitOfWork: Fiber): void { } else { workInProgress = next; } - - if (__DEV__ || !disableStringRefs) { - setCurrentOwner(null); - } } function replaySuspendedUnitOfWork(unitOfWork: Fiber): void { @@ -2399,7 +2396,6 @@ function replaySuspendedUnitOfWork(unitOfWork: Fiber): void { setCurrentDebugFiberInDEV(unitOfWork); let next; - setCurrentDebugFiberInDEV(unitOfWork); const isProfilingMode = enableProfilerTimer && (unitOfWork.mode & ProfileMode) !== NoMode; if (isProfilingMode) { @@ -2492,7 +2488,9 @@ function replaySuspendedUnitOfWork(unitOfWork: Fiber): void { // The begin phase finished successfully without suspending. Return to the // normal work loop. - resetCurrentDebugFiberInDEV(); + if (__DEV__ || !disableStringRefs) { + resetCurrentFiber(); + } unitOfWork.memoizedProps = unitOfWork.pendingProps; if (next === null) { // If this doesn't spawn new work, complete the current work. @@ -2500,10 +2498,6 @@ function replaySuspendedUnitOfWork(unitOfWork: Fiber): void { } else { workInProgress = next; } - - if (__DEV__ || !disableStringRefs) { - setCurrentOwner(null); - } } function throwAndUnwindWorkLoop( @@ -2893,11 +2887,6 @@ function commitRootImpl( const prevExecutionContext = executionContext; executionContext |= CommitContext; - // Reset this to null before calling lifecycles - if (__DEV__ || !disableStringRefs) { - setCurrentOwner(null); - } - // The commit phase is broken into several sub-phases. We do a separate pass // of the effect list for each phase: all mutation effects come before all // layout effects, and so on. diff --git a/packages/react-reconciler/src/ReactStrictModeWarnings.js b/packages/react-reconciler/src/ReactStrictModeWarnings.js index 2f223a0f0b83..d14a38a9020b 100644 --- a/packages/react-reconciler/src/ReactStrictModeWarnings.js +++ b/packages/react-reconciler/src/ReactStrictModeWarnings.js @@ -10,8 +10,8 @@ import type {Fiber} from './ReactInternalTypes'; import { - resetCurrentFiber as resetCurrentDebugFiberInDEV, - setCurrentFiber as setCurrentDebugFiberInDEV, + resetCurrentDebugFiberInDEV, + setCurrentDebugFiberInDEV, } from './ReactCurrentFiber'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import {StrictLegacyMode} from './ReactTypeOfMode'; From 599e50f75f602f98b712904c84fc98a4fdd88979 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 21 May 2024 12:27:27 -0400 Subject: [PATCH 2/2] Update packages/react-reconciler/src/ReactFiberAsyncDispatcher.js Co-authored-by: Sebastian Silbermann --- packages/react-reconciler/src/ReactFiberAsyncDispatcher.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberAsyncDispatcher.js b/packages/react-reconciler/src/ReactFiberAsyncDispatcher.js index 27d3866d5d6d..d329fb369cae 100644 --- a/packages/react-reconciler/src/ReactFiberAsyncDispatcher.js +++ b/packages/react-reconciler/src/ReactFiberAsyncDispatcher.js @@ -16,7 +16,7 @@ import {CacheContext} from './ReactFiberCacheComponent'; import {disableStringRefs} from 'shared/ReactFeatureFlags'; -import {current as currentOwner} from 'react-reconciler/src/ReactCurrentFiber'; +import {current as currentOwner} from './ReactCurrentFiber'; function getCacheForType(resourceType: () => T): T { if (!enableCache) {