Skip to content

Commit

Permalink
[Fiber] Delete isMounted internals (#31966)
Browse files Browse the repository at this point in the history 
The public API has been deleted a long time ago so this should be unused
unless it's used by hacks. It should be replaced with an
effect/lifecycle that manually tracks this if you need it.

The problem with this API is how the timing implemented because it
requires Placement/Hydration flags to be cleared too early. In fact,
that's why we also have a separate PlacementDEV flag that works
differently.


https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberCommitWork.js#L2157-L2165

We should be able to remove this code now.
  • Loading branch information
@sebmarkbage
sebmarkbage authored Jan 8, 2025
1 parent 379089d commit e30c669
Show file tree
Hide file tree
Showing 6 changed files with 0 additions and 316 deletions.
180 changes: 0 additions & 180 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
let act;

let React;
let ReactDOM;
let ReactDOMClient;
let assertConsoleErrorDev;
let assertConsoleWarnDev;
Expand Down Expand Up @@ -63,24 +62,6 @@ const POST_WILL_UNMOUNT_STATE = {
hasWillUnmountCompleted: true,
};

/**
* Every React component is in one of these life cycles.
*/
type ComponentLifeCycle =
/**
* Mounted components have a DOM node representation and are capable of
* receiving new props.
*/
| 'MOUNTED'
/**
* Unmounted components are inactive and cannot receive new props.
*/
| 'UNMOUNTED';

function getLifeCycleState(instance): ComponentLifeCycle {
return instance.updater.isMounted(instance) ? 'MOUNTED' : 'UNMOUNTED';
}

/**
* TODO: We should make any setState calls fail in
* `getInitialState` and `componentWillMount`. They will usually fail
Expand All @@ -99,7 +80,6 @@ describe('ReactComponentLifeCycle', () => {
} = require('internal-test-utils'));

React = require('react');
ReactDOM = require('react-dom');
ReactDOMClient = require('react-dom/client');
});

Expand Down Expand Up @@ -290,137 +270,6 @@ describe('ReactComponentLifeCycle', () => {
});
});

it('should correctly determine if a component is mounted', async () => {
class Component extends React.Component {
_isMounted() {
// No longer a public API, but we can test that it works internally by
// reaching into the updater.
return this.updater.isMounted(this);
}
UNSAFE_componentWillMount() {
expect(this._isMounted()).toBeFalsy();
}
componentDidMount() {
expect(this._isMounted()).toBeTruthy();
}
render() {
expect(this._isMounted()).toBeFalsy();
return <div />;
}
}

let instance;
const element = <Component ref={current => (instance = current)} />;

const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(element);
});
assertConsoleErrorDev([
'Component is accessing isMounted inside its render() function. ' +
'render() should be a pure function of props and state. ' +
'It should never access something that requires stale data ' +
'from the previous render, such as refs. ' +
'Move this logic to componentDidMount and componentDidUpdate instead.\n' +
' in Component (at **)',
]);
expect(instance._isMounted()).toBeTruthy();
});

it('should correctly determine if a null component is mounted', async () => {
class Component extends React.Component {
_isMounted() {
// No longer a public API, but we can test that it works internally by
// reaching into the updater.
return this.updater.isMounted(this);
}
UNSAFE_componentWillMount() {
expect(this._isMounted()).toBeFalsy();
}
componentDidMount() {
expect(this._isMounted()).toBeTruthy();
}
render() {
expect(this._isMounted()).toBeFalsy();
return null;
}
}

let instance;
const element = <Component ref={current => (instance = current)} />;

const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(element);
});
assertConsoleErrorDev([
'Component is accessing isMounted inside its render() function. ' +
'render() should be a pure function of props and state. ' +
'It should never access something that requires stale data ' +
'from the previous render, such as refs. ' +
'Move this logic to componentDidMount and componentDidUpdate instead.\n' +
' in Component (at **)',
]);
expect(instance._isMounted()).toBeTruthy();
});

it('isMounted should return false when unmounted', async () => {
class Component extends React.Component {
render() {
return <div />;
}
}

const root = ReactDOMClient.createRoot(document.createElement('div'));
const instanceRef = React.createRef();
await act(() => {
root.render(<Component ref={instanceRef} />);
});
const instance = instanceRef.current;

// No longer a public API, but we can test that it works internally by
// reaching into the updater.
expect(instance.updater.isMounted(instance)).toBe(true);

await act(() => {
root.unmount();
});

expect(instance.updater.isMounted(instance)).toBe(false);
});

// @gate www && classic
it('warns if legacy findDOMNode is used inside render', async () => {
class Component extends React.Component {
state = {isMounted: false};
componentDidMount() {
this.setState({isMounted: true});
}
render() {
if (this.state.isMounted) {
expect(ReactDOM.findDOMNode(this).tagName).toBe('DIV');
}
return <div />;
}
}

const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<Component />);
});
assertConsoleErrorDev([
'Component is accessing findDOMNode inside its render(). ' +
'render() should be a pure function of props and state. ' +
'It should never access something that requires stale data ' +
'from the previous render, such as refs. ' +
'Move this logic to componentDidMount and componentDidUpdate instead.\n' +
' in Component (at **)',
]);
});

it('should carry through each of the phases of setup', async () => {
class LifeCycleComponent extends React.Component {
constructor(props, context) {
Expand All @@ -433,31 +282,25 @@ describe('ReactComponentLifeCycle', () => {
hasWillUnmountCompleted: false,
};
this._testJournal.returnedFromGetInitialState = clone(initState);
this._testJournal.lifeCycleAtStartOfGetInitialState =
getLifeCycleState(this);
this.state = initState;
}

UNSAFE_componentWillMount() {
this._testJournal.stateAtStartOfWillMount = clone(this.state);
this._testJournal.lifeCycleAtStartOfWillMount = getLifeCycleState(this);
this.state.hasWillMountCompleted = true;
}

componentDidMount() {
this._testJournal.stateAtStartOfDidMount = clone(this.state);
this._testJournal.lifeCycleAtStartOfDidMount = getLifeCycleState(this);
this.setState({hasDidMountCompleted: true});
}

render() {
const isInitialRender = !this.state.hasRenderCompleted;
if (isInitialRender) {
this._testJournal.stateInInitialRender = clone(this.state);
this._testJournal.lifeCycleInInitialRender = getLifeCycleState(this);
} else {
this._testJournal.stateInLaterRender = clone(this.state);
this._testJournal.lifeCycleInLaterRender = getLifeCycleState(this);
}
// you would *NEVER* do anything like this in real code!
this.state.hasRenderCompleted = true;
Expand All @@ -466,8 +309,6 @@ describe('ReactComponentLifeCycle', () => {

componentWillUnmount() {
this._testJournal.stateAtStartOfWillUnmount = clone(this.state);
this._testJournal.lifeCycleAtStartOfWillUnmount =
getLifeCycleState(this);
this.state.hasWillUnmountCompleted = true;
}
}
Expand All @@ -480,52 +321,33 @@ describe('ReactComponentLifeCycle', () => {
await act(() => {
root.render(<LifeCycleComponent ref={instanceRef} />);
});
assertConsoleErrorDev([
'LifeCycleComponent is accessing isMounted inside its render() function. ' +
'render() should be a pure function of props and state. ' +
'It should never access something that requires stale data ' +
'from the previous render, such as refs. ' +
'Move this logic to componentDidMount and componentDidUpdate instead.\n' +
' in LifeCycleComponent (at **)',
]);
const instance = instanceRef.current;

// getInitialState
expect(instance._testJournal.returnedFromGetInitialState).toEqual(
GET_INIT_STATE_RETURN_VAL,
);
expect(instance._testJournal.lifeCycleAtStartOfGetInitialState).toBe(
'UNMOUNTED',
);

// componentWillMount
expect(instance._testJournal.stateAtStartOfWillMount).toEqual(
instance._testJournal.returnedFromGetInitialState,
);
expect(instance._testJournal.lifeCycleAtStartOfWillMount).toBe('UNMOUNTED');

// componentDidMount
expect(instance._testJournal.stateAtStartOfDidMount).toEqual(
DID_MOUNT_STATE,
);
expect(instance._testJournal.lifeCycleAtStartOfDidMount).toBe('MOUNTED');

// initial render
expect(instance._testJournal.stateInInitialRender).toEqual(
INIT_RENDER_STATE,
);
expect(instance._testJournal.lifeCycleInInitialRender).toBe('UNMOUNTED');

expect(getLifeCycleState(instance)).toBe('MOUNTED');

// Now *update the component*
instance.forceUpdate();

// render 2nd time
expect(instance._testJournal.stateInLaterRender).toEqual(NEXT_RENDER_STATE);
expect(instance._testJournal.lifeCycleInLaterRender).toBe('MOUNTED');

expect(getLifeCycleState(instance)).toBe('MOUNTED');

await act(() => {
root.unmount();
Expand All @@ -535,10 +357,8 @@ describe('ReactComponentLifeCycle', () => {
WILL_UNMOUNT_STATE,
);
// componentWillUnmount called right before unmount.
expect(instance._testJournal.lifeCycleAtStartOfWillUnmount).toBe('MOUNTED');

// But the current lifecycle of the component is unmounted.
expect(getLifeCycleState(instance)).toBe('UNMOUNTED');
expect(instance.state).toEqual(POST_WILL_UNMOUNT_STATE);
});

Expand Down
2 changes: 0 additions & 2 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
disableDefaultPropsExceptForClasses,
} from 'shared/ReactFeatureFlags';
import ReactStrictModeWarnings from './ReactStrictModeWarnings';
import {isMounted} from './ReactFiberTreeReflection';
import {get as getInstance, set as setInstance} from 'shared/ReactInstanceMap';
import shallowEqual from 'shared/shallowEqual';
import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
Expand Down Expand Up @@ -165,7 +164,6 @@ function applyDerivedStateFromProps(
}

const classComponentUpdater = {
isMounted,
// $FlowFixMe[missing-local-annot]
enqueueSetState(inst: any, payload: any, callback) {
const fiber = getInstance(inst);
Expand Down
8 changes: 0 additions & 8 deletions packages/react-reconciler/src/ReactFiberContext.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import type {Fiber} from './ReactInternalTypes';
import type {StackCursor} from './ReactFiberStack';

import {isFiberMounted} from './ReactFiberTreeReflection';
import {disableLegacyContext} from 'shared/ReactFeatureFlags';
import {ClassComponent, HostRoot} from './ReactWorkTags';
import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
Expand Down Expand Up @@ -285,13 +284,6 @@ function findCurrentUnmaskedContext(fiber: Fiber): Object {
} else {
// Currently this is only used with renderSubtreeIntoContainer; not sure if it
// makes sense elsewhere
if (!isFiberMounted(fiber) || fiber.tag !== ClassComponent) {
throw new Error(
'Expected subtree parent to be a mounted class component. ' +
'This error is likely caused by a bug in React. Please file an issue.',
);
}

let node: Fiber = fiber;
do {
switch (node.tag) {
Expand Down
35 changes: 0 additions & 35 deletions packages/react-reconciler/src/ReactFiberTreeReflection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@ import type {Fiber} from './ReactInternalTypes';
import type {Container, SuspenseInstance} from './ReactFiberConfig';
import type {SuspenseState} from './ReactFiberSuspenseComponent';

import {get as getInstance} from 'shared/ReactInstanceMap';
import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
import {
ClassComponent,
HostComponent,
HostHoistable,
HostSingleton,
Expand All @@ -24,7 +21,6 @@ import {
SuspenseComponent,
} from './ReactWorkTags';
import {NoFlags, Placement, Hydrating} from './ReactFiberFlags';
import {current as currentOwner, isRendering} from './ReactCurrentFiber';

export function getNearestMountedFiber(fiber: Fiber): null | Fiber {
let node = fiber;
Expand Down Expand Up @@ -83,37 +79,6 @@ export function getContainerFromFiber(fiber: Fiber): null | Container {
: null;
}

export function isFiberMounted(fiber: Fiber): boolean {
return getNearestMountedFiber(fiber) === fiber;
}

export function isMounted(component: React$Component<any, any>): boolean {
if (__DEV__) {
const owner = currentOwner;
if (owner !== null && isRendering && owner.tag === ClassComponent) {
const ownerFiber: Fiber = owner;
const instance = ownerFiber.stateNode;
if (!instance._warnedAboutRefsInRender) {
console.error(
'%s is accessing isMounted inside its render() function. ' +
'render() should be a pure function of props and state. It should ' +
'never access something that requires stale data from the previous ' +
'render, such as refs. Move this logic to componentDidMount and ' +
'componentDidUpdate instead.',
getComponentNameFromFiber(ownerFiber) || 'A component',
);
}
instance._warnedAboutRefsInRender = true;
}
}

const fiber: ?Fiber = getInstance(component);
if (!fiber) {
return false;
}
return getNearestMountedFiber(fiber) === fiber;
}

function assertIsMounted(fiber: Fiber) {
if (getNearestMountedFiber(fiber) !== fiber) {
throw new Error('Unable to find node on an unmounted component.');
Expand Down
Loading

0 comments on commit e30c669

Please sign in to comment.