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.

[bvaughn]

I think the rough logic flow for this could be something like?

1. Is the first param a string? If so, does it contain the substring "%c"? If so, we should bail.
2. If the first param is not a string, we should replace it with a string that has a "%s" for every subsequent param.
3. If the first param is a string, we should prepend "%c" and append "%s" for every subsequent param.

[billyjanitsch]

@bvaughn I've done something similar in #24306, not sure if you had a chance to look at that. I'm a bit confused by your proposal, though. If I'm understanding you correctly:

Is the first param a string? If so, does it contain the substring "%s"? If so, we should bail.

Do you mean something like: ['A %s', 'B']? If so, I don't understand why we'd bail in this case. We can transform this to ['%cA %s', 'color: grey', 'B'].

If the first param is not a string, we should replace it with a string that has a "%c" for every subsequent param.

Can you clarify how you're suggesting we'd transform [1, 'A']?

If the first param is a string, we should prepend "%s" and append "%c" for every subsequent param.

Can you clarify how you're suggesting we'd transform ['A', 1]?

[bvaughn]

Ooops. Haven't looked at the PR yet, sorry, but just realized I accidentally swapped my "%c" and "%s" args above.

[bvaughn]

My concern with us formatting a string that's already been formatted is that the result is unpredictable.

If the first param is not a string, we should replace it with a string that has a "%c" for every subsequent param.

Can you clarify how you're suggesting we'd transform [1, 'A']?

// [1, 'A']
console.log("%c%s %s", "color: gray", 1, "A");

If the first param is a string, we should prepend "%s" and append "%c" for every subsequent param.

Can you clarify how you're suggesting we'd transform ['A', 1]?

// ['A', 1]
console.log("%cA %s", "color: gray;", 1);

[gaearon]

Seems like an accidental close? The bug is there.

[bvaughn]

Yikes. Yes, definitely accidental. Thanks.

[billyjanitsch]

I accidentally swapped my "%c" and "%s" args above.

Ohh, everything makes sense now. 😅 Let me respond after inverting them:

Is the first param a string? If so, does it contain the substring "%c"? If so, we should bail.

We could bail here, but then we'd miss out on dimming subsequent args. So I tried something a bit more clever in #24306. Basically, put one dimmed color directive at the start of that string, and then put another at the end, before the rest of the placeholders.
e.g., ['A%cB', 'color: blue', 'C'] -> ['%cA%cB %c%s', 'color: grey', 'color: blue', 'color: grey', 'C']

I can revert that bit if you'd prefer to bail entirely if the user has done anything with colors.

Edit: realized that the user might use %c to specify other properties like background color, so I agree that it's better to bail from dimming entirely if we see a %c. Will update the PR.

If the first param is not a string, we should replace it with a string that has a "%s" for every subsequent param.

I mostly agree but %s will force a string cast which brings us back to the current undesirable behavior.
e.g., [new Set()] -> ['%c%s', 'color: grey', new Set()] which prints [object Set].

However, %o will print the "object", retaining devtools inspection. So my strategy in #24306 is to use %s for args that are strings (which will get dimmed) and %o for everything else (which won't get dimmed but will retain inspection).
e.g., ['A', new Set()] -> ['%c%s %o', 'color: grey', 'A', new Set()] which prints A Set(...), where the A is dimmed and the Set is not.

(Could also use %s for other primitive values that cast well.)

If the first param is a string, we should prepend "%c" and append "%s" for every subsequent param.

I agree, aside from the same %s/%o comment.

[bvaughn]

😱 TIL about %o – yes, that makes good sense to use that flag instead then.

[lunaruan]

@billyjanitsch Are you still working on this PR? If not, I can take it over (giving you credit of course!) Thanks so much regardless for helping us come up with a strategy to avoid string casting objects!

[billyjanitsch]

If not, I can take it over

@lunaruan that sounds great, thanks! Sorry I didn't wrap up the work, caught a cold over the weekend. 🙂