Console dimming on second StrictMode render forces string cast

[billyjanitsch]

React version: 18.0.0 (congrats on the release ☺️)

Steps To Reproduce

1. During rendering of a component, log something that doesn't naturally cast to a string (e.g., console.log(new Set())).
2. Wrap the tree in StrictMode.
3. Observe the console.

Link to code example: https://codesandbox.io/s/magical-roman-wyeud7?file=/src/App.js

Note that the console dimming isn't applied to the inline CodeSandbox dev tools, so to see the issue, you need to visit the "fullscreen view" here: https://wyeud7.csb.app

The current behavior

In Chrome:

As expected, there are two console logs, one dimmed. Unfortunately, the way that the dimming works forces the second log to be serialized to a string. This has two issues:

1. It can result in two of the "same" logs looking very "different" from each other, which is confusing to developers. For example, in the screenshot above, it's pretty surprising that those two console lines occur from the same console.log call.
2. It prevents browser dev tools introspection. This can make it inconvenient or impossible to compare the two values if the string cast doesn't include the value, as in the screenshot above. This is problematic because a key use case of printing both values is to check whether they're the same.

You can kind of work around the second issue by writing your own string cast at the log callsite, but you lose the DX of introspection, which is pretty unfortunate especially in the case of large/deeply-nested objects, etc. Easier to compare two native console values than two JSON.stringify dumps.

The expected behavior

While there is a new (appreciated!) dev tools option to suppress logging for the second render entirely, there is no way to disable the dimming feature.

Any of the following options would solve the issue:

1. Provide a dev tools option to disable dimming.
2. Remove the dimming feature entirely. (So that both logs are always printed the same way.)
3. Update the dimming implementation so that it doesn't force a string cast. (Guessing this isn't possible.)
4. Improve serialization of complex values. (IMO this isn't a great option because it doesn't solve the issue of consistency/confusion, but it would be better than the current behavior if all other options are ruled out.)

Thanks for considering.

[gaearon]

Unfortunately, the way that the dimming works forces the second log to be serialized to a string.

Update the dimming implementation so that it doesn't force a string cast. (Guessing this isn't possible.)

I don’t see why that wouldn’t be possible. I might be missing something but my guess is that it’s a simple mistake somewhere. Do you want to have a look?

[billyjanitsch]

I don’t see why that wouldn’t be possible.

The dimming works by using the browser's console styling API (see the "Styling console output" section of this page) which only lets you apply colors to strings because it's done via directive, e.g.:

console.log('%cfoo', 'color: blue')

So the React implementation has to cast to a string:

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

Line 294 in f56dfe9

originalMethod(`%c${format(...args)}`, `color: ${color}`);

(format happens to also cast to a string, but even if it didn't, the cast here would need to exist regardless. A bit more context in #22030 and #22064.)

So I don't see any way to preserve the dimming but avoid the cast.

(I guess you could apply the dimming conditionally, only to args that are strings? Not sure if that's more or less confusing, and the implementation would be a bit complex because, if I understood correctly, this is also the code path that formats component stacks.)

[gaearon]

I guess you could apply the dimming conditionally, only to args that are strings?

Yes. I think that would be fine.

[billyjanitsch]

Yes. I think that would be fine.

I think this is doable, it just gets a bit tricky because the console API has a mixed interface that supports both string substitutions and multiple arguments. So you need to interact with other directives. For example:

// userland code:
console.log('A %d %s', 1, 'B', 2, 'C')

// needs to be transformed to:
console.log('%cA %d %s %c%o %s', 'color: grey', 1, 'B', 'color: grey', 2, 'C')

Happy to try a PR. Are you confident enough in this behavior that I should get started, or do you want to discuss with anyone first?

I do have one question about the existing implementation. Currently, React re-implements the directive substitutions while casting all of the args to a single string:

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

Lines 175 to 210 in f56dfe9

	function format(
	maybeMessage: any,
	...inputArgs: $ReadOnlyArray<any>
	): string {
	const args = inputArgs.slice();
	// Symbols cannot be concatenated with Strings.
	let formatted = String(maybeMessage);
	// If the first argument is a string, check for substitutions.
	if (typeof maybeMessage === 'string') {
	if (args.length) {
	const REGEXP = /(%?)(%([jds]))/g;
	formatted = formatted.replace(REGEXP, (match, escaped, ptn, flag) => {
	let arg = args.shift();
	switch (flag) {
	case 's':
	arg += '';
	break;
	case 'd':
	case 'i':
	arg = parseInt(arg, 10).toString();
	break;
	case 'f':
	arg = parseFloat(arg).toString();
	break;
	}
	if (!escaped) {
	return arg;
	}
	args.unshift(arg);
	return match;
	});
	}
	}

I see why this is necessary when dimming the entire line, but I think it might be avoidable otherwise. So I just wondered if this has any other purpose (behavior in Node or other browser envs?) before I remove it. Didn't see anything in the implementing PR.

[gaearon]

I was thinking in the realm of changing

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

Lines 293 to 294 in f56dfe9

	if (color) {
	originalMethod(`%c${format(...args)}`, `color: ${color}`);

to something like

            if (color) {
              if (typeof args[0] === 'string') {
                originalMethod(`%c${format(...args)}`, `color: ${color}`);
              } else {
                originalMethod(...args);
              }

[gaearon]

Oh I see, you'd need to check all of them... I think as a first approximation it's ok to opt out if any of them are non-strings. But what you're suggesting is preferable to that. Yeah I think you can stake a stab at this.

[billyjanitsch]

(This is incorrect.)

Yeah I think it's not so bad as long as it's ok for me to squash the existing substitution re-implementation. Something like:

1. Chunk the args into groups that capture directives and values.
   e.g., ['A %s', 'B', 1, 'C'] => [['A %s', 'B'], [1], ['C']].
2. For each group, check whether the first arg is a string. If so, prefix it with '%c' and insert dimming after it.
   e.g., ['A %s', 'B'] => ['%cA %s', 'color: grey', 'B'].
3. Flatten the groups and forward to the native method.
   e.g., originalMethod('%cA %s', 'color: grey', 'B', 1, 'C')

[billyjanitsch]

Seems like it would be nice to get rid of the substitution code anyway because it's a bit buggy and probably hard to get perfectly right. e.g., console.log('%o', []) normally prints [] but when the console is patched by React rn it prints %o .

[billyjanitsch]

Ah sorry I misunderstood the console API: you can only specify directives in the first argument. I understand the current React implementation better now.

That means the approach we discussed won't work (because of cases like console.log('A', [], 'B')) but I'll try another idea.

[billyjanitsch]

Put up a WIP #24306 with a basic working implementation. Needs tests, callsite updates, etc. Not sure when I'll have time to continue so if anyone's eager and wants to take it over from there, feel free.