Improve ExceptionManager tests#48787
Closed
rickhanlonii wants to merge 4 commits into
Closed
Conversation
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: For reasons that will become clear in the next diff, I'm adding more assertions for things like LogBox being called, and the console being called. I also updated the patterns of things like `toHaveBeenCalledWith` vs `toBeCalledWith` so they're consistent throughout the file. Differential Revision: D68397529
facebook-github-bot
added
the
CLA Signed
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D68397529 |
Contributor
|
This pull request has been merged in b1b5a9e. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary: For reasons that will become clear in the next diff, I'm adding more assertions for things like LogBox being called, and the console being called. I also updated the patterns of things like
toHaveBeenCalledWithvstoBeCalledWithso they're consistent throughout the file.Differential Revision: D68397529