Improve warning for invalid class contextType (#15142)
* Improve warning for invalid class contextType * Don't warn for null * Grammar
Dan Abramov committed
Mar 19, 2019 at 13:31 UTC
7ad7386308abd55f12ad9f1cddac6a97860e92eb
4 files changed
+243
-33
packages/react-dom/src/__tests__/ReactServerRendering-test.js
+82
@@ -943,4 +943,86 @@ describe('ReactDOMServer', () => {
943
{withoutStack: true},
944
);
945
});
946
+
947
+ it('should not warn when class contextType is null', () => {
948
+ class Foo extends React.Component {
949
+ static contextType = null; // Handy for conditional declaration
950
+ render() {
951
+ return this.context.hello.world;
952
+ }
953
+ }
954
+
955
+ expect(() => {
956
+ ReactDOMServer.renderToString(<Foo />);
957
+ }).toThrow("Cannot read property 'world' of undefined");
958
+ });
959
+
960
+ it('should warn when class contextType is undefined', () => {
961
+ class Foo extends React.Component {
962
+ // This commonly happens with circular deps
963
+ // https://github.com/facebook/react/issues/13969
964
+ static contextType = undefined;
965
+ render() {
966
+ return this.context.hello.world;
967
+ }
968
+ }
969
+
970
+ expect(() => {
971
+ expect(() => {
972
+ ReactDOMServer.renderToString(<Foo />);
973
+ }).toThrow("Cannot read property 'world' of undefined");
974
+ }).toWarnDev(
975
+ 'Foo defines an invalid contextType. ' +
976
+ 'contextType should point to the Context object returned by React.createContext(). ' +
977
+ 'However, it is set to undefined. ' +
978
+ 'This can be caused by a typo or by mixing up named and default imports. ' +
979
+ 'This can also happen due to a circular dependency, ' +
980
+ 'so try moving the createContext() call to a separate file.',
981
+ {withoutStack: true},
982
+ );
983
+ });
984
+
985
+ it('should warn when class contextType is an object', () => {
986
+ class Foo extends React.Component {
987
+ // Can happen due to a typo
988
+ static contextType = {
989
+ x: 42,
990
+ y: 'hello',
991
+ };
992
+ render() {
993
+ return this.context.hello.world;
994
+ }
995
+ }
996
+
997
+ expect(() => {
998
+ expect(() => {
999
+ ReactDOMServer.renderToString(<Foo />);
1000
+ }).toThrow("Cannot read property 'hello' of undefined");
1001
+ }).toWarnDev(
1002
+ 'Foo defines an invalid contextType. ' +
1003
+ 'contextType should point to the Context object returned by React.createContext(). ' +
1004
+ 'However, it is set to an object with keys {x, y}.',
1005
+ {withoutStack: true},
1006
+ );
1007
+ });
1008
+
1009
+ it('should warn when class contextType is a primitive', () => {
1010
+ class Foo extends React.Component {
1011
+ static contextType = 'foo';
1012
+ render() {
1013
+ return this.context.hello.world;
1014
+ }
1015
+ }
1016
+
1017
+ expect(() => {
1018
+ expect(() => {
1019
+ ReactDOMServer.renderToString(<Foo />);
1020
+ }).toThrow("Cannot read property 'world' of undefined");
1021
+ }).toWarnDev(
1022
+ 'Foo defines an invalid contextType. ' +
1023
+ 'contextType should point to the Context object returned by React.createContext(). ' +
1024
+ 'However, it is set to a string.',
1025
+ {withoutStack: true},
1026
+ );
1027
+ });
1028
});
packages/react-dom/src/server/ReactPartialRendererContext.js
+43
-20
@@ -10,19 +10,19 @@
10
import type {ThreadID} from './ReactThreadIDAllocator';
11
import type {ReactContext} from 'shared/ReactTypes';
12
13
-import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
13
+import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols';
14
import ReactSharedInternals from 'shared/ReactSharedInternals';
15
import getComponentName from 'shared/getComponentName';
16
import warningWithoutStack from 'shared/warningWithoutStack';
17
import checkPropTypes from 'prop-types/checkPropTypes';
18
19
let ReactDebugCurrentFrame;
20
+let didWarnAboutInvalidateContextType;
21
if (__DEV__) {
22
ReactDebugCurrentFrame = ReactSharedInternals.ReactDebugCurrentFrame;
23
+ didWarnAboutInvalidateContextType = new Set();
24
}
25
24
-const didWarnAboutInvalidateContextType = {};
25
-
26
export const emptyObject = {};
27
if (__DEV__) {
28
Object.freeze(emptyObject);
@@ -75,26 +75,49 @@ export function processContext(
75
threadID: ThreadID,
76
) {
77
const contextType = type.contextType;
78
- if (typeof contextType === 'object' && contextType !== null) {
79
- if (__DEV__) {
80
- const isContextConsumer =
81
- contextType.$$typeof === REACT_CONTEXT_TYPE &&
82
- contextType._context !== undefined;
83
- if (contextType.$$typeof !== REACT_CONTEXT_TYPE || isContextConsumer) {
84
- let name = getComponentName(type) || 'Component';
85
- if (!didWarnAboutInvalidateContextType[name]) {
86
- didWarnAboutInvalidateContextType[name] = true;
87
- warningWithoutStack(
88
- false,
89
- '%s defines an invalid contextType. ' +
90
- 'contextType should point to the Context object returned by React.createContext(). ' +
91
- 'Did you accidentally pass the Context.%s instead?',
92
- name,
93
- isContextConsumer ? 'Consumer' : 'Provider',
94
- );
78
+ if (__DEV__) {
79
+ if ('contextType' in (type: any)) {
80
+ let isValid =
81
+ // Allow null for conditional declaration
82
+ contextType === null ||
83
+ (contextType !== undefined &&
84
+ contextType.$$typeof === REACT_CONTEXT_TYPE &&
85
+ contextType._context === undefined); // Not a <Context.Consumer>
86
+
87
+ if (!isValid && !didWarnAboutInvalidateContextType.has(type)) {
88
+ didWarnAboutInvalidateContextType.add(type);
89
+
90
+ let addendum = '';
91
+ if (contextType === undefined) {
92
+ addendum =
93
+ ' However, it is set to undefined. ' +
94
+ 'This can be caused by a typo or by mixing up named and default imports. ' +
95
+ 'This can also happen due to a circular dependency, so ' +
96
+ 'try moving the createContext() call to a separate file.';
97
+ } else if (typeof contextType !== 'object') {
98
+ addendum = ' However, it is set to a ' + typeof contextType + '.';
99
+ } else if (contextType.$$typeof === REACT_PROVIDER_TYPE) {
100
+ addendum = ' Did you accidentally pass the Context.Provider instead?';
101
+ } else if (contextType._context !== undefined) {
102
+ // <Context.Consumer>
103
+ addendum = ' Did you accidentally pass the Context.Consumer instead?';
104
+ } else {
105
+ addendum =
106
+ ' However, it is set to an object with keys {' +
107
+ Object.keys(contextType).join(', ') +
108
+ '}.';
109
}
110
+ warningWithoutStack(
111
+ false,
112
+ '%s defines an invalid contextType. ' +
113
+ 'contextType should point to the Context object returned by React.createContext().%s',
114
+ getComponentName(type) || 'Component',
115
+ addendum,
116
+ );
117
}
118
}
119
+ }
120
+ if (typeof contextType === 'object' && contextType !== null) {
121
validateContextBounds(contextType, threadID);
122
return contextType[threadID];
123
} else {
packages/react-reconciler/src/ReactFiberClassComponent.js
+37
-13
@@ -24,7 +24,7 @@ import shallowEqual from 'shared/shallowEqual';
24
import getComponentName from 'shared/getComponentName';
25
import invariant from 'shared/invariant';
26
import warningWithoutStack from 'shared/warningWithoutStack';
27
-import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
27
+import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols';
28
29
import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
30
import {resolveDefaultProps} from './ReactFiberLazyComponent';
@@ -513,27 +513,51 @@ function constructClassInstance(
513
let unmaskedContext = emptyContextObject;
514
let context = null;
515
const contextType = ctor.contextType;
516
- if (typeof contextType === 'object' && contextType !== null) {
517
- if (__DEV__) {
518
- const isContextConsumer =
519
- contextType.$$typeof === REACT_CONTEXT_TYPE &&
520
- contextType._context !== undefined;
521
- if (
522
- (contextType.$$typeof !== REACT_CONTEXT_TYPE || isContextConsumer) &&
523
- !didWarnAboutInvalidateContextType.has(ctor)
524
- ) {
516
+
517
+ if (__DEV__) {
518
+ if ('contextType' in ctor) {
519
+ let isValid =
520
+ // Allow null for conditional declaration
521
+ contextType === null ||
522
+ (contextType !== undefined &&
523
+ contextType.$$typeof === REACT_CONTEXT_TYPE &&
524
+ contextType._context === undefined); // Not a <Context.Consumer>
525
+
526
+ if (!isValid && !didWarnAboutInvalidateContextType.has(ctor)) {
527
didWarnAboutInvalidateContextType.add(ctor);
528
+
529
+ let addendum = '';
530
+ if (contextType === undefined) {
531
+ addendum =
532
+ ' However, it is set to undefined. ' +
533
+ 'This can be caused by a typo or by mixing up named and default imports. ' +
534
+ 'This can also happen due to a circular dependency, so ' +
535
+ 'try moving the createContext() call to a separate file.';
536
+ } else if (typeof contextType !== 'object') {
537
+ addendum = ' However, it is set to a ' + typeof contextType + '.';
538
+ } else if (contextType.$$typeof === REACT_PROVIDER_TYPE) {
539
+ addendum = ' Did you accidentally pass the Context.Provider instead?';
540
+ } else if (contextType._context !== undefined) {
541
+ // <Context.Consumer>
542
+ addendum = ' Did you accidentally pass the Context.Consumer instead?';
543
+ } else {
544
+ addendum =
545
+ ' However, it is set to an object with keys {' +
546
+ Object.keys(contextType).join(', ') +
547
+ '}.';
548
+ }
549
warningWithoutStack(
550
false,
551
'%s defines an invalid contextType. ' +
529
- 'contextType should point to the Context object returned by React.createContext(). ' +
530
- 'Did you accidentally pass the Context.%s instead?',
552
+ 'contextType should point to the Context object returned by React.createContext().%s',
553
getComponentName(ctor) || 'Component',
532
- isContextConsumer ? 'Consumer' : 'Provider',
554
+ addendum,
555
);
556
}
557
}
558
+ }
559
560
+ if (typeof contextType === 'object' && contextType !== null) {
561
context = readContext((contextType: any));
562
} else {
563
unmaskedContext = getUnmaskedContext(workInProgress, ctor, true);
packages/react/src/__tests__/ReactContextValidator-test.js
+81
@@ -578,6 +578,87 @@ describe('ReactContextValidator', () => {
578
);
579
});
580
581
+ it('should not warn when class contextType is null', () => {
582
+ class Foo extends React.Component {
583
+ static contextType = null; // Handy for conditional declaration
584
+ render() {
585
+ return this.context.hello.world;
586
+ }
587
+ }
588
+ expect(() => {
589
+ ReactTestUtils.renderIntoDocument(<Foo />);
590
+ }).toThrow("Cannot read property 'world' of undefined");
591
+ });
592
+
593
+ it('should warn when class contextType is undefined', () => {
594
+ class Foo extends React.Component {
595
+ // This commonly happens with circular deps
596
+ // https://github.com/facebook/react/issues/13969
597
+ static contextType = undefined;
598
+ render() {
599
+ return this.context.hello.world;
600
+ }
601
+ }
602
+
603
+ expect(() => {
604
+ expect(() => {
605
+ ReactTestUtils.renderIntoDocument(<Foo />);
606
+ }).toThrow("Cannot read property 'world' of undefined");
607
+ }).toWarnDev(
608
+ 'Foo defines an invalid contextType. ' +
609
+ 'contextType should point to the Context object returned by React.createContext(). ' +
610
+ 'However, it is set to undefined. ' +
611
+ 'This can be caused by a typo or by mixing up named and default imports. ' +
612
+ 'This can also happen due to a circular dependency, ' +
613
+ 'so try moving the createContext() call to a separate file.',
614
+ {withoutStack: true},
615
+ );
616
+ });
617
+
618
+ it('should warn when class contextType is an object', () => {
619
+ class Foo extends React.Component {
620
+ // Can happen due to a typo
621
+ static contextType = {
622
+ x: 42,
623
+ y: 'hello',
624
+ };
625
+ render() {
626
+ return this.context.hello.world;
627
+ }
628
+ }
629
+
630
+ expect(() => {
631
+ expect(() => {
632
+ ReactTestUtils.renderIntoDocument(<Foo />);
633
+ }).toThrow("Cannot read property 'hello' of undefined");
634
+ }).toWarnDev(
635
+ 'Foo defines an invalid contextType. ' +
636
+ 'contextType should point to the Context object returned by React.createContext(). ' +
637
+ 'However, it is set to an object with keys {x, y}.',
638
+ {withoutStack: true},
639
+ );
640
+ });
641
+
642
+ it('should warn when class contextType is a primitive', () => {
643
+ class Foo extends React.Component {
644
+ static contextType = 'foo';
645
+ render() {
646
+ return this.context.hello.world;
647
+ }
648
+ }
649
+
650
+ expect(() => {
651
+ expect(() => {
652
+ ReactTestUtils.renderIntoDocument(<Foo />);
653
+ }).toThrow("Cannot read property 'world' of undefined");
654
+ }).toWarnDev(
655
+ 'Foo defines an invalid contextType. ' +
656
+ 'contextType should point to the Context object returned by React.createContext(). ' +
657
+ 'However, it is set to a string.',
658
+ {withoutStack: true},
659
+ );
660
+ });
661
+
662
it('should warn if you define contextType on a function component', () => {
663
const Context = React.createContext();
664