Remove console.error patch for LogBox #48783
Open
+152
−128
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
Summary: React currently has a babel transform to replace all `console.error` calls with a special function for RN and www. The function adds component stacks, which doesn't make sense in an owner stack world because: - not all console.error calls are replaced, creating inconsistency - oss web users don't append component stacks to console.log any more - component stacks can be accessed for DEV modals like logbox with `captureOwnerStack` - owner stacks are already added to the console with createTask if you use console.error directly - the redirection is the single greatest source of fragility in the logbox reporting pipeline So we're removing this as part of the owner stack rollout. This should only be enabled with owner stacks. ## Example Screen Before: {F1974436644} After: {F1974436645} [General][Added] - Add full owner stack support to React Native Differential Revision: D68285628
Summary:
## Overview
This change adds code frames for component stacks if it differs from the call stack frame.
## Background
After adding native stack based stack frames, which we already symbolicate, we had the capability to show a code frame for component stacks as well as the stack frame (if they differ).
However, this usually wasn't that useful, because the native component stack was unlikely to be more useful than the component stack (see the comparisons below for key errors).
## Owner stacks
With owner stacks the component frame is a lot more useful and in many cases are better at showing the location than the call stack frame.
## Example Screens
Before:
{F1974436645}
After (with owner stacks):
{F1974436723}
Changelog:
[General][Added] - Add owner stack code frames to LogBox
Differential Revision: D68285627
Summary:
React requires React DevTools to be the first patch for console methods, because it assumes it is the _last_ on the stack called before calling the real console.
This is because React DevTools adds additional formatting for things like StrictMode dimming, and component stack formatting for browser specific consoles like Chrome and Firefox, where the DevTool extension runs.
If it's not the first patch, then other patches (like logging, or LogBox) will pick up the DevTools additions and include them, which breaks other tools (like the issue show in the screen below).
This diff ensures React DevTools is patched before the React Native console reporter by moving it to `setupErrorHandling`.
[General][Fixed] - Always patch React DevTools first so StrictMode dim chars are excluded from logs/logbox.
## Other places?
I'm not sure how we should handle this for the console polyfill or inside useAlwaysAvailableJSErrorHandling yet.
## Screens
### Before
3 errors, 1 with ANSI dim chars:
{F1974436874}
### After
Just 1:
{F1974436879}
Differential Revision: D68380665
Summary:
## Overview
This is the final boss of the new owner stacks feature. With owner stacks, we don't need to parse message strings to find the component stack for logbox. Instead, we can access the component stack directly with `captureOwnerStack`.
This means we don't need to install the LogBox console.error patch and can greatly simplify the process of handling errors and make it more reliable.
To do this, we rely only on adding LogBox to the ExceptionManager:
- `reactConsoleErrorHandler` -> `LogBox.addConsoleLog`
- `reportException` -> `LogBox.addException`
[General][Fixed] - Remove LogBox patch, de-duplicating errors
## Benefits
As a side effect, this removes a lot of duplicate errors. For example, currently if a component throws, you get 2 errors:
{F1974436906}
After this, there's just the one you expect:
{F1974436908}
## Followups
After this lands and doesn't need reverted for some reason, we can delete a ton of code from logbox for finding and detecting stacks from errors.
Differential Revision: D68380668
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D68380668 |
rickhanlonii
changed the title
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.
Stack:
Overview
This is the final boss of the new owner stacks feature. With owner stacks, we don't need to parse message strings to find the component stack for logbox. Instead, we can access the component stack directly with
captureOwnerStack.This means we don't need to install the LogBox console.error patch and can greatly simplify the process of handling errors and make it more reliable.
To do this, we rely only on adding LogBox to the ExceptionManager:
reactConsoleErrorHandler->LogBox.addConsoleLogreportException->LogBox.addException[General][Fixed] - Remove LogBox patch, de-duplicating errors
Benefits
As a side effect, this removes a lot of duplicate errors. For example, currently if a component throws, you get 2 errors:
After this, there's just the one you expect:
Screens
Here are some example before/after for all of the owner stack changes in this stack of diffs.
console errors
Less duplication, more information, and component (owner) stacks.
Before:
After:
Key error
This is subtle to see, but the key error code frame now points to the actual callsite where you would add the key.
Before:
After:
Followups
After this lands and doesn't need reverted for some reason, we can delete a ton of code from logbox for finding and detecting stacks from errors.
Differential Revision: D68380668