Skip to content

Commit 91096cf

Browse files
hoxyqhoxyq
committed
chore[react-devtools]: improve console arguments formatting before passing it to original console
1 parent b5b106b commit 91096cf

File tree

5 files changed

+209
-28
lines changed

5 files changed

+209
-28
lines changed

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

Lines changed: 69 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ import {
1414
} from 'react-devtools-shared/src/utils';
1515
import {stackToComponentSources} from 'react-devtools-shared/src/devtools/utils';
1616
import {
17-
format,
17+
formatConsoleArguments,
18+
formatConsoleArgumentsToSingleString,
1819
formatWithStyles,
1920
gt,
2021
gte,
@@ -123,51 +124,51 @@ describe('utils', () => {
123124
});
124125
});
125126

126-
describe('format', () => {
127-
// @reactVersion >= 16.0
127+
describe('formatConsoleArgumentsToSingleString', () => {
128128
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+
);
130132
});
131133

132-
// @reactVersion >= 16.0
133134
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+
);
135138
});
136139

137-
// @reactVersion >= 16.0
138140
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');
140144
});
141145

142-
// @reactVersion >= 16.0
143146
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)');
147150
});
148151

149-
// @reactVersion >= 16.0
150152
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+
);
152156
});
153157
});
154158

155159
describe('formatWithStyles', () => {
156-
// @reactVersion >= 16.0
157160
it('should format empty arrays', () => {
158161
expect(formatWithStyles([])).toEqual([]);
159162
expect(formatWithStyles([], 'gray')).toEqual([]);
160163
expect(formatWithStyles(undefined)).toEqual(undefined);
161164
});
162165

163-
// @reactVersion >= 16.0
164166
it('should bail out of strings with styles', () => {
165167
expect(
166168
formatWithStyles(['%ca', 'color: green', 'b', 'c'], 'color: gray'),
167169
).toEqual(['%ca', 'color: green', 'b', 'c']);
168170
});
169171

170-
// @reactVersion >= 16.0
171172
it('should format simple strings', () => {
172173
expect(formatWithStyles(['a'])).toEqual(['a']);
173174

@@ -186,7 +187,6 @@ describe('utils', () => {
186187
]);
187188
});
188189

189-
// @reactVersion >= 16.0
190190
it('should format string substituions', () => {
191191
expect(
192192
formatWithStyles(['%s %s %s', 'a', 'b', 'c'], 'color: gray'),
@@ -199,7 +199,6 @@ describe('utils', () => {
199199
);
200200
});
201201

202-
// @reactVersion >= 16.0
203202
it('should support multiple argument types', () => {
204203
const symbol = Symbol('a');
205204
expect(
@@ -219,7 +218,6 @@ describe('utils', () => {
219218
]);
220219
});
221220

222-
// @reactVersion >= 16.0
223221
it('should properly format escaped string substituions', () => {
224222
expect(formatWithStyles(['%%s'], 'color: gray')).toEqual([
225223
'%c%s',
@@ -234,7 +232,6 @@ describe('utils', () => {
234232
expect(formatWithStyles(['%%c%c'], 'color: gray')).toEqual(['%%c%c']);
235233
});
236234

237-
// @reactVersion >= 16.0
238235
it('should format non string inputs as the first argument', () => {
239236
expect(formatWithStyles([{foo: 'bar'}])).toEqual([{foo: 'bar'}]);
240237
expect(formatWithStyles([[1, 2, 3]])).toEqual([[1, 2, 3]]);
@@ -387,4 +384,55 @@ describe('utils', () => {
387384
});
388385
});
389386
});
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+
});
390438
});

‎packages/react-devtools-shared/src/backend/console.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,11 @@ import type {
1515
WorkTagMap,
1616
ConsolePatchSettings,
1717
} from './types';
18-
import {formatWithStyles} from './utils';
1918

19+
import {
20+
formatConsoleArguments,
21+
formatWithStyles,
22+
} from 'react-devtools-shared/src/backend/utils';
2023
import {
2124
FIREFOX_CONSOLE_DIMMING_COLOR,
2225
ANSI_STYLE_DIMMING_TEMPLATE,
@@ -335,7 +338,10 @@ export function patchForStrictMode() {
335338
...formatWithStyles(args, FIREFOX_CONSOLE_DIMMING_COLOR),
336339
);
337340
} else {
338-
originalMethod(ANSI_STYLE_DIMMING_TEMPLATE, ...args);
341+
originalMethod(
342+
ANSI_STYLE_DIMMING_TEMPLATE,
343+
...formatConsoleArguments(...args),
344+
);
339345
}
340346
}
341347
};

‎packages/react-devtools-shared/src/backend/renderer.js‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import {
4040
} from 'react-devtools-shared/src/utils';
4141
import {sessionStorageGetItem} from 'react-devtools-shared/src/storage';
4242
import {
43+
formatConsoleArgumentsToSingleString,
4344
gt,
4445
gte,
4546
parseSourceFromComponentStack,
@@ -95,7 +96,6 @@ import {
9596
MEMO_SYMBOL_STRING,
9697
SERVER_CONTEXT_SYMBOL_STRING,
9798
} from './ReactSymbols';
98-
import {format} from './utils';
9999
import {enableStyleXFeatures} from 'react-devtools-feature-flags';
100100
import is from 'shared/objectIs';
101101
import hasOwnProperty from 'shared/hasOwnProperty';
@@ -841,7 +841,14 @@ export function attach(
841841
return;
842842
}
843843
}
844-
const message = format(...args);
844+
845+
// We can't really use this message as a unique key, since we can't distinguish
846+
// different objects in this implementation. We have to delegate displaying of the objects
847+
// to the environment, the browser console, for example, so this is why this should be kept
848+
// as an array of arguments, instead of the plain string.
849+
// [Warning: %o, {...}] and [Warning: %o, {...}] will be considered as the same message,
850+
// even if objects are different
851+
const message = formatConsoleArgumentsToSingleString(...args);
845852
if (__DEBUG__) {
846853
debug('onErrorOrWarning', fiber, null, `${type}: "${message}"`);
847854
}

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

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ export function serializeToString(data: any): string {
164164
);
165165
}
166166

