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

Make createElement(undefined) warning more descriptive #7307

Closed
gaearon opened this issue Jul 19, 2016 · 24 comments
Closed

Make createElement(undefined) warning more descriptive #7307

gaearon opened this issue Jul 19, 2016 · 24 comments

Comments

@gaearon
Copy link
Collaborator

gaearon commented Jul 19, 2016

Update: claimed by @jin

This is what I see when I mistype an import:

screen shot 2016-07-19 at 21 12 01

This is not very useful.

When type is undefined, we should provide a better message. In 95% of cases it is caused by an invalid import. We should create a page explaining common cases how this could happen (e.g. mismatching default/named export, forgetting to export the component, or importing a non-existing named export), and link to that page from the warning.

@gaearon
Copy link
Collaborator Author

gaearon commented Jul 19, 2016

If you intend to work on this, please write in a comment here. The change to the code is very small so most of the effort would be spent on creating a good warning page explaining common problems and solutions.

Here is an example of such warning page: https://github.com/facebook/react/blob/master/docs/warnings/legacy-factories.md. This one would be similar (but about a different topic).

@jin
Copy link

jin commented Jul 19, 2016

Can I look into this?

@gaearon
Copy link
Collaborator Author

gaearon commented Jul 19, 2016

Sure!

@gaearon gaearon self-assigned this Jul 19, 2016
@gaearon
Copy link
Collaborator Author

gaearon commented Jul 19, 2016

I’m “assigning” myself so I keep track of this issue but it’s yours.

@Pajn
Copy link

Pajn commented Jul 19, 2016

Please include a stacktrace!
This is the worst possible message to get after doing a big merge as there are absolutely nothing to guide you to where the problem can be.

@jin
Copy link

jin commented Jul 19, 2016

@Pajn Will do! I'm bitten by the lack of information many times too, especially when dealing with HOC.

Edit 7/26: didn't manage to find time last week to work on this, but will be doing that this week

@zpao
Copy link
Member

zpao commented Jul 19, 2016

Similar: #7215.

@gaearon
Copy link
Collaborator Author

gaearon commented Aug 1, 2016

@jin Any updates? If you’re too busy, it’s fine—just let us know and we’ll ask somebody else to look into this!

@jin
Copy link

jin commented Aug 1, 2016

@gaearon Really sorry for losing track of this - please free this up for someone else. Took me a little too long to digest the React source.

@gaearon
Copy link
Collaborator Author

gaearon commented Aug 1, 2016

@jin Anything we can help you with? This should only involve changing these few lines. Also would love to hear your feedback on the confusing parts of the codebase.

@alexzherdev
Copy link
Contributor

I can pick this up if you guys decide this needs a fresh pair of eyes.

@jin
Copy link

jin commented Aug 1, 2016

@gaearon Actually, I'm done with changing that warning to link to a warning docs page. Right now, I'm primarily blocked with coming up with the different examples/scenarios that the issue can occur in, on top of the ones you've mentioned.

The codebase isn't confusing per se, it's the part of transforming my understanding from actual applications of React to the underlying implementations of them.

@jin
Copy link

jin commented Aug 2, 2016

@gaearon done! Happy to hear any feedback.

@Wilfred
Copy link

Wilfred commented Aug 12, 2016

As discussed in #7485, I think we should also report /what/ value we saw, rather than just a list of forbidden types. Compare

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).

with:

Warning: React.createElement: 5 is an invalid value for type. It should be a string (for DOM elements) or a ReactClass (for composite components).

@jin
Copy link

jin commented Aug 13, 2016

Hi all, really really sorry for the delayed response: I'm moving between cities these couple of weeks, haven't had the chance to sit down to address the feedback yet.

I expect the move to settle down by the middle of next week, and will resume progress ASAP.

On Aug 13, 2016, at 3:10 AM, Wilfred Hughes notifications@github.com wrote:

As discussed in #7485, I think we should also report /what/ value we saw, rather than just a list of forbidden types. Compare

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).

with:

Warning: React.createElement: 5 is an invalid value for type. It should be a string (for DOM elements) or a ReactClass (for composite components).


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@Slruh
Copy link

Slruh commented Sep 5, 2016

@jin, have you made any progress on this issue? If you are too busy, I would love to take a stab at it.

@jin
Copy link

jin commented Sep 5, 2016

@Slruh Yep! There's some progress here: #7402

I'm in the midst of addressing @gaearon's comments, and feel free to suggest any improvements!

@Slruh
Copy link

Slruh commented Sep 5, 2016

I'll take a look

@gravitypersists
Copy link

gravitypersists commented Oct 11, 2016

For me, this error is almost always a chore in finding the component that it's referring to. It'd help if the error message did better to identify that component. Maybe it'd help to display everything known about the element? Such as props, or the location of the element in the component hierarchy (perhaps a parent)?

Warning: React.createElement: undefined is an invalid value for type. It should be a string (for DOM elements) or a ReactClass (for composite components).

<App>
    <Rooter>
        <div>
            <undefined>
             ^^^^^^^^^

Is something like that feasible and desirable?

@syranide
Copy link
Contributor

@gravitypersists It just needs to explicitly say that it is undefined (rather than some obscure error) when React.createElement is called.

@joelseq
Copy link
Contributor

joelseq commented Dec 5, 2016

Is this still being worked on or can I maybe try to take a stab at it?

@jin
Copy link

jin commented Dec 5, 2016 via email

@churchie317
Copy link
Contributor

Looks like there hasn't been any activity for a few weeks. Anyone mind if I claim this for now?

@gaearon
Copy link
Collaborator Author

gaearon commented Dec 28, 2016

I think this was fixed in #8612, and being further improved in #8495.
Thanks for the offer to help though!

@gaearon gaearon closed this as completed Dec 28, 2016
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