Skip to content

Commit 17d70df

Browse files
authored
Warn when mixing createRoot() and old APIs (#14615)
* Warn when mixing createRoot() and old APIs * Move container checks to entry points This way further warning check doesn't crash on bad inputs. * Fix Flow * Rename flag to be clearer * managed by -> passed to * Revert accidental change * Fix Fire shim to match
1 parent 4846809 commit 17d70df

File tree

3 files changed

+202
-16
lines changed

3 files changed

+202
-16
lines changed

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

+100
Original file line numberDiff line numberDiff line change
@@ -374,4 +374,104 @@ describe('ReactDOMRoot', () => {
374374
'unstable_createRoot(...): Target container is not a DOM element.',
375375
);
376376
});
377+
378+
it('warns when rendering with legacy API into createRoot() container', () => {
379+
const root = ReactDOM.unstable_createRoot(container);
380+
root.render(<div>Hi</div>);
381+
jest.runAllTimers();
382+
expect(container.textContent).toEqual('Hi');
383+
expect(() => {
384+
ReactDOM.render(<div>Bye</div>, container);
385+
}).toWarnDev(
386+
[
387+
// We care about this warning:
388+
'You are calling ReactDOM.render() on a container that was previously ' +
389+
'passed to ReactDOM.unstable_createRoot(). This is not supported. ' +
390+
'Did you mean to call root.render(element)?',
391+
// This is more of a symptom but restructuring the code to avoid it isn't worth it:
392+
'Replacing React-rendered children with a new root component.',
393+
],
394+
{withoutStack: true},
395+
);
396+
jest.runAllTimers();
397+
// This works now but we could disallow it:
398+
expect(container.textContent).toEqual('Bye');
399+
});
400+
401+
it('warns when hydrating with legacy API into createRoot() container', () => {
402+
const root = ReactDOM.unstable_createRoot(container);
403+
root.render(<div>Hi</div>);
404+
jest.runAllTimers();
405+
expect(container.textContent).toEqual('Hi');
406+
expect(() => {
407+
ReactDOM.hydrate(<div>Hi</div>, container);
408+
}).toWarnDev(
409+
[
410+
// We care about this warning:
411+
'You are calling ReactDOM.hydrate() on a container that was previously ' +
412+
'passed to ReactDOM.unstable_createRoot(). This is not supported. ' +
413+
'Did you mean to call root.render(element, {hydrate: true})?',
414+
// This is more of a symptom but restructuring the code to avoid it isn't worth it:
415+
'Replacing React-rendered children with a new root component.',
416+
],
417+
{withoutStack: true},
418+
);
419+
});
420+
421+
it('warns when unmounting with legacy API (no previous content)', () => {
422+
const root = ReactDOM.unstable_createRoot(container);
423+
root.render(<div>Hi</div>);
424+
jest.runAllTimers();
425+
expect(container.textContent).toEqual('Hi');
426+
let unmounted = false;
427+
expect(() => {
428+
unmounted = ReactDOM.unmountComponentAtNode(container);
429+
}).toWarnDev(
430+
[
431+
// We care about this warning:
432+
'You are calling ReactDOM.unmountComponentAtNode() on a container that was previously ' +
433+
'passed to ReactDOM.unstable_createRoot(). This is not supported. Did you mean to call root.unmount()?',
434+
// This is more of a symptom but restructuring the code to avoid it isn't worth it:
435+
"The node you're attempting to unmount was rendered by React and is not a top-level container.",
436+
],
437+
{withoutStack: true},
438+
);
439+
expect(unmounted).toBe(false);
440+
jest.runAllTimers();
441+
expect(container.textContent).toEqual('Hi');
442+
root.unmount();
443+
jest.runAllTimers();
444+
expect(container.textContent).toEqual('');
445+
});
446+
447+
it('warns when unmounting with legacy API (has previous content)', () => {
448+
// Currently createRoot().render() doesn't clear this.
449+
container.appendChild(document.createElement('div'));
450+
// The rest is the same as test above.
451+
const root = ReactDOM.unstable_createRoot(container);
452+
root.render(<div>Hi</div>);
453+
jest.runAllTimers();
454+
expect(container.textContent).toEqual('Hi');
455+
let unmounted = false;
456+
expect(() => {
457+
unmounted = ReactDOM.unmountComponentAtNode(container);
458+
}).toWarnDev('Did you mean to call root.unmount()?', {withoutStack: true});
459+
expect(unmounted).toBe(false);
460+
jest.runAllTimers();
461+
expect(container.textContent).toEqual('Hi');
462+
root.unmount();
463+
jest.runAllTimers();
464+
expect(container.textContent).toEqual('');
465+
});
466+
467+
it('warns when passing legacy container to createRoot()', () => {
468+
ReactDOM.render(<div>Hi</div>, container);
469+
expect(() => {
470+
ReactDOM.unstable_createRoot(container);
471+
}).toWarnDev(
472+
'You are calling ReactDOM.unstable_createRoot() on a container that was previously ' +
473+
'passed to ReactDOM.render(). This is not supported.',
474+
{withoutStack: true},
475+
);
476+
});
377477
});

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

