Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure react dev tools is patched before console.error #48784

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

rickhanlonii
Copy link
Member

@rickhanlonii rickhanlonii commented Jan 19, 2025

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:
image

After

Just 1:
image

Differential Revision: D68380665

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
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Jan 19, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D68380665

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants