@samitouri / QOS-React-2 / commits / 107a2f8c3e

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

Stacked on https://github.com/facebook/react/pull/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.

Ruslan Lesiutin committed Jun 17, 2024 at 16:38 UTC 107a2f8c3e43ee5f4f6872e68cd9aff14f3fa6d3
5 files changed +209 -28
packages/react-devtools-shared/src/__tests__/utils-test.js
+69 -21
@@ -14,7 +14,8 @@ import {
14 } from 'react-devtools-shared/src/utils';
15 import {stackToComponentSources} from 'react-devtools-shared/src/devtools/utils';
16 import {
17 - format,
17 + formatConsoleArguments,
18 + formatConsoleArgumentsToSingleString,
19 formatWithStyles,
20 gt,
21 gte,
@@ -123,51 +124,51 @@ describe('utils', () => {
124 });
125 });
126
126 - describe('format', () => {
127 - // @reactVersion >= 16.0
127 + describe('formatConsoleArgumentsToSingleString', () => {
128 it('should format simple strings', () => {
129 - expect(format('a', 'b', 'c')).toEqual('a b c');
129 + expect(formatConsoleArgumentsToSingleString('a', 'b', 'c')).toEqual(
130 + 'a b c',
131 + );
132 });
133
132 - // @reactVersion >= 16.0
134 it('should format multiple argument types', () => {
134 - expect(format('abc', 123, true)).toEqual('abc 123 true');
135 + expect(formatConsoleArgumentsToSingleString('abc', 123, true)).toEqual(
136 + 'abc 123 true',
137 + );
138 });
139
137 - // @reactVersion >= 16.0
140 it('should support string substitutions', () => {
139 - expect(format('a %s b %s c', 123, true)).toEqual('a 123 b true c');
141 + expect(
142 + formatConsoleArgumentsToSingleString('a %s b %s c', 123, true),
143 + ).toEqual('a 123 b true c');
144 });
145
142 - // @reactVersion >= 16.0
146 it('should gracefully handle Symbol types', () => {
144 - expect(format(Symbol('a'), 'b', Symbol('c'))).toEqual(
145 - 'Symbol(a) b Symbol(c)',
146 - );
147 + expect(
148 + formatConsoleArgumentsToSingleString(Symbol('a'), 'b', Symbol('c')),
149 + ).toEqual('Symbol(a) b Symbol(c)');
150 });
151
149 - // @reactVersion >= 16.0
152 it('should gracefully handle Symbol type for the first argument', () => {
151 - expect(format(Symbol('abc'), 123)).toEqual('Symbol(abc) 123');
153 + expect(formatConsoleArgumentsToSingleString(Symbol('abc'), 123)).toEqual(
154 + 'Symbol(abc) 123',
155 + );
156 });
157 });
158
159 describe('formatWithStyles', () => {
156 - // @reactVersion >= 16.0
160 it('should format empty arrays', () => {
161 expect(formatWithStyles([])).toEqual([]);
162 expect(formatWithStyles([], 'gray')).toEqual([]);
163 expect(formatWithStyles(undefined)).toEqual(undefined);
164 });
165
163 - // @reactVersion >= 16.0
166 it('should bail out of strings with styles', () => {
167 expect(
168 formatWithStyles(['%ca', 'color: green', 'b', 'c'], 'color: gray'),
169 ).toEqual(['%ca', 'color: green', 'b', 'c']);
170 });
171
170 - // @reactVersion >= 16.0
172 it('should format simple strings', () => {
173 expect(formatWithStyles(['a'])).toEqual(['a']);
174
@@ -186,7 +187,6 @@ describe('utils', () => {
187 ]);
188 });
189
189 - // @reactVersion >= 16.0
190 it('should format string substituions', () => {
191 expect(
192 formatWithStyles(['%s %s %s', 'a', 'b', 'c'], 'color: gray'),
@@ -199,7 +199,6 @@ describe('utils', () => {
199 );
200 });
201
202 - // @reactVersion >= 16.0
202 it('should support multiple argument types', () => {
203 const symbol = Symbol('a');
204 expect(
@@ -219,7 +218,6 @@ describe('utils', () => {
218 ]);
219 });
220
222 - // @reactVersion >= 16.0
221 it('should properly format escaped string substituions', () => {
222 expect(formatWithStyles(['%%s'], 'color: gray')).toEqual([
223 '%c%s',
@@ -234,7 +232,6 @@ describe('utils', () => {
232 expect(formatWithStyles(['%%c%c'], 'color: gray')).toEqual(['%%c%c']);
233 });
234
237 - // @reactVersion >= 16.0
235 it('should format non string inputs as the first argument', () => {
236 expect(formatWithStyles([{foo: 'bar'}])).toEqual([{foo: 'bar'}]);
237 expect(formatWithStyles([[1, 2, 3]])).toEqual([[1, 2, 3]]);
@@ -387,4 +384,55 @@ describe('utils', () => {
384 });
385 });
386 });
387 +
388 + describe('formatConsoleArguments', () => {
389 + it('works with empty arguments list', () => {
390 + expect(formatConsoleArguments(...[])).toEqual([]);
391 + });
392 +
393 + it('works for string without escape sequences', () => {
394 + expect(
395 + formatConsoleArguments('This is the template', 'And another string'),
396 + ).toEqual(['This is the template', 'And another string']);
397 + });
398 +
399 + it('works with strings templates', () => {
400 + expect(formatConsoleArguments('This is %s template', 'the')).toEqual([
401 + 'This is the template',
402 + ]);
403 + });
404 +
405 + it('skips %%s', () => {
406 + expect(formatConsoleArguments('This %%s is %s template', 'the')).toEqual([
407 + 'This %%s is the template',
408 + ]);
409 + });
410 +
411 + it('works with %%%s', () => {
412 + expect(
413 + formatConsoleArguments('This %%%s is %s template', 'test', 'the'),
414 + ).toEqual(['This %%test is the template']);
415 + });
416 +
417 + it("doesn't inline objects", () => {
418 + expect(
419 + formatConsoleArguments('This is %s template with object %o', 'the', {}),
420 + ).toEqual(['This is the template with object %o', {}]);
421 + });
422 +
423 + it("doesn't inline css", () => {
424 + expect(
425 + formatConsoleArguments(
426 + 'This is template with %c %s object %o',
427 + 'color: rgba(...)',
428 + 'the',
429 + {},
430 + ),
431 + ).toEqual([
432 + 'This is template with %c the object %o',
433 + 'color: rgba(...)',
434 + {},
435 + ]);
436 + });
437 + });
438 });
packages/react-devtools-shared/src/backend/console.js
+8 -2
@@ -15,8 +15,11 @@ import type {
15 WorkTagMap,
16 ConsolePatchSettings,
17 } from './types';
18 -import {formatWithStyles} from './utils';
18
19 +import {
20 + formatConsoleArguments,
21 + formatWithStyles,
22 +} from 'react-devtools-shared/src/backend/utils';
23 import {
24 FIREFOX_CONSOLE_DIMMING_COLOR,
25 ANSI_STYLE_DIMMING_TEMPLATE,
@@ -335,7 +338,10 @@ export function patchForStrictMode() {
338 ...formatWithStyles(args, FIREFOX_CONSOLE_DIMMING_COLOR),
339 );
340 } else {
338 - originalMethod(ANSI_STYLE_DIMMING_TEMPLATE, ...args);
341 + originalMethod(
342 + ANSI_STYLE_DIMMING_TEMPLATE,
343 + ...formatConsoleArguments(...args),
344 + );
345 }
346 }
347 };
packages/react-devtools-shared/src/backend/renderer.js
+9 -2
@@ -40,6 +40,7 @@ import {
40 } from 'react-devtools-shared/src/utils';
41 import {sessionStorageGetItem} from 'react-devtools-shared/src/storage';
42 import {
43 + formatConsoleArgumentsToSingleString,
44 gt,
45 gte,
46 parseSourceFromComponentStack,
@@ -95,7 +96,6 @@ import {
96 MEMO_SYMBOL_STRING,
97 SERVER_CONTEXT_SYMBOL_STRING,
98 } from './ReactSymbols';
98 -import {format} from './utils';
99 import {enableStyleXFeatures} from 'react-devtools-feature-flags';
100 import is from 'shared/objectIs';
101 import hasOwnProperty from 'shared/hasOwnProperty';
@@ -851,7 +851,14 @@ export function attach(
851 return;
852 }
853 }
854 - const message = format(...args);
854 +
855 + // We can't really use this message as a unique key, since we can't distinguish
856 + // different objects in this implementation. We have to delegate displaying of the objects
857 + // to the environment, the browser console, for example, so this is why this should be kept
858 + // as an array of arguments, instead of the plain string.
859 + // [Warning: %o, {...}] and [Warning: %o, {...}] will be considered as the same message,
860 + // even if objects are different
861 + const message = formatConsoleArgumentsToSingleString(...args);
862 if (__DEBUG__) {
863 debug('onErrorOrWarning', fiber, null, `${type}: "${message}"`);
864 }
packages/react-devtools-shared/src/backend/utils.js
+64 -2
@@ -164,6 +164,7 @@ export function serializeToString(data: any): string {
164 );
165 }
166
167 +// NOTE: KEEP IN SYNC with src/hook.js
168 // Formats an array of args with a style for console methods, using
169 // the following algorithm:
170 // 1. The first param is a string that contains %c
@@ -220,11 +221,72 @@ export function formatWithStyles(
221 }
222 }
223
224 +// NOTE: KEEP IN SYNC with src/hook.js
225 +// Skips CSS and object arguments, inlines other in the first argument as a template string
226 +export function formatConsoleArguments(
227 + maybeMessage: any,
228 + ...inputArgs: $ReadOnlyArray<any>
229 +): $ReadOnlyArray<any> {
230 + if (inputArgs.length === 0 || typeof maybeMessage !== 'string') {
231 + return [maybeMessage, ...inputArgs];
232 + }
233 +
234 + const args = inputArgs.slice();
235 +
236 + let template = '';
237 + let argumentsPointer = 0;
238 + for (let i = 0; i < maybeMessage.length; ++i) {
239 + const currentChar = maybeMessage[i];
240 + if (currentChar !== '%') {
241 + template += currentChar;
242 + continue;
243 + }
244 +
245 + const nextChar = maybeMessage[i + 1];
246 + ++i;
247 +
248 + // Only keep CSS and objects, inline other arguments
249 + switch (nextChar) {
250 + case 'c':
251 + case 'O':
252 + case 'o': {
253 + ++argumentsPointer;
254 + template += `%${nextChar}`;
255 +
256 + break;
257 + }
258 + case 'd':
259 + case 'i': {
260 + const [arg] = args.splice(argumentsPointer, 1);
261 + template += parseInt(arg, 10).toString();
262 +
263 + break;
264 + }
265 + case 'f': {
266 + const [arg] = args.splice(argumentsPointer, 1);
267 + template += parseFloat(arg).toString();
268 +
269 + break;
270 + }
271 + case 's': {
272 + const [arg] = args.splice(argumentsPointer, 1);
273 + template += arg.toString();
274 +
275 + break;
276 + }
277 +
278 + default:
279 + template += `%${nextChar}`;
280 + }
281 + }
282 +
283 + return [template, ...args];
284 +}
285 +
286 // based on https://github.com/tmpfs/format-util/blob/0e62d430efb0a1c51448709abd3e2406c14d8401/format.js#L1
287 // based on https://developer.mozilla.org/en-US/docs/Web/API/console#Using_string_substitutions
288 // Implements s, d, i and f placeholders
226 -// NOTE: KEEP IN SYNC with src/hook.js
227 -export function format(
289 +export function formatConsoleArgumentsToSingleString(
290 maybeMessage: any,
291 ...inputArgs: $ReadOnlyArray<any>
292 ): string {
packages/react-devtools-shared/src/hook.js
+59 -1
@@ -220,6 +220,61 @@ export function installHook(target: any): DevToolsHook | null {
220 return [firstArg, style, ...inputArgs];
221 }
222 }
223 + // NOTE: KEEP IN SYNC with src/backend/utils.js
224 + function formatConsoleArguments(
225 + maybeMessage: any,
226 + ...inputArgs: $ReadOnlyArray<any>
227 + ): $ReadOnlyArray<any> {
228 + if (inputArgs.length === 0 || typeof maybeMessage !== 'string') {
229 + return [maybeMessage, ...inputArgs];
230 + }
231 +
232 + const args = inputArgs.slice();
233 +
234 + let template = '';
235 + let argumentsPointer = 0;
236 + for (let i = 0; i < maybeMessage.length; ++i) {
237 + const currentChar = maybeMessage[i];
238 + if (currentChar !== '%') {
239 + template += currentChar;
240 + continue;
241 + }
242 +
243 + const nextChar = maybeMessage[i + 1];
244 + ++i;
245 +
246 + // Only keep CSS and objects, inline other arguments
247 + switch (nextChar) {
248 + case 'c':
249 + case 'O':
250 + case 'o': {
251 + ++argumentsPointer;
252 + template += `%${nextChar}`;
253 +
254 + break;
255 + }
256 + case 'd':
257 + case 'i': {
258 + const [arg] = args.splice(argumentsPointer, 1);
259 + template += parseInt(arg, 10).toString();
260 +
261 + break;
262 + }
263 + case 'f': {
264 + const [arg] = args.splice(argumentsPointer, 1);
265 + template += parseFloat(arg).toString();
266 +
267 + break;
268 + }
269 + case 's': {
270 + const [arg] = args.splice(argumentsPointer, 1);
271 + template += arg.toString();
272 + }
273 + }
274 + }
275 +
276 + return [template, ...args];
277 + }
278
279 let unpatchFn = null;
280
@@ -274,7 +329,10 @@ export function installHook(target: any): DevToolsHook | null {
329 ...formatWithStyles(args, FIREFOX_CONSOLE_DIMMING_COLOR),
330 );
331 } else {
277 - originalMethod(ANSI_STYLE_DIMMING_TEMPLATE, ...args);
332 + originalMethod(
333 + ANSI_STYLE_DIMMING_TEMPLATE,
334 + ...formatConsoleArguments(...args),
335 + );
336 }
337 }
338 };