Skip to content

Commit

Permalink
Improve error messages for invalid element types
Browse files Browse the repository at this point in the history
  • Loading branch information
sophiebits committed Dec 21, 2016
1 parent ac24197 commit 9d2d815
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 53 deletions.
2 changes: 1 addition & 1 deletion scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js
* does not warns for iterable elements with keys
* does not warn when the element is directly in rest args
* does not warn when the array contains a non-element
* gives a helpful error when passing null, undefined, boolean, or number
* gives a helpful error when passing invalid types
* should check default prop values
* should not check the default for explicit null
* should check declared prop types
Expand Down
31 changes: 24 additions & 7 deletions src/isomorphic/classic/element/ReactElementValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,30 @@ var ReactElementValidator = {
// We warn in this case but don't throw. We expect the element creation to
// succeed and there will likely be errors in render.
if (!validType) {
warning(
false,
'React.createElement: type should not be null, undefined, boolean, or ' +
'number. It should be a string (for DOM elements) or a ReactClass ' +
'(for composite components).%s',
getDeclarationErrorAddendum()
);
if (
typeof type !== 'function' &&
typeof type !== 'string'
) {
var info = '';
if (
typeof type === 'object' &&
type !== null &&
Object.keys(type).length === 0
) {
info +=
' You likely forgot to export your component from the file ' +
'it\'s defined in.';
}
info += getDeclarationErrorAddendum();
warning(
false,
'React.createElement: type is invalid -- expected a string (for ' +
'built-in components) or a class/function (for composite ' +
'components) but got: %s.%s',
type == null ? type : typeof type,
info,
);
}
}

var element = ReactElement.createElement.apply(this, arguments);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,35 +289,48 @@ describe('ReactElementValidator', () => {
);
});

