From 18467082c0922ecd73651f1668299f71b341ed68 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Tue, 20 Dec 2016 19:42:48 -0800 Subject: [PATCH] Improve error messages for invalid element types cc @mkonicek https://twitter.com/martinkonicek/status/810917317639110661 --- scripts/fiber/tests-passing.txt | 2 +- .../classic/element/ReactElementValidator.js | 32 +++++++--- .../__tests__/ReactElementValidator-test.js | 58 ++++++++++++------- .../ReactJSXElementValidator-test.js | 29 ++++++---- src/renderers/shared/fiber/ReactFiber.js | 24 +++++++- .../ReactIncrementalErrorHandling-test.js | 12 +++- .../shared/__tests__/ReactComponent-test.js | 7 ++- .../reconciler/instantiateReactComponent.js | 35 ++++++++--- 8 files changed, 141 insertions(+), 58 deletions(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 9a72e473b6aad..2ab3530ca0cb8 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -286,7 +286,7 @@ src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js * does not warn when the element is directly in rest args * does not warn when the array contains a non-element * should give context for PropType errors in nested components. -* 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 diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js index 477c814d2f0c4..ef024c7bf9811 100644 --- a/src/isomorphic/classic/element/ReactElementValidator.js +++ b/src/isomorphic/classic/element/ReactElementValidator.js @@ -191,13 +191,31 @@ 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 ( + type === undefined || + 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); diff --git a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js index 6783f762697fc..af5b2ca3909e1 100644 --- a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js +++ b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js @@ -286,35 +286,49 @@ 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. You likely forgot to export your ' + + 'component from the file it\'s defined in.' ); 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', () => { @@ -333,10 +347,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`.' ); }); @@ -534,9 +547,10 @@ describe('ReactElementValidator', () => { void {[
]}; 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. You likely forgot to export your ' + + 'component from the file it\'s defined in.' ); }); diff --git a/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js b/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js index 21d9999ebb557..a3698414da3cb 100644 --- a/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js +++ b/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js @@ -256,21 +256,26 @@ describe('ReactJSXElementValidator', () => { void ; void ; 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. You likely forgot to export your ' + + 'component from the file it\'s defined in.' ); - 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
; expectDev(console.error.calls.count()).toBe(4); diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 3b9a88fa88faf..26c8efa50700f 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -334,7 +334,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; @@ -343,12 +347,26 @@ 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 ( + type === undefined || + 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; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js index 879ebad7e6fcd..2a922286ccdd0 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js @@ -752,7 +752,9 @@ describe('ReactIncrementalErrorHandling', () => { ReactNoop.flush(); expect(ReactNoop.getChildren()).toEqual([span( 'Element type is invalid: expected a string (for built-in components) or ' + - 'a class/function (for composite components) but got: undefined.' + 'a class/function (for composite components) but got: undefined. ' + + 'You likely forgot to export your component from the file it\'s ' + + 'defined in.' )]); expect(console.error.calls.count()).toBe(1); }); @@ -794,7 +796,9 @@ describe('ReactIncrementalErrorHandling', () => { ReactNoop.flush(); expect(ReactNoop.getChildren()).toEqual([span( 'Element type is invalid: expected a string (for built-in components) or ' + - 'a class/function (for composite components) but got: undefined.' + 'a class/function (for composite components) but got: undefined. ' + + 'You likely forgot to export your component from the file it\'s ' + + 'defined in.' )]); expect(console.error.calls.count()).toBe(1); }); @@ -807,7 +811,9 @@ describe('ReactIncrementalErrorHandling', () => { ReactNoop.flush(); }).toThrowError( 'Element type is invalid: expected a string (for built-in components) or ' + - 'a class/function (for composite components) but got: undefined.' + 'a class/function (for composite components) but got: undefined. ' + + 'You likely forgot to export your component from the file it\'s ' + + 'defined in.' ); ReactNoop.render(); diff --git a/src/renderers/shared/shared/__tests__/ReactComponent-test.js b/src/renderers/shared/shared/__tests__/ReactComponent-test.js index f3c1e14f0f47a..1195070857744 100644 --- a/src/renderers/shared/shared/__tests__/ReactComponent-test.js +++ b/src/renderers/shared/shared/__tests__/ReactComponent-test.js @@ -334,7 +334,9 @@ describe('ReactComponent', () => { var X = undefined; expect(() => ReactTestUtils.renderIntoDocument()).toThrowError( 'Element type is invalid: expected a string (for built-in components) ' + - 'or a class/function (for composite components) but got: undefined.' + 'or a class/function (for composite components) but got: undefined. ' + + 'You likely forgot to export your component from the file it\'s ' + + 'defined in.' ); var Y = null; @@ -358,7 +360,8 @@ describe('ReactComponent', () => { expect(() => ReactTestUtils.renderIntoDocument()).toThrowError( 'Element type is invalid: expected a string (for built-in components) ' + 'or a class/function (for composite components) but got: undefined. ' + - 'Check the render method of `Foo`.' + 'You likely forgot to export your component from the file it\'s ' + + 'defined in. Check the render method of `Foo`.' ); // One warning for each element creation diff --git a/src/renderers/shared/stack/reconciler/instantiateReactComponent.js b/src/renderers/shared/stack/reconciler/instantiateReactComponent.js index 202768d8bba18..d1b5c23262c6a 100644 --- a/src/renderers/shared/stack/reconciler/instantiateReactComponent.js +++ b/src/renderers/shared/stack/reconciler/instantiateReactComponent.js @@ -72,14 +72,33 @@ 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 ( + type === undefined || + 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') {