From 6c585fd835b10aa379b1fe2b01ac836f39854423 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Wed, 12 Apr 2017 21:11:36 -0700 Subject: [PATCH] Add component stack to invalid-object-child error https://twitter.com/EsaMatti/status/835952325525188608 (28 likes!) --- .../children/traverseAllChildren.js | 27 +++++--------- .../__tests__/ReactComponent-test.js | 35 +++++++++++++++---- .../__tests__/ReactMultiChild-test.js | 7 ++-- src/renderers/shared/fiber/ReactChildFiber.js | 24 ++----------- .../stack/reconciler/traverseStackChildren.js | 27 +++++--------- 5 files changed, 55 insertions(+), 65 deletions(-) diff --git a/src/isomorphic/children/traverseAllChildren.js b/src/isomorphic/children/traverseAllChildren.js index a68d1816d68..4f1692e2773 100644 --- a/src/isomorphic/children/traverseAllChildren.js +++ b/src/isomorphic/children/traverseAllChildren.js @@ -12,13 +12,18 @@ 'use strict'; var REACT_ELEMENT_TYPE = require('ReactElementSymbol'); -var ReactCurrentOwner = require('ReactCurrentOwner'); var getIteratorFn = require('getIteratorFn'); var invariant = require('fbjs/lib/invariant'); var KeyEscapeUtils = require('KeyEscapeUtils'); var warning = require('fbjs/lib/warning'); +if (__DEV__) { + var { + getCurrentStackAddendum, + } = require('ReactComponentTreeHook'); +} + var SEPARATOR = '.'; var SUBSEPARATOR = ':'; @@ -114,21 +119,12 @@ function traverseAllChildrenImpl( if (__DEV__) { // Warn about using Maps as children if (iteratorFn === children.entries) { - let mapsAsChildrenAddendum = ''; - if (ReactCurrentOwner.current) { - var mapsAsChildrenOwnerName = ReactCurrentOwner.current.getName(); - if (mapsAsChildrenOwnerName) { - mapsAsChildrenAddendum = '\n\nCheck the render method of `' + - mapsAsChildrenOwnerName + - '`.'; - } - } warning( didWarnAboutMaps, 'Using Maps as children is unsupported and will likely yield ' + 'unexpected results. Convert it to a sequence/iterable of keyed ' + 'ReactElements instead.%s', - mapsAsChildrenAddendum, + getCurrentStackAddendum(), ); didWarnAboutMaps = true; } @@ -151,13 +147,8 @@ function traverseAllChildrenImpl( var addendum = ''; if (__DEV__) { addendum = ' If you meant to render a collection of children, use an array ' + - 'instead.'; - if (ReactCurrentOwner.current) { - var name = ReactCurrentOwner.current.getName(); - if (name) { - addendum += '\n\nCheck the render method of `' + name + '`.'; - } - } + 'instead.' + + getCurrentStackAddendum(); } var childrenString = '' + children; invariant( diff --git a/src/renderers/__tests__/ReactComponent-test.js b/src/renderers/__tests__/ReactComponent-test.js index 50dcec84efa..aaba580bde8 100644 --- a/src/renderers/__tests__/ReactComponent-test.js +++ b/src/renderers/__tests__/ReactComponent-test.js @@ -17,6 +17,10 @@ var ReactDOMFeatureFlags; var ReactTestUtils; describe('ReactComponent', () => { + function normalizeCodeLocInfo(str) { + return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); + } + beforeEach(() => { React = require('react'); ReactDOM = require('react-dom'); @@ -389,10 +393,19 @@ describe('ReactComponent', () => { }; var element =
{[children]}
; var container = document.createElement('div'); - expect(() => ReactDOM.render(element, container)).toThrowError( + var ex; + try { + ReactDOM.render(element, container); + } catch (e) { + ex = e; + } + expect(ex).toBeDefined(); + expect(normalizeCodeLocInfo(ex.message)).toBe( 'Objects are not valid as a React child (found: object with keys ' + - '{x, y, z}). If you meant to render a collection of children, use an ' + - 'array instead.', + '{x, y, z}). If you meant to render a collection of children, use ' + + 'an array instead.' + + // Fiber gives a slightly better stack with the nearest host components + (ReactDOMFeatureFlags.useFiber ? '\n in div (at **)' : ''), ); }); @@ -408,10 +421,20 @@ describe('ReactComponent', () => { } } var container = document.createElement('div'); - expect(() => ReactDOM.render(, container)).toThrowError( + var ex; + try { + ReactDOM.render(, container); + } catch (e) { + ex = e; + } + expect(ex).toBeDefined(); + expect(normalizeCodeLocInfo(ex.message)).toBe( 'Objects are not valid as a React child (found: object with keys ' + - '{a, b, c}). If you meant to render a collection of children, use an ' + - 'array instead.\n\nCheck the render method of `Foo`.', + '{a, b, c}). If you meant to render a collection of children, use ' + + 'an array instead.\n' + + // Fiber gives a slightly better stack with the nearest host components + (ReactDOMFeatureFlags.useFiber ? ' in div (at **)\n' : '') + + ' in Foo (at **)', ); }); }); diff --git a/src/renderers/__tests__/ReactMultiChild-test.js b/src/renderers/__tests__/ReactMultiChild-test.js index d3c408f7ac2..f8262f8b8a1 100644 --- a/src/renderers/__tests__/ReactMultiChild-test.js +++ b/src/renderers/__tests__/ReactMultiChild-test.js @@ -274,10 +274,13 @@ describe('ReactMultiChild', () => { var container = document.createElement('div'); ReactDOM.render(, container); expectDev(console.error.calls.count()).toBe(1); - expectDev(console.error.calls.argsFor(0)[0]).toBe( + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( 'Warning: Using Maps as children is unsupported and will likely yield ' + 'unexpected results. Convert it to a sequence/iterable of keyed ' + - 'ReactElements instead.\n\nCheck the render method of `Parent`.', + 'ReactElements instead.\n' + + // Fiber gives a slightly better stack with the nearest host components + (ReactDOMFeatureFlags.useFiber ? ' in div (at **)\n' : '') + + ' in Parent (at **)', ); }); diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index ac6eab0138d..dc93dfbae4e 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -36,11 +36,9 @@ var emptyObject = require('fbjs/lib/emptyObject'); var getIteratorFn = require('getIteratorFn'); var invariant = require('fbjs/lib/invariant'); var ReactFeatureFlags = require('ReactFeatureFlags'); -var {ReactCurrentOwner} = require('ReactGlobalSharedState'); if (__DEV__) { var {getCurrentFiberStackAddendum} = require('ReactDebugCurrentFiber'); - var getComponentName = require('getComponentName'); var warning = require('fbjs/lib/warning'); var didWarnAboutMaps = false; } @@ -127,14 +125,8 @@ function throwOnInvalidObjectType(returnFiber: Fiber, newChild: Object) { let addendum = ''; if (__DEV__) { addendum = ' If you meant to render a collection of children, use an array ' + - 'instead.'; - const owner = ReactCurrentOwner.owner || returnFiber._debugOwner; - if (owner && typeof owner.tag === 'number') { - const name = getComponentName((owner: any)); - if (name) { - addendum += '\n\nCheck the render method of `' + name + '`.'; - } - } + 'instead.' + + (getCurrentFiberStackAddendum() || ''); } invariant( false, @@ -813,22 +805,12 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { if (typeof newChildrenIterable.entries === 'function') { const possibleMap = (newChildrenIterable: any); if (possibleMap.entries === iteratorFn) { - let mapsAsChildrenAddendum = ''; - const owner = ReactCurrentOwner.owner || returnFiber._debugOwner; - if (owner && typeof owner.tag === 'number') { - const mapsAsChildrenOwnerName = getComponentName((owner: any)); - if (mapsAsChildrenOwnerName) { - mapsAsChildrenAddendum = '\n\nCheck the render method of `' + - mapsAsChildrenOwnerName + - '`.'; - } - } warning( didWarnAboutMaps, 'Using Maps as children is unsupported and will likely yield ' + 'unexpected results. Convert it to a sequence/iterable of keyed ' + 'ReactElements instead.%s', - mapsAsChildrenAddendum, + getCurrentFiberStackAddendum(), ); didWarnAboutMaps = true; } diff --git a/src/renderers/shared/stack/reconciler/traverseStackChildren.js b/src/renderers/shared/stack/reconciler/traverseStackChildren.js index b717ea28fbc..3bf7550ae38 100644 --- a/src/renderers/shared/stack/reconciler/traverseStackChildren.js +++ b/src/renderers/shared/stack/reconciler/traverseStackChildren.js @@ -12,13 +12,18 @@ 'use strict'; var REACT_ELEMENT_TYPE = require('ReactElementSymbol'); -var {ReactCurrentOwner} = require('ReactGlobalSharedState'); var getIteratorFn = require('getIteratorFn'); var invariant = require('fbjs/lib/invariant'); var KeyEscapeUtils = require('KeyEscapeUtils'); var warning = require('fbjs/lib/warning'); +if (__DEV__) { + var { + getCurrentStackAddendum, + } = require('ReactGlobalSharedState').ReactComponentTreeHook; +} + var SEPARATOR = '.'; var SUBSEPARATOR = ':'; @@ -114,21 +119,12 @@ function traverseStackChildrenImpl( if (__DEV__) { // Warn about using Maps as children if (iteratorFn === children.entries) { - let mapsAsChildrenAddendum = ''; - if (ReactCurrentOwner.current) { - var mapsAsChildrenOwnerName = ReactCurrentOwner.current.getName(); - if (mapsAsChildrenOwnerName) { - mapsAsChildrenAddendum = '\n\nCheck the render method of `' + - mapsAsChildrenOwnerName + - '`.'; - } - } warning( didWarnAboutMaps, 'Using Maps as children is unsupported and will likely yield ' + 'unexpected results. Convert it to a sequence/iterable of keyed ' + 'ReactElements instead.%s', - mapsAsChildrenAddendum, + getCurrentStackAddendum(), ); didWarnAboutMaps = true; } @@ -151,13 +147,8 @@ function traverseStackChildrenImpl( var addendum = ''; if (__DEV__) { addendum = ' If you meant to render a collection of children, use an array ' + - 'instead.'; - if (ReactCurrentOwner.current) { - var name = ReactCurrentOwner.current.getName(); - if (name) { - addendum += '\n\nCheck the render method of `' + name + '`.'; - } - } + 'instead.' + + getCurrentStackAddendum(); } var childrenString = '' + children; invariant(