@samitouri / QOS-React-2 / commits / e26682a9f3

Removed Root API callback params and added warnings (#17916)

Brian Vaughn committed Jan 27, 2020 at 12:35 UTC e26682a9f3889439765942f1510f280466c3433a
3 files changed +61 -37
packages/react-dom/src/__tests__/ReactDOMRoot-test.js
+29
@@ -40,6 +40,35 @@ describe('ReactDOMRoot', () => {
40 expect(container.textContent).toEqual('Hi');
41 });
42
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 +
72 it('unmounts children', () => {
73 const root = ReactDOM.createRoot(container);
74 root.render(<div>Hi</div>);
packages/react-dom/src/client/ReactDOMLegacy.js
+14 -5
@@ -16,11 +16,7 @@ import {
16 isContainerMarkedAsRoot,
17 unmarkContainerAsRoot,
18 } from './ReactDOMComponentTree';
19 -import {
20 - createLegacyRoot,
21 - isValidContainer,
22 - warnOnInvalidCallback,
23 -} from './ReactDOMRoot';
19 +import {createLegacyRoot, isValidContainer} from './ReactDOMRoot';
20 import {ROOT_ATTRIBUTE_NAME} from '../shared/DOMProperty';
21 import {
22 DOCUMENT_NODE,
@@ -163,6 +159,19 @@ function legacyCreateRootFromDOMContainer(
159 );
160 }
161
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 +
175 function legacyRenderSubtreeIntoContainer(
176 parentComponent: ?React$Component<any, any>,
177 children: ReactNodeList,
packages/react-dom/src/client/ReactDOMRoot.js
+18 -32
@@ -15,8 +15,8 @@ import type {ReactNodeList} from 'shared/ReactTypes';
15 import type {FiberRoot} from 'react-reconciler/src/ReactFiberRoot';
16
17 export type RootType = {
18 - render(children: ReactNodeList, callback: ?() => mixed): void,
19 - unmount(callback: ?() => mixed): void,
18 + render(children: ReactNodeList): void,
19 + unmount(): void,
20 _internalRoot: FiberRoot,
21 ...
22 };
@@ -62,30 +62,32 @@ function ReactDOMBlockingRoot(
62
63 ReactDOMRoot.prototype.render = ReactDOMBlockingRoot.prototype.render = function(
64 children: ReactNodeList,
65 - callback: ?() => mixed,
65 ): void {
67 - const root = this._internalRoot;
68 - const cb = callback === undefined ? null : callback;
66 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 + }
73 }
72 - updateContainer(children, root, null, cb);
74 + const root = this._internalRoot;
75 + updateContainer(children, root, null, null);
76 };
77
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 {
79 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 + }
86 }
87 + const root = this._internalRoot;
88 const container = root.containerInfo;
89 updateContainer(null, root, null, () => {
90 unmarkContainerAsRoot(container);
86 - if (cb !== null) {
87 - cb();
88 - }
91 });
92 };
93
@@ -152,22 +154,6 @@ export function isValidContainer(node: mixed): boolean {
154 );
155 }
156
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 -
157 function warnIfReactDOMContainerInDEV(container) {
158 if (__DEV__) {
159 if (isContainerMarkedAsRoot(container)) {