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

Is there a way to let users know which invalid type was returned? #7215

Closed
kevinSuttle opened this issue Jul 7, 2016 · 9 comments
Closed

Comments

@kevinSuttle
Copy link
Contributor

kevinSuttle commented Jul 7, 2016

This is the error message in question:

LabelButton(...): A valid React element (or null) must be returned. 
You may have returned undefined, an array or some other invalid object.

Not very helpful. Is it technically possible to show what was returned?

@syranide
Copy link
Contributor

syranide commented Jul 7, 2016

@kevinSuttle To some extent yes, but if you look at your code I imagine it would be quite obvious where it went wrong. The render function should be rather predictable and simple, especially when it comes to the root.

@gaearon
Copy link
Collaborator

gaearon commented Jul 7, 2016

I think we’re open to fixing it to be more specific. Would you like to make a PR?

@kevinSuttle
Copy link
Contributor Author

Where would be a good place to start?

@zpao
Copy link
Member

zpao commented Jul 8, 2016

I would search for that message (or part of it) and look at the conditions that lead to it (I think all in ReactCompositeComponent.js). It might fire in a couple places. We could probably add a typeof in there (not perfect but it's more). Could potentially do an Object.keys when it is an object.

It might help to understand the typical cases leading up to this warning so we can lead people in the right direction instead of just showing more info. Thoughts?

@kevinSuttle
Copy link
Contributor Author

kevinSuttle commented Jul 8, 2016

Yeah, I usually run into this when I do things like return the wrong type in an HOC (e.g. a props object vs. a React Element) or forget to export a default module.

@Pajn
Copy link

Pajn commented Jul 26, 2016

@syranide I don't think it's "quite obvious" when receiving this after a larger merge.
Just as with the other referenced warning it's for some reason taken for granted that you get this while developing, personally almost all cases I receive this is after a pull of either the project itself or one of our component libraries. It's especially useless when you get the component name connect(Component).
The only real possibility for actually finding the error without manually checking each end every file is to run a TS check but as only about half our code is in TS so there is quite a risk that you will have to spend some time on it.

@kevinSuttle
Copy link
Contributor Author

This is a symptom of a larger issue in approach to error messages. e.g.

Stateless function components cannot have refs. Invariant Violation: Stateless function components cannot have refs.

Ok, so what can they have? Why can't they have refs? Give me a link.

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

@kevinslin The problem with commenting in an unrelated issue is that we never know it was a problem (since we don't always check notifications for known issues). It's not a problem in the overall "approach", it's just a particular warning we missed that should have been more actionable. But since it wasn't reported directly, I only read your comment a year later.

I'll close this issue because React 16 allows more return types and should give better messages with component stack traces.

@gaearon gaearon closed this as completed Oct 4, 2017
@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

To be clear, it is expected that warnings (which fire earlier) might be more actionable than errors (that fire later).

For example, the "cannot have refs" warning includes both component name and stack traces:

ReactTestUtils.renderIntoDocument(<ParentUsingStringRef />);
expectDev(console.error.calls.count()).toBe(1);
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
'Warning: Stateless function components cannot be given refs. ' +
'Attempts to access this ref will fail.\n\nCheck the render method ' +
'of `ParentUsingStringRef`.\n' +
' in StatelessComponent (at **)\n' +
' in div (at **)\n' +
' in Indirection (at **)\n' +
' in ParentUsingStringRef (at **)',
);
ReactTestUtils.renderIntoDocument(<ParentUsingStringRef />);
expectDev(console.error.calls.count()).toBe(1);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants