Warn for invalid type in renderer with the correct RSC stack #30102
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
CLA Signed
React Core Team
This lets us delete more of this code once it lands fully. In particular the hard coded client reference is no longer needed.
We'll handle this in the reconciler instead with the proper stack traces.
This ensures that we transfer the stack/owner from the element to the new Fiber.
We let the client decide if it's valid or not and provide the error message if not.
This gets serialized as a tuple in the Flight protocol so we think it needs a key but it doesn't. It'll error later on the client. Fix test asserting lazy because it's not actually executing the rendering pass on the client since the transport is not live in Noop. Technically this doesn't error if the Shared Component renders something that can be a type since we actually resolve this component as if it is just some value.
It breaks the other tests.
|
|
eps1lon
approved these changes
Jun 27, 2024
github-actions bot
pushed a commit
that referenced
this pull request
Jun 27, 2024
This is all behind the `enableOwnerStacks` flag. This is a follow up to #29088. In that I moved type validation into the renderer since that's the one that knows what types are allowed. However, I only removed it from `React.createElement` and not the JSX which was an oversight. However, I also noticed that for invalid types we don't have the right stack trace for throws because we're not yet inside the JSX element that itself is invalid. We should use its stack for the stack trace. That's the reason it's enough to just use the throw now because we can get a good stack trace from the owner stack. This is fixed by creating a fake Throw Fiber that gets assigned the right stack. Additionally, I noticed that for certain invalid types like the most common one `undefined` we error in Flight so a missing import in RSC leads to a generic error. Instead of erroring on the Flight side we should just let anything that's not a Server Component through to the client and then let the Client renderer determine whether it's a valid type or not. Since we now have owner stacks through the server too, this will still be able to provide a good stack trace on the client that points to the server in that case. <img width="571" alt="Screenshot 2024-06-25 at 6 46 35 PM" src="https://github.com/facebook/react/assets/63648/6812c24f-e274-4e09-b4de-21deda9ea1d4"> To get the best stack you have to expand the little icon and the regular stack is noisy [due to this Chrome bug](https://issues.chromium.org/issues/345248263) which makes it a little harder to find but once that's fixed it might be easier. DiffTrain build for commit e02baf6.
github-actions bot
pushed a commit
that referenced
this pull request
Jun 27, 2024
This is all behind the `enableOwnerStacks` flag. This is a follow up to #29088. In that I moved type validation into the renderer since that's the one that knows what types are allowed. However, I only removed it from `React.createElement` and not the JSX which was an oversight. However, I also noticed that for invalid types we don't have the right stack trace for throws because we're not yet inside the JSX element that itself is invalid. We should use its stack for the stack trace. That's the reason it's enough to just use the throw now because we can get a good stack trace from the owner stack. This is fixed by creating a fake Throw Fiber that gets assigned the right stack. Additionally, I noticed that for certain invalid types like the most common one `undefined` we error in Flight so a missing import in RSC leads to a generic error. Instead of erroring on the Flight side we should just let anything that's not a Server Component through to the client and then let the Client renderer determine whether it's a valid type or not. Since we now have owner stacks through the server too, this will still be able to provide a good stack trace on the client that points to the server in that case. <img width="571" alt="Screenshot 2024-06-25 at 6 46 35 PM" src="https://github.com/facebook/react/assets/63648/6812c24f-e274-4e09-b4de-21deda9ea1d4"> To get the best stack you have to expand the little icon and the regular stack is noisy [due to this Chrome bug](https://issues.chromium.org/issues/345248263) which makes it a little harder to find but once that's fixed it might be easier. DiffTrain build for [e02baf6](e02baf6)
This was referenced Jul 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is all behind the
enableOwnerStacksflag.This is a follow up to #29088. In that I moved type validation into the renderer since that's the one that knows what types are allowed. However, I only removed it from
React.createElementand not the JSX which was an oversight.However, I also noticed that for invalid types we don't have the right stack trace for throws because we're not yet inside the JSX element that itself is invalid. We should use its stack for the stack trace. That's the reason it's enough to just use the throw now because we can get a good stack trace from the owner stack. This is fixed by creating a fake Throw Fiber that gets assigned the right stack.
Additionally, I noticed that for certain invalid types like the most common one
undefinedwe error in Flight so a missing import in RSC leads to a generic error. Instead of erroring on the Flight side we should just let anything that's not a Server Component through to the client and then let the Client renderer determine whether it's a valid type or not. Since we now have owner stacks through the server too, this will still be able to provide a good stack trace on the client that points to the server in that case.To get the best stack you have to expand the little icon and the regular stack is noisy due to this Chrome bug which makes it a little harder to find but once that's fixed it might be easier.