chore[react-devtools]: improve console arguments formatting before passing it to original console #29873

hoxyq · 2024-06-12T16:43:51Z

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.

vercel · 2024-06-12T16:43:55Z

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:39pm

hoxyq · 2024-06-12T16:45:05Z

packages/react-devtools-shared/src/__tests__/utils-test.js

    });

-    // @reactVersion >= 16.0


We don't really need this pragma here, because format has nothing to do with different versions of React, so there is no reason to test it against them.

react-sizebot · 2024-06-12T16:48:14Z

Comparing: f5af92d...91096cf

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	=	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	=	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 91096cf

yungsters · 2024-06-12T19:44:51Z

I’m not terribly familiar with these code paths.

Can you elaborate your test plan to include what specific changes you’re looking for, and also what you validated in specific existing browsers (e.g. specific behavior you were verifying in Firefox).

hoxyq · 2024-06-12T20:17:51Z

I’m not terribly familiar with these code paths.

Can you elaborate your test plan to include what specific changes you’re looking for, and also what you validated in specific existing browsers (e.g. specific behavior you were verifying in Firefox).

I've split these changes into 3 different PRs, some of which doesn't require the familiarity with this codebase. Some demos for React Native are in #29869, will add the Firefox here as well later.

vzaidman · 2024-06-17T12:48:29Z

What if there's a style applied to the console already when it gets to us? @hoxyq

hoxyq · 2024-06-17T13:01:53Z

What if there's a style applied to the console already when it gets to us? @hoxyq

Then we are not going to inline the css part into the string,

react/packages/react-devtools-shared/src/__tests__/utils-test.js

Lines 423 to 437 in 91096cf

    
             it("doesn't inline css", () => { 
        
               expect( 
        
                 formatConsoleArguments( 
        
                   'This is template with %c %s object %o', 
        
                   'color: rgba(...)', 
        
                   'the', 
        
                   {}, 
        
                 ), 
        
               ).toEqual([ 
        
                 'This is template with %c the object %o', 
        
                 'color: rgba(...)', 
        
                 {}, 
        
               ]); 
        
             }); 
        
           });

:

Depending on where the %c substitution is located in the user's string, the user-defined stylings will override our ANSI-based stylings:

vzaidman · 2024-06-17T13:11:09Z

the user-defined stylings will override our ANSI-based stylings

@hoxyq OK! This is kinda surprising to me.

But what if the log starts with their own ANSI?

vzaidman · 2024-06-17T13:12:51Z

packages/react-devtools-shared/src/backend/utils.js

+  maybeMessage: any,
+  ...inputArgs: $ReadOnlyArray<any>
+): $ReadOnlyArray<any> {
+  if (inputArgs.length === 0 || typeof maybeMessage !== 'string') {


maybe it is worth checking here if "maybeMessage" is an ANSI template.

maybe it is worth checking here if "maybeMessage" is an ANSI template.

Should be safe if this is correct ANSI template, it should override the stylings applied by React DevTools:

hoxyq · 2024-06-17T13:13:25Z

the user-defined stylings will override our ANSI-based stylings

@hoxyq OK! This is kinda surprising to me.

But what if the log starts with their own ANSI?

It will override our stylings:

vzaidman · 2024-06-17T13:17:31Z

It will override our stylings

OK! Approved.

…ssing it to original console

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))

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jun 12, 2024

hoxyq commented Jun 12, 2024

View reviewed changes

vercel bot deployed to Preview June 12, 2024 16:48 View deployment

hoxyq requested review from yungsters, rickhanlonii and vzaidman June 12, 2024 18:35

hoxyq mentioned this pull request Jun 13, 2024

toggle consoleManagedByDevToolsDuringStrictMode flag for native #29887

Open

vzaidman approved these changes Jun 17, 2024

View reviewed changes

vzaidman self-requested a review June 17, 2024 12:55

vzaidman reviewed Jun 17, 2024

View reviewed changes

vzaidman approved these changes Jun 17, 2024

View reviewed changes

hoxyq force-pushed the react-devtools/better-formatting-for-ansi-escape-stylings branch from 91096cf to 6d14668 Compare June 17, 2024 15:15

vercel bot deployed to Preview June 17, 2024 15:18 View deployment

chore[react-devtools]: improve console arguments formatting before pa…

116a875

…ssing it to original console

hoxyq force-pushed the react-devtools/better-formatting-for-ansi-escape-stylings branch from 6d14668 to 116a875 Compare June 17, 2024 15:32

hoxyq merged commit 107a2f8 into facebook:main Jun 17, 2024
37 of 42 checks passed

hoxyq deleted the react-devtools/better-formatting-for-ansi-escape-stylings branch June 17, 2024 15:38

vercel bot deployed to Preview June 17, 2024 15:39 View deployment

hoxyq mentioned this pull request Jun 17, 2024

React DevTools 5.2.0 -> 5.3.0 #29918

Merged

eps1lon mentioned this pull request Jul 16, 2024

Update React from dfd30974ab-20240613 to 01172397-20240716 vercel/next.js#67810

Closed

This was referenced Jul 25, 2024

Update React from 6230622a1a-20240610 to 76002254-20240724 vercel/next.js#68149

Merged

Update React from 6230622a1a-20240610 to 3208e73e-20240730 vercel/next.js#68330

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore[react-devtools]: improve console arguments formatting before passing it to original console #29873

chore[react-devtools]: improve console arguments formatting before passing it to original console #29873

hoxyq commented Jun 12, 2024 •

edited

Loading

vercel bot commented Jun 12, 2024 •

edited

Loading

hoxyq Jun 12, 2024

react-sizebot commented Jun 12, 2024

yungsters commented Jun 12, 2024

hoxyq commented Jun 12, 2024

vzaidman commented Jun 17, 2024

hoxyq commented Jun 17, 2024

vzaidman commented Jun 17, 2024

vzaidman Jun 17, 2024

hoxyq Jun 17, 2024

hoxyq commented Jun 17, 2024

vzaidman commented Jun 17, 2024

chore[react-devtools]: improve console arguments formatting before passing it to original console #29873

chore[react-devtools]: improve console arguments formatting before passing it to original console #29873

Conversation

hoxyq commented Jun 12, 2024 • edited Loading

Summary

How did you test this change?

vercel bot commented Jun 12, 2024 • edited Loading

hoxyq Jun 12, 2024

Choose a reason for hiding this comment

react-sizebot commented Jun 12, 2024

Critical size changes

Significant size changes

yungsters commented Jun 12, 2024

hoxyq commented Jun 12, 2024

vzaidman commented Jun 17, 2024

hoxyq commented Jun 17, 2024

vzaidman commented Jun 17, 2024

vzaidman Jun 17, 2024

Choose a reason for hiding this comment

hoxyq Jun 17, 2024

Choose a reason for hiding this comment

hoxyq commented Jun 17, 2024

vzaidman commented Jun 17, 2024

hoxyq commented Jun 12, 2024 •

edited

Loading

vercel bot commented Jun 12, 2024 •

edited

Loading