Bug: "The above error…" should appear after the error message but appears before (for some errors)

[sophiebits]

React version: both latest (17.0.2) and next (18.0.0-alpha-9c8161ba8-20211028)

Steps To Reproduce

1. Render <input>hello</input> to trigger the "input is a void element tag" error

Link to code example: https://codesandbox.io/s/cocky-matan-ydmys

The current behavior

Two log lines appear:

1. "The above error occurred in the <input> component"
2. "input is a void element tag"

The expected behavior

They should be in the other order:

1. "input is a void element tag"
2. "The above error occurred in the <input> component"

This problem doesn't occur in the same way if a component throws an error. Presumably this goes through a different error handling path because it comes from the host config.

If someone works on this, you may also want to review look at this issue at the same time:

• Arbitrary log lines may appear in between an error and the subsequent "The above error…" message #18101

[aniler0]

The input tag should not have any children. Instead of this you should use input and label tag if you want to write text out of input area.

[sophiebits]

Hi @aniler0 – I'm aware. :) But when someone makes this mistake, the reporting of the error should be a little different than it is today, and that's what this bug is about.

[Berci-Ionut]

When using style as a string instead of an object, the error doesn't show at all. Is only this message:

(this is caused by something like <div style="position: absolute" />. I am also aware this is wrong, but I really think that react should report a proper error for this to give you a hint why the application is crashing). Thank you!

[sophiebits]

@Berci-Ionut I can't reproduce that. For me it does show the error:

If you can reproduce this in a standalone code sandbox, do feel free to link it here though.

[Berci-Ionut]

Hmm... Seems like is only reproducing when using NextJS (example here) . In this case I am not sure if the issue is related to react or rather NextJS
EDIT: it seems like a forked nextJS link is failing on sandbox for some reason (at least for me). But if you create a new NextJS sandbox and paste the code it will be reproduced.

[sophiebits]

Still relevant (just verified in 18.0.0 stable) – this and #18101 are both interesting IMO.

[girishsontakke]

Hi @sophiebits, I would like to work on this issue.

[gaearon]

Sure, please feel free to. The first step would be to investigate why this is happening and post a comment here explaining the difference in behavior. Then we can look at the implementation.

[sylvesterAdewale]

The input tag should be a self closing tag

[gaearon]

@sylvesterAdewale This is not what the issue is about. The issue is about the ordering of the messages. @sophiebits already responded to this in #22656 (comment).

[MustafaEminn]

Oops. I have worked for a while on the issue. I guess I understand why messages are ordering like that. But I didn't know issues can be take.

[girishsontakke]

Hi @MustafaEminn, If you already have worked on this issue then you can surely write a patch for this, I will try to learn from your patch.

[MustafaEminn]

We can't run console methods after throwing the error. "The above error occurred..." error is logging by console.error method and the second error "... is a void element..." is throw a new error. If I want to tell in order.
assertValidProps method throw an error --> renderRootSync catch the error --> commitUpdateQueue call callCallback method and print the first error using console.error. Also, this callback set hasUncaughtError variable to true. --> commitRootImpl works again and throws an error which is "... is a void element..." error.

What is the solution?
I guess changing the above word to below will be good. Because we can't run console methods after throwing the error. I already changed and tested with yarn test. There is no failure.

If we want to log both of these errors, we can't interrupt the execution without throwing another error.

If seems good I can open a pull request.

[gaearon]

I don’t think it sounds right to me that it’s not possible to fix this without changing the message. Why can’t we delay the console.error call until we know for sure the error has been logged? We could change where we log it from the updater callback to some other place.

[MustafaEminn]

We can delay the console.error with setTimeout but it does not seem an efficient way. If we log the error with console.error instead of throwing a new Error then we can change the order without setTimeout. Because we should log the first "is a void element" message and If we throw an error instantly (assume renderRootSync does not catch the error), execution will be interrupted. That's why I can't manage the second log order. Is interrupting the execution important? If we log the first error with console.error and the second error throw an error, handleError method catches the error and the second error won't be logged.

There is good execution. If we change the code structure and log error with console.error instead of throwing a new Error, this doesn't look good. Maybe there is another way to do it but I'm new and I keep learning the React codebase 😊