+51-8
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import type {
1414
FiberRoot,
1515
Batch as FiberRootBatch,
1616
} from 'react-reconciler/src/ReactFiberRoot';
17-
import type {Container} from './ReactDOMHostConfig';
1817

1918
import '../shared/checkReact';
2019
import './ReactDOMClientInjection';
@@ -160,9 +159,11 @@ setRestoreImplementation(restoreControlledState);
160159
export type DOMContainer =
161160
| (Element & {
162161
_reactRootContainer: ?Root,
162+
_reactHasBeenPassedToCreateRootDEV: ?boolean,
163163
})
164164
| (Document & {
165165
_reactRootContainer: ?Root,
166+
_reactHasBeenPassedToCreateRootDEV: ?boolean,
166167
});
167168

168169
type Batch = FiberRootBatch & {
@@ -362,7 +363,7 @@ ReactWork.prototype._onCommit = function(): void {
362363
};
363364

364365
function ReactRoot(
365-
container: Container,
366+
container: DOMContainer,
366367
isConcurrent: boolean,
367368
hydrate: boolean,
368369
) {
@@ -543,12 +544,6 @@ function legacyRenderSubtreeIntoContainer(
543544
forceHydrate: boolean,
544545
callback: ?Function,
545546
) {
546-
// TODO: Ensure all entry points contain this check
547-
invariant(
548-
isValidContainer(container),
549-
'Target container is not a DOM element.',
550-
);
551-
552547
if (__DEV__) {
553548
topLevelUpdateWarnings(container);
554549
}
@@ -652,6 +647,19 @@ const ReactDOM: Object = {
652647
},
653648

654649
hydrate(element: React$Node, container: DOMContainer, callback: ?Function) {
650+
invariant(
651+
isValidContainer(container),
652+
'Target container is not a DOM element.',
653+
);
654+
if (__DEV__) {
655+
warningWithoutStack(
656+
!container._reactHasBeenPassedToCreateRootDEV,
657+
'You are calling ReactDOM.hydrate() on a container that was previously ' +
658+
'passed to ReactDOM.%s(). This is not supported. ' +
659+
'Did you mean to call root.render(element, {hydrate: true})?',
660+
enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot',
661+
);
662+
}
655663
// TODO: throw or warn if we couldn't hydrate?
656664
return legacyRenderSubtreeIntoContainer(
657665
null,
@@ -667,6 +675,19 @@ const ReactDOM: Object = {
667675
container: DOMContainer,
668676
callback: ?Function,
669677
) {
678+
invariant(
679+
isValidContainer(container),
680+
'Target container is not a DOM element.',
681+
);
682+
if (__DEV__) {
683+
warningWithoutStack(
684+
!container._reactHasBeenPassedToCreateRootDEV,
685+
'You are calling ReactDOM.render() on a container that was previously ' +
686+
'passed to ReactDOM.%s(). This is not supported. ' +
687+
'Did you mean to call root.render(element)?',
688+
enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot',
689+
);
690+
}
670691
return legacyRenderSubtreeIntoContainer(
671692
null,
672693
element,
@@ -682,6 +703,10 @@ const ReactDOM: Object = {
682703
containerNode: DOMContainer,
683704
callback: ?Function,
684705
) {
706+
invariant(
707+
isValidContainer(containerNode),
708+
'Target container is not a DOM element.',
709+
);
685710
invariant(
686711
parentComponent != null && hasInstance(parentComponent),
687712
'parentComponent must be a valid React Component',
@@ -701,6 +726,15 @@ const ReactDOM: Object = {
701726
'unmountComponentAtNode(...): Target container is not a DOM element.',
702727
);
703728

729+
if (__DEV__) {
730+
warningWithoutStack(
731+
!container._reactHasBeenPassedToCreateRootDEV,
732+
'You are calling ReactDOM.unmountComponentAtNode() on a container that was previously ' +
733+
'passed to ReactDOM.%s(). This is not supported. Did you mean to call root.unmount()?',
734+
enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot',
735+
);
736+
}
737+
704738
if (container._reactRootContainer) {
705739
if (__DEV__) {
706740
const rootEl = getReactRootElementInContainer(container);
@@ -805,6 +839,15 @@ function createRoot(container: DOMContainer, options?: RootOptions): ReactRoot {
805839
'%s(...): Target container is not a DOM element.',
806840
functionName,
807841
);
842+
if (__DEV__) {
843+
warningWithoutStack(
844+
!container._reactRootContainer,
845+
'You are calling ReactDOM.%s() on a container that was previously ' +
846+
'passed to ReactDOM.render(). This is not supported.',
847+
enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot',
848+
);
849+
container._reactHasBeenPassedToCreateRootDEV = true;
850+
}
808851
const hydrate = options != null && options.hydrate === true;
809852
return new ReactRoot(container, true, hydrate);
810853
}

packages/react-dom/src/fire/ReactFire.js

+51-8
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import type {
1919
FiberRoot,
2020
Batch as FiberRootBatch,
2121
} from 'react-reconciler/src/ReactFiberRoot';
22-
import type {Container} from '../client/ReactDOMHostConfig';
2322

2423
import '../shared/checkReact';
2524
import '../client/ReactDOMClientInjection';
@@ -165,9 +164,11 @@ setRestoreImplementation(restoreControlledState);
165164
export type DOMContainer =
166165
| (Element & {
167166
_reactRootContainer: ?Root,
167+
_reactHasBeenPassedToCreateRootDEV: ?boolean,
168168
})
169169
| (Document & {
170170
_reactRootContainer: ?Root,
171+
_reactHasBeenPassedToCreateRootDEV: ?boolean,
171172
});
172173

173174
type Batch = FiberRootBatch & {
@@ -367,7 +368,7 @@ ReactWork.prototype._onCommit = function(): void {
367368
};
368369

369370
function ReactRoot(
370-
container: Container,
371+
container: DOMContainer,
371372
isConcurrent: boolean,
372373
hydrate: boolean,
373374
) {
@@ -548,12 +549,6 @@ function legacyRenderSubtreeIntoContainer(
548549
forceHydrate: boolean,
549550
callback: ?Function,
550551
) {
551-
// TODO: Ensure all entry points contain this check
552-
invariant(
553-
isValidContainer(container),
554-
'Target container is not a DOM element.',
555-
);
556-
557552
if (__DEV__) {
558553
topLevelUpdateWarnings(container);
559554
}
@@ -657,6 +652,19 @@ const ReactDOM: Object = {
657652
},
658653

659654
hydrate(element: React$Node, container: DOMContainer, callback: ?Function) {
655+
invariant(
656+
isValidContainer(container),
657+
'Target container is not a DOM element.',
658+
);
659+
if (__DEV__) {
660+
warningWithoutStack(
661+
!container._reactHasBeenPassedToCreateRootDEV,
662+
'You are calling ReactDOM.hydrate() on a container that was previously ' +
663+
'passed to ReactDOM.%s(). This is not supported. ' +
664+
'Did you mean to call root.render(element, {hydrate: true})?',
665+
enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot',
666+
);
667+
}
660668
// TODO: throw or warn if we couldn't hydrate?
661669
return legacyRenderSubtreeIntoContainer(
662670
null,
@@ -672,6 +680,19 @@ const ReactDOM: Object = {
672680
container: DOMContainer,
673681
callback: ?Function,
674682
) {
683+
invariant(
684+
isValidContainer(container),
685+
'Target container is not a DOM element.',
686+
);
687+
if (__DEV__) {
688+
warningWithoutStack(
689+
!container._reactHasBeenPassedToCreateRootDEV,
690+
'You are calling ReactDOM.render() on a container that was previously ' +
691+
'passed to ReactDOM.%s(). This is not supported. ' +
692+
'Did you mean to call root.render(element)?',
693+
enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot',
694+
);
695+
}
675696
return legacyRenderSubtreeIntoContainer(
676697
null,
677698
element,
@@ -687,6 +708,10 @@ const ReactDOM: Object = {
687708
containerNode: DOMContainer,
688709
callback: ?Function,
689710
) {
711+
invariant(
712+
isValidContainer(containerNode),
713+
'Target container is not a DOM element.',
714+
);
690715
invariant(
691716
parentComponent != null && hasInstance(parentComponent),
692717
'parentComponent must be a valid React Component',
@@ -706,6 +731,15 @@ const ReactDOM: Object = {
706731
'unmountComponentAtNode(...): Target container is not a DOM element.',
707732
);
708733

734+
if (__DEV__) {
735+
warningWithoutStack(
736+
!container._reactHasBeenPassedToCreateRootDEV,
737+
'You are calling ReactDOM.unmountComponentAtNode() on a container that was previously ' +
738+
'passed to ReactDOM.%s(). This is not supported. Did you mean to call root.unmount()?',
739+
enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot',
740+
);
741+
}
742+
709743
if (container._reactRootContainer) {
710744
if (__DEV__) {
711745
const rootEl = getReactRootElementInContainer(container);
@@ -810,6 +844,15 @@ function createRoot(container: DOMContainer, options?: RootOptions): ReactRoot {
810844
'%s(...): Target container is not a DOM element.',
811845
functionName,
812846
);
847+
if (__DEV__) {
848+
warningWithoutStack(
849+
!container._reactRootContainer,
850+
'You are calling ReactDOM.%s() on a container that was previously ' +
851+
'passed to ReactDOM.render(). This is not supported.',
852+
enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot',
853+
);
854+
container._reactHasBeenPassedToCreateRootDEV = true;
855+
}
813856
const hydrate = options != null && options.hydrate === true;
814857
return new ReactRoot(container, true, hydrate);
815858
}

0 commit comments

Comments
 (0)