it('gives a helpful error when passing null, undefined, boolean, or number', () => {
it('gives a helpful error when passing invalid types', () => {
spyOn(console, 'error');
React.createElement(undefined);
React.createElement(null);
React.createElement(true);
React.createElement(123);
expectDev(console.error.calls.count()).toBe(4);
React.createElement({x: 17});
React.createElement({});
expectDev(console.error.calls.count()).toBe(6);
expectDev(console.error.calls.argsFor(0)[0]).toBe(
'Warning: React.createElement: type should not be null, undefined, ' +
'boolean, or number. It should be a string (for DOM elements) or a ' +
'ReactClass (for composite components).'
'Warning: React.createElement: type is invalid -- expected a string ' +
'(for built-in components) or a class/function (for composite ' +
'components) but got: undefined.'
);
expectDev(console.error.calls.argsFor(1)[0]).toBe(
'Warning: React.createElement: type should not be null, undefined, ' +
'boolean, or number. It should be a string (for DOM elements) or a ' +
'ReactClass (for composite components).'
'Warning: React.createElement: type is invalid -- expected a string ' +
'(for built-in components) or a class/function (for composite ' +
'components) but got: null.'
);
expectDev(console.error.calls.argsFor(2)[0]).toBe(
'Warning: React.createElement: type should not be null, undefined, ' +
'boolean, or number. It should be a string (for DOM elements) or a ' +
'ReactClass (for composite components).'
'Warning: React.createElement: type is invalid -- expected a string ' +
'(for built-in components) or a class/function (for composite ' +
'components) but got: boolean.'
);
expectDev(console.error.calls.argsFor(3)[0]).toBe(
'Warning: React.createElement: type should not be null, undefined, ' +
'boolean, or number. It should be a string (for DOM elements) or a ' +
'ReactClass (for composite components).'
'Warning: React.createElement: type is invalid -- expected a string ' +
'(for built-in components) or a class/function (for composite ' +
'components) but got: number.'
);
expectDev(console.error.calls.argsFor(4)[0]).toBe(
'Warning: React.createElement: type is invalid -- expected a string ' +
'(for built-in components) or a class/function (for composite ' +
'components) but got: object.'
);
expectDev(console.error.calls.argsFor(5)[0]).toBe(
'Warning: React.createElement: type is invalid -- expected a string ' +
'(for built-in components) or a class/function (for composite ' +
'components) but got: object. You likely forgot to export your ' +
'component from the file it\'s defined in.'
);
React.createElement('div');
expectDev(console.error.calls.count()).toBe(4);
expectDev(console.error.calls.count()).toBe(6);
});

it('includes the owner name when passing null, undefined, boolean, or number', () => {
Expand All @@ -336,10 +349,9 @@ describe('ReactElementValidator', () => {
);
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toBe(
'Warning: React.createElement: type should not be null, undefined, ' +
'boolean, or number. It should be a string (for DOM elements) or a ' +
'ReactClass (for composite components). Check the render method of ' +
'`ParentComp`.'
'Warning: React.createElement: type is invalid -- expected a string ' +
'(for built-in components) or a class/function (for composite ' +
'components) but got: null. Check the render method of `ParentComp`.'
);
});

Expand Down Expand Up @@ -537,9 +549,9 @@ describe('ReactElementValidator', () => {
void <Foo>{[<div />]}</Foo>;
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toBe(
'Warning: React.createElement: type should not be null, undefined, ' +
'boolean, or number. It should be a string (for DOM elements) or a ' +
'ReactClass (for composite components).'
'Warning: React.createElement: type is invalid -- expected a string ' +
'(for built-in components) or a class/function (for composite ' +
'components) but got: undefined.'
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,21 +218,25 @@ describe('ReactJSXElementValidator', () => {
void <True />;
void <Num />;
expectDev(console.error.calls.count()).toBe(4);
expectDev(console.error.calls.argsFor(0)[0]).toContain(
'type should not be null, undefined, boolean, or number. It should be ' +
'a string (for DOM elements) or a ReactClass (for composite components).'
expectDev(console.error.calls.argsFor(0)[0]).toBe(
'Warning: React.createElement: type is invalid -- expected a string ' +
'(for built-in components) or a class/function (for composite ' +
'components) but got: undefined.'
);
expectDev(console.error.calls.argsFor(1)[0]).toContain(
'type should not be null, undefined, boolean, or number. It should be ' +
'a string (for DOM elements) or a ReactClass (for composite components).'
expectDev(console.error.calls.argsFor(1)[0]).toBe(
'Warning: React.createElement: type is invalid -- expected a string ' +
'(for built-in components) or a class/function (for composite ' +
'components) but got: null.'
);
expectDev(console.error.calls.argsFor(2)[0]).toContain(
'type should not be null, undefined, boolean, or number. It should be ' +
'a string (for DOM elements) or a ReactClass (for composite components).'
expectDev(console.error.calls.argsFor(2)[0]).toBe(
'Warning: React.createElement: type is invalid -- expected a string ' +
'(for built-in components) or a class/function (for composite ' +
'components) but got: boolean.'
);
expectDev(console.error.calls.argsFor(3)[0]).toContain(
'type should not be null, undefined, boolean, or number. It should be ' +
'a string (for DOM elements) or a ReactClass (for composite components).'
expectDev(console.error.calls.argsFor(3)[0]).toBe(
'Warning: React.createElement: type is invalid -- expected a string ' +
'(for built-in components) or a class/function (for composite ' +
'components) but got: number.'
);
void <Div />;
expectDev(console.error.calls.count()).toBe(4);
Expand Down
23 changes: 20 additions & 3 deletions src/renderers/shared/fiber/ReactFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,11 @@ function createFiberFromElementType(type : mixed, key : null | string) : Fiber {
} else if (typeof type === 'string') {
fiber = createFiber(HostComponent, key);
fiber.type = type;
} else if (typeof type === 'object' && type !== null) {
} else if (
typeof type === 'object' &&
type !== null &&
typeof type.tag === 'number'
) {
// Currently assumed to be a continuation and therefore is a fiber already.
// TODO: The yield system is currently broken for updates in some cases.
// The reified yield stores a fiber, but we don't know which fiber that is;
Expand All @@ -317,12 +321,25 @@ function createFiberFromElementType(type : mixed, key : null | string) : Fiber {
// There is probably a clever way to restructure this.
fiber = ((type : any) : Fiber);
} else {
let info = '';
if (__DEV__) {
if (
typeof type === 'object' &&
type !== null &&
Object.keys(type).length === 0
) {
info +=
' You likely forgot to export your component from the file ' +
'it\'s defined in.';
}
}
// TODO: Stack also includes owner name in the message.
invariant(
false,
'Element type is invalid: expected a string (for built-in components) ' +
'or a class/function (for composite components) but got: %s.',
'or a class/function (for composite components) but got: %s.%s',
type == null ? type : typeof type,
// TODO: Stack also includes owner name in the message.
info,
);
}
return fiber;
Expand Down
34 changes: 26 additions & 8 deletions src/renderers/shared/stack/reconciler/instantiateReactComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,32 @@ function instantiateReactComponent(node, shouldHaveDebugID) {
instance = ReactEmptyComponent.create(instantiateReactComponent);
} else if (typeof node === 'object') {
var element = node;
invariant(
element && (typeof element.type === 'function' ||
typeof element.type === 'string'),
'Element type is invalid: expected a string (for built-in components) ' +
'or a class/function (for composite components) but got: %s.%s',
element.type == null ? element.type : typeof element.type,
getDeclarationErrorAddendum(element._owner)
);
var type = element.type;
if (
typeof type !== 'function' &&
typeof type !== 'string'
) {
var info = '';
if (__DEV__) {
if (
typeof type === 'object' &&
type !== null &&
Object.keys(type).length === 0
) {
info +=
' You likely forgot to export your component from the file ' +
'it\'s defined in.';
}
}
info += getDeclarationErrorAddendum(element._owner);
invariant(
false,
'Element type is invalid: expected a string (for built-in components) ' +
'or a class/function (for composite components) but got: %s.%s',
type == null ? type : typeof type,
info,
);
}

// Special case string values
if (typeof element.type === 'string') {
Expand Down

0 comments on commit 9d2d815

Please sign in to comment.