Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fiber] Include owner in invalid element type invariant #8637

Merged
merged 1 commit into from
Dec 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 0 additions & 6 deletions scripts/fiber/tests-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ src/addons/__tests__/ReactFragment-test.js
src/isomorphic/classic/__tests__/ReactContextValidator-test.js
* should pass previous context to lifecycles

src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js
* includes the owner name when passing null, undefined, boolean, or number

src/renderers/dom/__tests__/ReactDOMProduction-test.js
* should throw with an error code in production

Expand Down Expand Up @@ -63,9 +60,6 @@ src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js
src/renderers/shared/hooks/__tests__/ReactHostOperationHistoryHook-test.js
* gets recorded during an update

src/renderers/shared/shared/__tests__/ReactComponent-test.js
* includes owner name in the error about badly-typed elements

src/renderers/shared/shared/__tests__/ReactComponentLifeCycle-test.js
* should carry through each of the phases of setup

Expand Down
2 changes: 2 additions & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js
* 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 invalid types
* includes the owner name when passing null, undefined, boolean, or number
* should check default prop values
* should not check the default for explicit null
* should check declared prop types
Expand Down Expand Up @@ -1334,6 +1335,7 @@ src/renderers/shared/shared/__tests__/ReactComponent-test.js
* should call refs at the correct time
* fires the callback after a component is rendered
* throws usefully when rendering badly-typed elements
* includes owner name in the error about badly-typed elements

src/renderers/shared/shared/__tests__/ReactComponentLifeCycle-test.js
* should not reuse an instance when it has been unmounted
Expand Down
21 changes: 18 additions & 3 deletions src/renderers/shared/fiber/ReactDebugCurrentFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@
import type { Fiber } from 'ReactFiber';

if (__DEV__) {
var {
IndeterminateComponent,
FunctionalComponent,
ClassComponent,
HostComponent,
} = require('ReactTypeOfWork');
var getComponentName = require('getComponentName');
var { getStackAddendumByWorkInProgressFiber } = require('ReactComponentTreeHook');
}
Expand All @@ -25,10 +31,19 @@ function getCurrentFiberOwnerName() : string | null {
if (fiber == null) {
return null;
}
if (fiber._debugOwner == null) {
return null;
switch (fiber.tag) {
case IndeterminateComponent:
case FunctionalComponent:
case ClassComponent:
return getComponentName(fiber);
case HostComponent:
if (fiber._debugOwner != null) {
return getComponentName(fiber._debugOwner);
}
return null;
default:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will the other tag types ever have a component name to return?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe coroutines but we haven't implemented them fully yet. I'm aiming for parity for now.

return null;
}
return getComponentName(fiber._debugOwner);
}
return null;
}
Expand Down
9 changes: 8 additions & 1 deletion src/renderers/shared/fiber/ReactFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ var {

var invariant = require('invariant');

if (__DEV__) {
var { getCurrentFiberOwnerName } = require('ReactDebugCurrentFiber');
}

// A Fiber is work on a Component that needs to be done or was done. There can
// be more than one per component.
export type Fiber = {
Expand Down Expand Up @@ -359,8 +363,11 @@ function createFiberFromElementType(type : mixed, key : null | string) : Fiber {
' You likely forgot to export your component from the file ' +
'it\'s defined in.';
}
const ownerName = getCurrentFiberOwnerName();
if (ownerName) {
info += ' Check the render method of `' + ownerName + '`.';
}
}
// TODO: Stack also includes owner name in the message.
invariant(
false,
'Element type is invalid: expected a string (for built-in components) ' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,7 @@ describe('ReactIncrementalErrorHandling', () => {
'Element 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.'
'defined in. Check the render method of `BrokenRender`.'
)]);
expect(console.error.calls.count()).toBe(1);
});
Expand Down Expand Up @@ -798,7 +798,7 @@ describe('ReactIncrementalErrorHandling', () => {
'Element 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.'
'defined in. Check the render method of `BrokenRender`.'
)]);
expect(console.error.calls.count()).toBe(1);
});
Expand Down