[Flight] Override prepareStackTrace when reading stacks #29740
Merged
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
This lets us ensure that we use the original V8 format and it lets us skip
source mapping. Source mapping every call can be expensive since we do it
eagerly even if an error doesn't happen.
In the case of an error being thrown we don't actually always do this in
practice because if a try/catch before us touches it or if something in
onError touches it (which the default console.error does), it has already
been initialized. So we have to be resilient to thrown errors having other
formats.
These are not as perf sensitive since something actually threw but if you
want better perf in these cases, you can simply do something like
`onError(error) { console.error(error.message) }` instead.
The server has to be aware whether it's looking up original or compiled
output. I currently use the file:// check to determine if it's referring to a
source mapped file or compiled file.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
CLA Signed
React Core Team
|
@unstubbable This might help improve your RSC rendering performance because it no longer applies the source maps eagerly. |
unstubbable
added a commit
to unstubbable/next.js
that referenced
this pull request
Jun 3, 2024
|
Awesome, thanks for the quick turnaround! I've tested the fix in Next.js, and can confirm that it completely fixes the observed performance regression in Node 18 (Node 20 was fine). |
eps1lon
approved these changes
Jun 5, 2024
github-actions bot
pushed a commit
that referenced
this pull request
Jun 5, 2024
This lets us ensure that we use the original V8 format and it lets us
skip source mapping. Source mapping every call can be expensive since we
do it eagerly for server components even if an error doesn't happen.
In the case of an error being thrown we don't actually always do this in
practice because if a try/catch before us touches it or if something in
onError touches it (which the default console.error does), it has
already been initialized. So we have to be resilient to thrown errors
having other formats.
These are not as perf sensitive since something actually threw but if
you want better perf in these cases, you can simply do something like
`onError(error) { console.error(error.message) }` instead.
The server has to be aware whether it's looking up original or compiled
output. I currently use the file:// check to determine if it's referring
to a source mapped file or compiled file in the fixture. A bundled app
can more easily check if it's a bundle or not.
DiffTrain build for commit 1df34bd.
sebmarkbage
added a commit
that referenced
this pull request
Jun 7, 2024
Stacked on #29740. Before: <img width="719" alt="Screenshot 2024-06-02 at 11 51 20 AM" src="https://github.com/facebook/react/assets/63648/8f79fa82-2474-4583-894e-a2329e9a6304"> After (updated with my patches to Chrome): <img width="813" alt="Screenshot 2024-06-06 at 5 16 20 PM" src="https://github.com/facebook/react/assets/63648/bcc4f52f-e0ac-4708-ac2b-9629acdff705"> Sources panel after: <img width="1188" alt="Screenshot 2024-06-06 at 5 14 21 PM" src="https://github.com/facebook/react/assets/63648/2c673fac-d32d-42e4-8fac-bb63704e4b7f"> The fake eval file is now under "React" and the real file is now under `file://`
github-actions bot
pushed a commit
that referenced
this pull request
Jun 7, 2024
Stacked on #29740. Before: <img width="719" alt="Screenshot 2024-06-02 at 11 51 20 AM" src="https://github.com/facebook/react/assets/63648/8f79fa82-2474-4583-894e-a2329e9a6304"> After (updated with my patches to Chrome): <img width="813" alt="Screenshot 2024-06-06 at 5 16 20 PM" src="https://github.com/facebook/react/assets/63648/bcc4f52f-e0ac-4708-ac2b-9629acdff705"> Sources panel after: <img width="1188" alt="Screenshot 2024-06-06 at 5 14 21 PM" src="https://github.com/facebook/react/assets/63648/2c673fac-d32d-42e4-8fac-bb63704e4b7f"> The fake eval file is now under "React" and the real file is now under `file://` DiffTrain build for commit cc1ec60.
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 lets us ensure that we use the original V8 format and it lets us skip source mapping. Source mapping every call can be expensive since we do it eagerly for server components even if an error doesn't happen.
In the case of an error being thrown we don't actually always do this in practice because if a try/catch before us touches it or if something in onError touches it (which the default console.error does), it has already been initialized. So we have to be resilient to thrown errors having other formats.
These are not as perf sensitive since something actually threw but if you want better perf in these cases, you can simply do something like
onError(error) { console.error(error.message) }instead.The server has to be aware whether it's looking up original or compiled output. I currently use the file:// check to determine if it's referring to a source mapped file or compiled file in the fixture. A bundled app can more easily check if it's a bundle or not.