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
Dan Abramov committed
Jan 18, 2019 at 00:20 UTC
17d70df91939b3459813660d90871ae41096d349
3 files changed
+202
-16
packages/react-dom/src/__tests__/ReactDOMRoot-test.js
+100
@@ -374,4 +374,104 @@ describe('ReactDOMRoot', () => {
374
'unstable_createRoot(...): Target container is not a DOM element.',
375
);
376
});
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
+ });
477
});
packages/react-dom/src/client/ReactDOM.js
+51
-8
@@ -14,7 +14,6 @@ import type {
14
FiberRoot,
15
Batch as FiberRootBatch,
16
} from 'react-reconciler/src/ReactFiberRoot';
17
-import type {Container} from './ReactDOMHostConfig';
17
18
import '../shared/checkReact';
19
import './ReactDOMClientInjection';
@@ -160,9 +159,11 @@ setRestoreImplementation(restoreControlledState);
159
export type DOMContainer =
160
| (Element & {
161
_reactRootContainer: ?Root,
162
+ _reactHasBeenPassedToCreateRootDEV: ?boolean,
163
})
164
| (Document & {
165
_reactRootContainer: ?Root,
166
+ _reactHasBeenPassedToCreateRootDEV: ?boolean,
167
});
168
169
type Batch = FiberRootBatch & {
@@ -362,7 +363,7 @@ ReactWork.prototype._onCommit = function(): void {
363
};
364
365
function ReactRoot(
365
- container: Container,
366
+ container: DOMContainer,
367
isConcurrent: boolean,
368
hydrate: boolean,
369
) {
@@ -543,12 +544,6 @@ function legacyRenderSubtreeIntoContainer(
544
forceHydrate: boolean,
545
callback: ?Function,
546
) {
546
- // TODO: Ensure all entry points contain this check
547
- invariant(
548
- isValidContainer(container),
549
- 'Target container is not a DOM element.',
550
- );
551
-
547
if (__DEV__) {
548
topLevelUpdateWarnings(container);
549
}
@@ -652,6 +647,19 @@ const ReactDOM: Object = {
647
},
648
649
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
+ }
663
// TODO: throw or warn if we couldn't hydrate?
664
return legacyRenderSubtreeIntoContainer(
665
null,
@@ -667,6 +675,19 @@ const ReactDOM: Object = {
675
container: DOMContainer,
676
callback: ?Function,
677
) {
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
+ }
691
return legacyRenderSubtreeIntoContainer(
692
null,
693
element,
@@ -682,6 +703,10 @@ const ReactDOM: Object = {
703
containerNode: DOMContainer,
704
callback: ?Function,
705
) {
706
+ invariant(
707
+ isValidContainer(containerNode),
708
+ 'Target container is not a DOM element.',
709
+ );
710
invariant(
711
parentComponent != null && hasInstance(parentComponent),
712
'parentComponent must be a valid React Component',
@@ -701,6 +726,15 @@ const ReactDOM: Object = {
726
'unmountComponentAtNode(...): Target container is not a DOM element.',
727
);
728
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
+
738
if (container._reactRootContainer) {
739
if (__DEV__) {
740
const rootEl = getReactRootElementInContainer(container);
@@ -805,6 +839,15 @@ function createRoot(container: DOMContainer, options?: RootOptions): ReactRoot {
839
'%s(...): Target container is not a DOM element.',
840
functionName,
841
);
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
+ }
851
const hydrate = options != null && options.hydrate === true;
852
return new ReactRoot(container, true, hydrate);
853
}
packages/react-dom/src/fire/ReactFire.js
+51
-8
@@ -19,7 +19,6 @@ import type {
19
FiberRoot,
20
Batch as FiberRootBatch,
21
} from 'react-reconciler/src/ReactFiberRoot';
22
-import type {Container} from '../client/ReactDOMHostConfig';
22
23
import '../shared/checkReact';
24
import '../client/ReactDOMClientInjection';
@@ -165,9 +164,11 @@ setRestoreImplementation(restoreControlledState);
164
export type DOMContainer =
165
| (Element & {
166
_reactRootContainer: ?Root,
167
+ _reactHasBeenPassedToCreateRootDEV: ?boolean,
168
})
169
| (Document & {
170
_reactRootContainer: ?Root,
171
+ _reactHasBeenPassedToCreateRootDEV: ?boolean,
172
});
173
174
type Batch = FiberRootBatch & {
@@ -367,7 +368,7 @@ ReactWork.prototype._onCommit = function(): void {
368
};
369
370
function ReactRoot(
370
- container: Container,
371
+ container: DOMContainer,
372
isConcurrent: boolean,
373
hydrate: boolean,
374
) {
@@ -548,12 +549,6 @@ function legacyRenderSubtreeIntoContainer(
549
forceHydrate: boolean,
550
callback: ?Function,
551
) {
551
- // TODO: Ensure all entry points contain this check
552
- invariant(
553
- isValidContainer(container),
554
- 'Target container is not a DOM element.',
555
- );
556
-
552
if (__DEV__) {
553
topLevelUpdateWarnings(container);
554
}
@@ -657,6 +652,19 @@ const ReactDOM: Object = {
652
},
653
654
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
+ }
668
// TODO: throw or warn if we couldn't hydrate?
669
return legacyRenderSubtreeIntoContainer(
670
null,
@@ -672,6 +680,19 @@ const ReactDOM: Object = {
680
container: DOMContainer,
681
callback: ?Function,
682
) {
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
+ }
696
return legacyRenderSubtreeIntoContainer(
697
null,
698
element,
@@ -687,6 +708,10 @@ const ReactDOM: Object = {
708
containerNode: DOMContainer,
709
callback: ?Function,
710
) {
711
+ invariant(
712
+ isValidContainer(containerNode),
713
+ 'Target container is not a DOM element.',
714
+ );
715
invariant(
716
parentComponent != null && hasInstance(parentComponent),
717
'parentComponent must be a valid React Component',
@@ -706,6 +731,15 @@ const ReactDOM: Object = {
731
'unmountComponentAtNode(...): Target container is not a DOM element.',
732
);
733
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
+
743
if (container._reactRootContainer) {
744
if (__DEV__) {
745
const rootEl = getReactRootElementInContainer(container);
@@ -810,6 +844,15 @@ function createRoot(container: DOMContainer, options?: RootOptions): ReactRoot {
844
'%s(...): Target container is not a DOM element.',
845
functionName,
846
);
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
+ }
856
const hydrate = options != null && options.hydrate === true;
857
return new ReactRoot(container, true, hydrate);
858
}