Skip to content

Commit e26682a

Browse files
author
Brian Vaughn
authored
Removed Root API callback params and added warnings (#17916)
1 parent cf00812 commit e26682a

File tree

3 files changed

+61
-37
lines changed

3 files changed

+61
-37
lines changed

packages/react-dom/src/__tests__/ReactDOMRoot-test.js

+29
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,35 @@ describe('ReactDOMRoot', () => {
4040
expect(container.textContent).toEqual('Hi');
4141
});
4242

43+
it('warns if a callback parameter is provided to render', () => {
44+
const callback = jest.fn();
45+
const root = ReactDOM.createRoot(container);
46+
expect(() =>
47+
root.render(<div>Hi</div>, callback),
48+
).toErrorDev(
49+
'render(...): does not support the second callback argument. ' +
50+
'To execute a side effect after rendering, declare it in a component body with useEffect().',
51+
{withoutStack: true},
52+
);
53+
Scheduler.unstable_flushAll();
54+
expect(callback).not.toHaveBeenCalled();
55+
});
56+
57+
it('warns if a callback parameter is provided to unmount', () => {
58+
const callback = jest.fn();
59+
const root = ReactDOM.createRoot(container);
60+
root.render(<div>Hi</div>);
61+
expect(() =>
62+
root.unmount(callback),
63+
).toErrorDev(
64+
'unmount(...): does not support a callback argument. ' +
65+
'To execute a side effect after rendering, declare it in a component body with useEffect().',
66+
{withoutStack: true},
67+
);
68+
Scheduler.unstable_flushAll();
69+
expect(callback).not.toHaveBeenCalled();
70+
});
71+
4372
it('unmounts children', () => {
4473
const root = ReactDOM.createRoot(container);
4574
root.render(<div>Hi</div>);

packages/react-dom/src/client/ReactDOMLegacy.js

+14-5
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,7 @@ import {
1616
isContainerMarkedAsRoot,
1717
unmarkContainerAsRoot,
1818
} from './ReactDOMComponentTree';
19-
import {
20-
createLegacyRoot,
21-
isValidContainer,
22-
warnOnInvalidCallback,
23-
} from './ReactDOMRoot';
19+
import {createLegacyRoot, isValidContainer} from './ReactDOMRoot';
2420
import {ROOT_ATTRIBUTE_NAME} from '../shared/DOMProperty';
2521
import {
2622
DOCUMENT_NODE,
@@ -163,6 +159,19 @@ function legacyCreateRootFromDOMContainer(
163159
);
164160
}
165161

162+
function warnOnInvalidCallback(callback: mixed, callerName: string): void {
163+
if (__DEV__) {
164+
if (callback !== null && typeof callback !== 'function') {
165+
console.error(
166+
'%s(...): Expected the last optional `callback` argument to be a ' +
167+
'function. Instead received: %s.',
168+
callerName,
169+
callback,
170+
);
171+
}
172+
}
173+
}
174+
166175
function legacyRenderSubtreeIntoContainer(
167176
parentComponent: ?React$Component<any, any>,
168177
children: ReactNodeList,

packages/react-dom/src/client/ReactDOMRoot.js

+18-32
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ import type {ReactNodeList} from 'shared/ReactTypes';
1515
import type {FiberRoot} from 'react-reconciler/src/ReactFiberRoot';
1616

1717
export type RootType = {
18-
render(children: ReactNodeList, callback: ?() => mixed): void,
19-
unmount(callback: ?() => mixed): void,
18+
render(children: ReactNodeList): void,
19+
unmount(): void,
2020
_internalRoot: FiberRoot,
2121
...
2222
};
@@ -62,30 +62,32 @@ function ReactDOMBlockingRoot(
6262

6363
ReactDOMRoot.prototype.render = ReactDOMBlockingRoot.prototype.render = function(
6464
children: ReactNodeList,
65-
callback: ?() => mixed,
6665
): void {
67-
const root = this._internalRoot;
68-
const cb = callback === undefined ? null : callback;
6966
if (__DEV__) {
70-
warnOnInvalidCallback(cb, 'render');
67+
if (typeof arguments[1] === 'function') {
68+
console.error(
69+
'render(...): does not support the second callback argument. ' +
70+
'To execute a side effect after rendering, declare it in a component body with useEffect().',
71+
);
72+
}
7173
}
72-
updateContainer(children, root, null, cb);
74+
const root = this._internalRoot;
75+
updateContainer(children, root, null, null);
7376
};
7477

75-
ReactDOMRoot.prototype.unmount = ReactDOMBlockingRoot.prototype.unmount = function(
76-
callback: ?() => mixed,
77-
): void {
78-
const root = this._internalRoot;
79-
const cb = callback === undefined ? null : callback;
78+
ReactDOMRoot.prototype.unmount = ReactDOMBlockingRoot.prototype.unmount = function(): void {
8079
if (__DEV__) {
81-
warnOnInvalidCallback(cb, 'render');
80+
if (typeof arguments[0] === 'function') {
81+
console.error(
82+
'unmount(...): does not support a callback argument. ' +
83+
'To execute a side effect after rendering, declare it in a component body with useEffect().',
84+
);
85+
}
8286
}
87+
const root = this._internalRoot;
8388
const container = root.containerInfo;
8489
updateContainer(null, root, null, () => {
8590
unmarkContainerAsRoot(container);
86-
if (cb !== null) {
87-
cb();
88-
}
8991
});
9092
};
9193

@@ -152,22 +154,6 @@ export function isValidContainer(node: mixed): boolean {
152154
);
153155
}
154156

155-
export function warnOnInvalidCallback(
156-
callback: mixed,
157-
callerName: string,
158-
): void {
159-
if (__DEV__) {
160-
if (callback !== null && typeof callback !== 'function') {
161-
console.error(
162-
'%s(...): Expected the last optional `callback` argument to be a ' +
163-
'function. Instead received: %s.',
164-
callerName,
165-
callback,
166-
);
167-
}
168-
}
169-
}
170-
171157
function warnIfReactDOMContainerInDEV(container) {
172158
if (__DEV__) {
173159
if (isContainerMarkedAsRoot(container)) {

0 commit comments

Comments
 (0)