Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fiber] Delete isMounted internals #31966

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 0 additions & 180 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
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
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
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
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
Loading