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

chore[react-devtools]: unify console patching and default to ansi escape symbols #29869

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

hoxyq
Copy link
Contributor

@hoxyq hoxyq commented Jun 12, 2024

Stacked on #29856.

Summary

By default, React DevTools will apply dimming with ANSI escape symbols, so it works for both terminals and browser consoles.

For Firefox, which doesn't support ANSI escape symbols console stylings, we fallback to css properties, like we used to do before.

How did you test this change?

Environment Dark mode Light mode
Terminal Screenshot 2024-06-12 at 19 39 46 Screenshot 2024-06-12 at 19 39 09
Fusebox console Screenshot 2024-06-12 at 21 03 14 Screenshot 2024-06-12 at 21 02 48
Firefox console Screenshot 2024-06-12 at 21 40 29 Screenshot 2024-06-12 at 21 40 42
Chrome console Screenshot 2024-06-12 at 21 43 09 Screenshot 2024-06-12 at 21 43 00

Copy link

vercel bot commented Jun 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 17, 2024 3:17pm

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jun 12, 2024
@hoxyq hoxyq changed the title chore[react-devtools]: unify console patching and fallback to ansi escape symbols chore[react-devtools]: unify console patching and default to ansi escape symbols Jun 12, 2024
@react-sizebot
Copy link

Comparing: f5af92d...88b169f

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.66 kB 6.66 kB +0.05% 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 497.44 kB 497.44 kB = 89.16 kB 89.16 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB +0.05% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 502.26 kB 502.26 kB = 89.85 kB 89.85 kB
facebook-www/ReactDOM-prod.classic.js = 596.94 kB 596.94 kB = 105.24 kB 105.24 kB
facebook-www/ReactDOM-prod.modern.js = 571.12 kB 571.12 kB = 101.19 kB 101.19 kB
test_utils/ReactAllWarnings.js Deleted 63.89 kB 0.00 kB Deleted 15.97 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
test_utils/ReactAllWarnings.js Deleted 63.89 kB 0.00 kB Deleted 15.97 kB 0.00 kB

Generated by 🚫 dangerJS against 88b169f

@hoxyq hoxyq force-pushed the react-devtools/unify-console-patching branch from 88b169f to 9e0d928 Compare June 17, 2024 15:14
@hoxyq hoxyq merged commit ff6e05a into facebook:main Jun 17, 2024
42 of 43 checks passed
@hoxyq hoxyq deleted the react-devtools/unify-console-patching branch June 17, 2024 15:31
hoxyq added a commit that referenced this pull request Jun 17, 2024
…ssing it to original console (#29873)

Stacked on #29869.

## Summary

When using ANSI escape sequences, we construct a message in the
following way: `console.<method>('\x1b...%s\x1b[0m',
userspaceArgument1?, userspaceArgument2?, userspaceArgument3?, ...)`.

This won't dim all arguments, if user had something like `console.log(1,
2, 3)`, we would only apply it to `1`, since this is the first
arguments, so we need to:
- inline everything whats possible into a single string, while
preserving console substitutions defined by the user
- omit css and object substitutions, since we can't really inline them
and will delegate in to the environment

## How did you test this change?

Added some tests, manually inspected that it works well for web and
native cases.
hoxyq added a commit that referenced this pull request Jun 18, 2024
Full list of changes:

* chore[react-devtools]: improve console arguments formatting before
passing it to original console ([hoxyq](https://github.com/hoxyq) in
[#29873](#29873))
* chore[react-devtools]: unify console patching and default to ansi
escape symbols ([hoxyq](https://github.com/hoxyq) in
[#29869](#29869))
* chore[react-devtools/backend]: remove
consoleManagedByDevToolsDuringStrictMode
([hoxyq](https://github.com/hoxyq) in
[#29856](#29856))
* chore[react-devtools/extensions]: make source maps url relative
([hoxyq](https://github.com/hoxyq) in
[#29886](#29886))
* fix[react-devtools] divided inspecting elements between inspecting do…
([vzaidman](https://github.com/vzaidman) in
[#29885](#29885))
* [Fiber] Create virtual Fiber when an error occurs during reconcilation
([sebmarkbage](https://github.com/sebmarkbage) in
[#29804](#29804))
* fix[react-devtools] component badge in light mode is now not invisible
([vzaidman](https://github.com/vzaidman) in
[#29852](#29852))
* Remove Warning: prefix and toString on console Arguments
([sebmarkbage](https://github.com/sebmarkbage) in
[#29839](#29839))
* Add jest lint rules ([rickhanlonii](https://github.com/rickhanlonii)
in [#29760](#29760))
* [Fiber] Track the Real Fiber for Key Warnings
([sebmarkbage](https://github.com/sebmarkbage) in
[#29791](#29791))
* fix[react-devtools/store-test]: fork the test to represent current be…
([hoxyq](https://github.com/hoxyq) in
[#29777](#29777))
* Default native inspections config false
([vzaidman](https://github.com/vzaidman) in
[#29784](#29784))
* fix[react-devtools] remove native inspection button when it can't be
used ([vzaidman](https://github.com/vzaidman) in
[#29779](#29779))
* chore[react-devtools]: ip => internal-ip
([hoxyq](https://github.com/hoxyq) in
[#29772](#29772))
* Fix #29724: `ip` dependency update for CVE-2024-29415
([Rekl0w](https://github.com/Rekl0w) in
[#29725](#29725))
* cleanup[react-devtools]: remove unused supportsProfiling flag from
store config ([hoxyq](https://github.com/hoxyq) in
[#29193](#29193))
* [Fiber] Enable Native console.createTask Stacks When Available
([sebmarkbage](https://github.com/sebmarkbage) in
[#29223](#29223))
* Move createElement/JSX Warnings into the Renderer
([sebmarkbage](https://github.com/sebmarkbage) in
[#29088](#29088))
* Set the current fiber to the source of the error during error
reporting ([sebmarkbage](https://github.com/sebmarkbage) in
[#29044](#29044))
* Unify ReactFiberCurrentOwner and ReactCurrentFiber
([sebmarkbage](https://github.com/sebmarkbage) in
[#29038](#29038))
* Dim `console` calls on additional Effect invocations due to
`StrictMode` ([eps1lon](https://github.com/eps1lon) in
[#29007](#29007))
* refactor[react-devtools]: rewrite context menus
([hoxyq](https://github.com/hoxyq) in
[#29049](#29049))
hoxyq added a commit that referenced this pull request Jun 20, 2024
Somehow missed this while working on
#29869.

With these changes, manual inspection of the
`react_devtools_backend_compact.js` doesn't have any occurences of
`__IS_FIREFOX__` flag.
hoxyq added a commit that referenced this pull request Sep 18, 2024
…gs (#30566)

Stacked on #30564.

We are no longer using browser theme in our console patching, this was
removed in unification of console patching for strict mode, we started
using ansi escape symbols and forking based on browser theme is no
longer required - #29869

The real browser theme initialization for frontend is happening at the
other place and is not affected:

https://github.com/facebook/react/blob/40be968257a7a10a267210670103f20dd0429ef3/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js#L117-L120
github-actions bot pushed a commit that referenced this pull request Sep 18, 2024
…gs (#30566)

Stacked on #30564.

We are no longer using browser theme in our console patching, this was
removed in unification of console patching for strict mode, we started
using ansi escape symbols and forking based on browser theme is no
longer required - #29869

The real browser theme initialization for frontend is happening at the
other place and is not affected:

https://github.com/facebook/react/blob/40be968257a7a10a267210670103f20dd0429ef3/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js#L117-L120

DiffTrain build for [b521ef8](b521ef8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants