Skip to content

Commit

Permalink
Improve warning for invalid class contextType (#15142)
Browse files Browse the repository at this point in the history 
* Improve warning for invalid class contextType

* Don't warn for null

* Grammar
  • Loading branch information
@gaearon
gaearon committed Mar 27, 2019
1 parent aa8736a commit e0c2c56
Show file tree
Hide file tree
Showing 4 changed files with 243 additions and 33 deletions.
82 changes: 82 additions & 0 deletions packages/react-dom/src/__tests__/ReactServerRendering-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -943,4 +943,86 @@ describe('ReactDOMServer', () => {
{withoutStack: true},
);
});

it('should not warn when class contextType is null', () => {
class Foo extends React.Component {
static contextType = null; // Handy for conditional declaration
render() {
return this.context.hello.world;
}
}

expect(() => {
ReactDOMServer.renderToString(<Foo />);
}).toThrow("Cannot read property 'world' of undefined");
});

it('should warn when class contextType is undefined', () => {
class Foo extends React.Component {
// This commonly happens with circular deps
// https://github.com/facebook/react/issues/13969
static contextType = undefined;
render() {
return this.context.hello.world;
}
}

expect(() => {
expect(() => {
ReactDOMServer.renderToString(<Foo />);
}).toThrow("Cannot read property 'world' of undefined");
}).toWarnDev(
'Foo defines an invalid contextType. ' +
'contextType should point to the Context object returned by React.createContext(). ' +
'However, it is set to undefined. ' +
'This can be caused by a typo or by mixing up named and default imports. ' +
'This can also happen due to a circular dependency, ' +
'so try moving the createContext() call to a separate file.',
{withoutStack: true},
);
});

it('should warn when class contextType is an object', () => {
class Foo extends React.Component {
// Can happen due to a typo
static contextType = {
x: 42,
y: 'hello',
};
render() {
return this.context.hello.world;
}
}

expect(() => {
expect(() => {
ReactDOMServer.renderToString(<Foo />);
}).toThrow("Cannot read property 'hello' of undefined");
}).toWarnDev(
'Foo defines an invalid contextType. ' +
'contextType should point to the Context object returned by React.createContext(). ' +
'However, it is set to an object with keys {x, y}.',
{withoutStack: true},
);
});

it('should warn when class contextType is a primitive', () => {
class Foo extends React.Component {
static contextType = 'foo';
render() {
return this.context.hello.world;
}
}

expect(() => {
expect(() => {
ReactDOMServer.renderToString(<Foo />);
}).toThrow("Cannot read property 'world' of undefined");
}).toWarnDev(
'Foo defines an invalid contextType. ' +
'contextType should point to the Context object returned by React.createContext(). ' +
'However, it is set to a string.',
{withoutStack: true},
);
});
});
63 changes: 43 additions & 20 deletions packages/react-dom/src/server/ReactPartialRendererContext.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,19 @@
import type {ThreadID} from './ReactThreadIDAllocator';
import type {ReactContext} from 'shared/ReactTypes';

import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import getComponentName from 'shared/getComponentName';
import warningWithoutStack from 'shared/warningWithoutStack';
import checkPropTypes from 'prop-types/checkPropTypes';

let ReactDebugCurrentFrame;
let didWarnAboutInvalidateContextType;
if (__DEV__) {
ReactDebugCurrentFrame = ReactSharedInternals.ReactDebugCurrentFrame;
didWarnAboutInvalidateContextType = new Set();
}

const didWarnAboutInvalidateContextType = {};

export const emptyObject = {};
if (__DEV__) {
Object.freeze(emptyObject);
Expand Down Expand Up @@ -75,26 +75,49 @@ export function processContext(
threadID: ThreadID,
) {
const contextType = type.contextType;
if (typeof contextType === 'object' && contextType !== null) {
if (__DEV__) {
const isContextConsumer =
contextType.$$typeof === REACT_CONTEXT_TYPE &&
contextType._context !== undefined;
if (contextType.$$typeof !== REACT_CONTEXT_TYPE || isContextConsumer) {
let name = getComponentName(type) || 'Component';
if (!didWarnAboutInvalidateContextType[name]) {
didWarnAboutInvalidateContextType[name] = true;
warningWithoutStack(
false,
'%s defines an invalid contextType. ' +
'contextType should point to the Context object returned by React.createContext(). ' +
'Did you accidentally pass the Context.%s instead?',
name,
isContextConsumer ? 'Consumer' : 'Provider',
);
if (__DEV__) {
if ('contextType' in (type: any)) {
let isValid =
// Allow null for conditional declaration
contextType === null ||
(contextType !== undefined &&
contextType.$$typeof === REACT_CONTEXT_TYPE &&
contextType._context === undefined); // Not a <Context.Consumer>

if (!isValid && !didWarnAboutInvalidateContextType.has(type)) {
didWarnAboutInvalidateContextType.add(type);

let addendum = '';
if (contextType === undefined) {
addendum =
' However, it is set to undefined. ' +
'This can be caused by a typo or by mixing up named and default imports. ' +
'This can also happen due to a circular dependency, so ' +
'try moving the createContext() call to a separate file.';
} else if (typeof contextType !== 'object') {
addendum = ' However, it is set to a ' + typeof contextType + '.';
} else if (contextType.$$typeof === REACT_PROVIDER_TYPE) {
addendum = ' Did you accidentally pass the Context.Provider instead?';
} else if (contextType._context !== undefined) {
// <Context.Consumer>
addendum = ' Did you accidentally pass the Context.Consumer instead?';
} else {
addendum =
' However, it is set to an object with keys {' +
Object.keys(contextType).join(', ') +
'}.';
}
warningWithoutStack(
false,
'%s defines an invalid contextType. ' +
'contextType should point to the Context object returned by React.createContext().%s',
getComponentName(type) || 'Component',
addendum,
);
}
}
}
if (typeof contextType === 'object' && contextType !== null) {
validateContextBounds(contextType, threadID);
return contextType[threadID];
} else {
Expand Down
50 changes: 37 additions & 13 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import shallowEqual from 'shared/shallowEqual';
import getComponentName from 'shared/getComponentName';
import invariant from 'shared/invariant';
import warningWithoutStack from 'shared/warningWithoutStack';
import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols';

import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
import {resolveDefaultProps} from './ReactFiberLazyComponent';
Expand Down Expand Up @@ -513,27 +513,51 @@ function constructClassInstance(
let unmaskedContext = emptyContextObject;
let context = null;
const contextType = ctor.contextType;
if (typeof contextType === 'object' && contextType !== null) {
if (__DEV__) {
const isContextConsumer =
contextType.$$typeof === REACT_CONTEXT_TYPE &&
contextType._context !== undefined;
if (
(contextType.$$typeof !== REACT_CONTEXT_TYPE || isContextConsumer) &&
!didWarnAboutInvalidateContextType.has(ctor)
) {

if (__DEV__) {
if ('contextType' in ctor) {
let isValid =
// Allow null for conditional declaration
contextType === null ||
(contextType !== undefined &&
contextType.$$typeof === REACT_CONTEXT_TYPE &&
contextType._context === undefined); // Not a <Context.Consumer>

if (!isValid && !didWarnAboutInvalidateContextType.has(ctor)) {
didWarnAboutInvalidateContextType.add(ctor);

let addendum = '';
if (contextType === undefined) {
addendum =
' However, it is set to undefined. ' +
'This can be caused by a typo or by mixing up named and default imports. ' +
'This can also happen due to a circular dependency, so ' +
'try moving the createContext() call to a separate file.';
} else if (typeof contextType !== 'object') {
addendum = ' However, it is set to a ' + typeof contextType + '.';
} else if (contextType.$$typeof === REACT_PROVIDER_TYPE) {
addendum = ' Did you accidentally pass the Context.Provider instead?';
} else if (contextType._context !== undefined) {
// <Context.Consumer>
addendum = ' Did you accidentally pass the Context.Consumer instead?';
} else {
addendum =
' However, it is set to an object with keys {' +
Object.keys(contextType).join(', ') +
'}.';
}
warningWithoutStack(
false,
'%s defines an invalid contextType. ' +
'contextType should point to the Context object returned by React.createContext(). ' +
'Did you accidentally pass the Context.%s instead?',
'contextType should point to the Context object returned by React.createContext().%s',
getComponentName(ctor) || 'Component',
isContextConsumer ? 'Consumer' : 'Provider',
addendum,
);
}
}
}

if (typeof contextType === 'object' && contextType !== null) {
context = readContext((contextType: any));
} else {
unmaskedContext = getUnmaskedContext(workInProgress, ctor, true);
Expand Down
81 changes: 81 additions & 0 deletions packages/react/src/__tests__/ReactContextValidator-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,87 @@ describe('ReactContextValidator', () => {
);
});

it('should not warn when class contextType is null', () => {
class Foo extends React.Component {
static contextType = null; // Handy for conditional declaration
render() {
return this.context.hello.world;
}
}
expect(() => {
ReactTestUtils.renderIntoDocument(<Foo />);
}).toThrow("Cannot read property 'world' of undefined");
});

it('should warn when class contextType is undefined', () => {
class Foo extends React.Component {
// This commonly happens with circular deps
// https://github.com/facebook/react/issues/13969
static contextType = undefined;
render() {
return this.context.hello.world;
}
}

expect(() => {
expect(() => {
ReactTestUtils.renderIntoDocument(<Foo />);
}).toThrow("Cannot read property 'world' of undefined");
}).toWarnDev(
'Foo defines an invalid contextType. ' +
'contextType should point to the Context object returned by React.createContext(). ' +
'However, it is set to undefined. ' +
'This can be caused by a typo or by mixing up named and default imports. ' +
'This can also happen due to a circular dependency, ' +
'so try moving the createContext() call to a separate file.',
{withoutStack: true},
);
});

it('should warn when class contextType is an object', () => {
class Foo extends React.Component {
// Can happen due to a typo
static contextType = {
x: 42,
y: 'hello',
};
render() {
return this.context.hello.world;
}
}

expect(() => {
expect(() => {
ReactTestUtils.renderIntoDocument(<Foo />);
}).toThrow("Cannot read property 'hello' of undefined");
}).toWarnDev(
'Foo defines an invalid contextType. ' +
'contextType should point to the Context object returned by React.createContext(). ' +
'However, it is set to an object with keys {x, y}.',
{withoutStack: true},
);
});

it('should warn when class contextType is a primitive', () => {
class Foo extends React.Component {
static contextType = 'foo';
render() {
return this.context.hello.world;
}
}

expect(() => {
expect(() => {
ReactTestUtils.renderIntoDocument(<Foo />);
}).toThrow("Cannot read property 'world' of undefined");
}).toWarnDev(
'Foo defines an invalid contextType. ' +
'contextType should point to the Context object returned by React.createContext(). ' +
'However, it is set to a string.',
{withoutStack: true},
);
});

it('should warn if you define contextType on a function component', () => {
const Context = React.createContext();

Expand Down

0 comments on commit e0c2c56

Please sign in to comment.