167+
// NOTE: KEEP IN SYNC with src/hook.js
167168
// Formats an array of args with a style for console methods, using
168169
// the following algorithm:
169170
// 1. The first param is a string that contains %c
@@ -220,11 +221,72 @@ export function formatWithStyles(
220221
}
221222
}
222223

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+
223286
// based on https://github.com/tmpfs/format-util/blob/0e62d430efb0a1c51448709abd3e2406c14d8401/format.js#L1
224287
// based on https://developer.mozilla.org/en-US/docs/Web/API/console#Using_string_substitutions
225288
// Implements s, d, i and f placeholders
226-
// NOTE: KEEP IN SYNC with src/hook.js
227-
export function format(
289+
export function formatConsoleArgumentsToSingleString(
228290
maybeMessage: any,
229291
...inputArgs: $ReadOnlyArray<any>
230292
): string {

‎packages/react-devtools-shared/src/hook.js‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,61 @@ export function installHook(target: any): DevToolsHook | null {
220220
return [firstArg, style, ...inputArgs];
221221
}
222222
}
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+
}
223278

224279
let unpatchFn = null;
225280

@@ -274,7 +329,10 @@ export function installHook(target: any): DevToolsHook | null {
274329
...formatWithStyles(args, FIREFOX_CONSOLE_DIMMING_COLOR),
275330
);
276331
} else {
277-
originalMethod(ANSI_STYLE_DIMMING_TEMPLATE, ...args);
332+
originalMethod(
333+
ANSI_STYLE_DIMMING_TEMPLATE,
334+
...formatConsoleArguments(...args),
335+
);
278336
}
279337
}
280338
};

0 commit comments

Comments
 (0)