Skip to content

Commit

Permalink
Unify ReactFiberCurrentOwner and ReactCurrentFiber (#29038)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
sebmarkbage and eps1lon authored May 23, 2024
1 parent 4c2e457 commit 2e3e6a9
Show file tree
Hide file tree
Showing 13 changed files with 56 additions and 66 deletions.
12 changes: 6 additions & 6 deletions packages/react-devtools-shared/src/__tests__/treeContext-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2586,14 +2586,14 @@ describe('TreeListContext', () => {
utils.act(() => TestRenderer.create(<Contexts />));

expect(store).toMatchInlineSnapshot(`
2, ⚠ 0
1, ⚠ 0
[root]
<ErrorBoundary> ✕
`);

selectNextErrorOrWarning();
expect(state).toMatchInlineSnapshot(`
2, ⚠ 0
1, ⚠ 0
[root]
→ <ErrorBoundary> ✕
`);
Expand Down Expand Up @@ -2648,14 +2648,14 @@ describe('TreeListContext', () => {
utils.act(() => TestRenderer.create(<Contexts />));

expect(store).toMatchInlineSnapshot(`
2, ⚠ 0
1, ⚠ 0
[root]
<ErrorBoundary> ✕
`);

selectNextErrorOrWarning();
expect(state).toMatchInlineSnapshot(`
2, ⚠ 0
1, ⚠ 0
[root]
→ <ErrorBoundary> ✕
`);
Expand Down Expand Up @@ -2705,15 +2705,15 @@ describe('TreeListContext', () => {
utils.act(() => TestRenderer.create(<Contexts />));

expect(store).toMatchInlineSnapshot(`
3, ⚠ 0
2, ⚠ 0
[root]
▾ <ErrorBoundary> ✕
<Child> ✕
`);

selectNextErrorOrWarning();
expect(state).toMatchInlineSnapshot(`
3, ⚠ 0
2, ⚠ 0
[root]
→ ▾ <ErrorBoundary> ✕
<Child> ✕
Expand Down
4 changes: 2 additions & 2 deletions packages/react-dom/src/__tests__/refs-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 5 additions & 2 deletions packages/react-dom/src/client/ReactDOMRootFB.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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(
Expand Down
7 changes: 5 additions & 2 deletions packages/react-native-renderer/src/ReactNativePublicCompat.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<TElementType: ElementType>(
componentOrHandle: ?(ElementRef<TElementType> | number),
): ?ElementRef<HostComponent<mixed>> {
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(). ' +
Expand Down
16 changes: 14 additions & 2 deletions packages/react-reconciler/src/ReactCurrentFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberAsyncDispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {CacheContext} from './ReactFiberCacheComponent';

import {disableStringRefs} from 'shared/ReactFeatureFlags';

import {currentOwner} from './ReactFiberCurrentOwner';
import {current as currentOwner} from './ReactCurrentFiber';

function getCacheForType<T>(resourceType: () => T): T {
if (!enableCache) {
Expand Down
7 changes: 2 additions & 5 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ import {
import {
getCurrentFiberOwnerNameInDevOrNull,
setIsRendering,
setCurrentFiber,
} from './ReactCurrentFiber';
import {
resolveFunctionForHotReloading,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -432,7 +432,6 @@ function updateForwardRef(
markComponentRenderStarted(workInProgress);
}
if (__DEV__) {
setCurrentOwner(workInProgress);
setIsRendering(true);
nextChildren = renderWithHooks(
current,
Expand Down Expand Up @@ -1150,7 +1149,6 @@ function updateFunctionComponent(
markComponentRenderStarted(workInProgress);
}
if (__DEV__) {
setCurrentOwner(workInProgress);
setIsRendering(true);
nextChildren = renderWithHooks(
current,
Expand Down Expand Up @@ -1373,7 +1371,7 @@ function finishClassComponent(

// Rerender
if (__DEV__ || !disableStringRefs) {
setCurrentOwner(workInProgress);
setCurrentFiber(workInProgress);
}
let nextChildren;
if (
Expand Down Expand Up @@ -3419,7 +3417,6 @@ function updateContextConsumer(
}
let newChildren;
if (__DEV__) {
setCurrentOwner(workInProgress);
setIsRendering(true);
newChildren = render(newValue);
setIsRendering(false);
Expand Down
8 changes: 5 additions & 3 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -2486,7 +2486,7 @@ export function commitMutationEffects(

setCurrentDebugFiberInDEV(finishedWork);
commitMutationEffectsOnFiber(finishedWork, root, committedLanes);
setCurrentDebugFiberInDEV(finishedWork);
resetCurrentDebugFiberInDEV();

inProgressLanes = null;
inProgressRoot = null;
Expand Down Expand Up @@ -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;
Expand Down
16 changes: 0 additions & 16 deletions packages/react-reconciler/src/ReactFiberCurrentOwner.js

This file was deleted.

4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberTreeReflection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -90,7 +90,7 @@ export function isFiberMounted(fiber: Fiber): boolean {
export function isMounted(component: React$Component<any, any>): 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) {
Expand Down
31 changes: 10 additions & 21 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ import {
ContextOnlyDispatcher,
} from './ReactFiberHooks';
import {DefaultAsyncDispatcher} from './ReactFiberAsyncDispatcher';
import {setCurrentOwner} from './ReactFiberCurrentOwner';
import {
createCapturedValueAtFiber,
type CapturedValue,
Expand All @@ -232,8 +231,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,
Expand Down Expand Up @@ -1686,9 +1686,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) {
Expand Down Expand Up @@ -2386,18 +2385,16 @@ 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.
completeUnitOfWork(unitOfWork);
} else {
workInProgress = next;
}

if (__DEV__ || !disableStringRefs) {
setCurrentOwner(null);
}
}

function replaySuspendedUnitOfWork(unitOfWork: Fiber): void {
Expand All @@ -2408,7 +2405,6 @@ function replaySuspendedUnitOfWork(unitOfWork: Fiber): void {
setCurrentDebugFiberInDEV(unitOfWork);

let next;
setCurrentDebugFiberInDEV(unitOfWork);
const isProfilingMode =
enableProfilerTimer && (unitOfWork.mode & ProfileMode) !== NoMode;
if (isProfilingMode) {
Expand Down Expand Up @@ -2501,18 +2497,16 @@ 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.
completeUnitOfWork(unitOfWork);
} else {
workInProgress = next;
}

if (__DEV__ || !disableStringRefs) {
setCurrentOwner(null);
}
}

function throwAndUnwindWorkLoop(
Expand Down Expand Up @@ -2902,11 +2896,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.
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactStrictModeWarnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down

0 comments on commit 2e3e6a9

Please sign in to comment.