Clearer ssr error message 11902

[HeroProtagonist]

Resolves #11902

Only in development was a warning being thrown about ReactElements with non string type. Nothing stopped the code from continuing to execute calling the string method on invalid types.

I added a check in ReactPartialRenderer.js that throws an error before toLowerCase is called. Also a simple test of this new behavior.

[gaearon]

Not necessarily. This function expected a string. But that's just because we let the code go far enough to go into this function. From user's point of view, they might expect it to be a custom component but they messed up the import and got undefined instead.

We should check earlier and show a full message, with the same wording as on the client. We should also add a test case to ReactDOMIntegrationElement-test similar to existing itThrowsWhenRendering calls in it. This way we verify the behavior is identical on the client and on the server.

[HeroProtagonist]

@gaearon should this then be an invariant and have a shorter version of this error thrown when NODE_ENV=production? Otherwise, production would have the toLowerCase error displayed

[gaearon]

No, what I'm saying is we should check if it's a string just before entering renderDOM. If it's not a string then we should throw.

I guess technically it means the check could be inside renderDOM but I still find the flow to be more confusing since it depends on renderDOM call being the last branch. Moving the check out would make more sense to me.

[HeroProtagonist]

Ah I see in render which in turn calls renderDOM has lots of checks in it. I will update PR soon!

[HeroProtagonist]

This issue has been addressed, but I updated the message to match the message given here

[gaearon]

Please add another test case for undefined type.

[gaearon]

This line will crash if elementType is null or undefined.

[HeroProtagonist]

@gaearon Added code and tests for the null/undefined case 👍

[gaearon]

Hmm wait. I don't see how this could work. Owner is defined on the element. Not on the elementType.

[gaearon]

In other words you want nextElement._owner here. We know nextElement exists because we already read type from it.

[HeroProtagonist]

I got confused on that element check :(. Fixed that and updated the tests to check for the warning that createElement has on dev.

[gaearon]

Lets remove this beforeEach and createElement override

[gaearon]

Why are we doing this here instead of wrapping <UndefinedComponent /> JSX call where it happens?

let el;
expect(() => {
  el = <UndefinedComponent />;
}).toWarnInDev(...)
await render(el);

[HeroProtagonist]

Didn't make the connection that I could do this. Thanks, this is so much easier than overriding the function. Removed the beforeEach and added this to the tests

outdated

[gaearon]

Looking good to me. Thank you so much!

[timneutkens]

@HeroProtagonist @gaearon thanks, we were running into this with Next 🙏 ❤️