Ensure arguments are coerced to strings in warnings #13385
Conversation
This prevents a bug where Chrome reports `Array(n)` where `n` is the size of the array.
nhunzaker
changed the title
|
Run Prettier? |
|
Hmm. Should we disallow non-string/number args to warning? |
Oops. Done.
I kind of think so, ya. I wonder if we could just set this with Flow and fix what fails. |
|
|
||
| let argIndex = 0; | ||
| const message = | ||
| 'Warning: ' + format.replace(/%s/g, () => `${args[argIndex++]}`); |
There was a problem hiding this comment.
Hmmm. 'Warning: ' + is redundant with ${ args }. Fixing real quick.
| } | ||
|
|
||
| let argIndex = 0; | ||
| const message = 'Warning: ' + format.replace(/%s/g, () => args[argIndex++]); |
There was a problem hiding this comment.
@gaearon Ended up just moving this line up a bit so the string replacement is consistent between the two blocks. Feel okay?
There was a problem hiding this comment.
Sorry, that was an intentional change. See my collapsed comment below (silly GitHub)
There was a problem hiding this comment.
Okay. No worries :)
|
|
||
| if (typeof console !== 'undefined') { | ||
| console.error('Warning: ' + format, ...args); | ||
| console.error(message); |
There was a problem hiding this comment.
The part about passing ...args is intentional. It lets other tools read the stack separately.
I'm only suggesting to map over them and coerce them to strings.
| } | ||
| if (typeof console !== 'undefined') { | ||
| console.error('Warning: ' + format, ...args); | ||
| const strings = args.map(item => '' + item); |
gaearon
changed the title
|
Thanks for catching this before the release! |
|
the release? |
|
We’ll need to cut a patch soon |
4 participants
I noticed this working on the Hydration fixture. It looks like we have a warning that depends on the
%sreplacement to stringify an array into a list of words. Instead it reportsArray(n):I use the 1Password browser extension, which adds some data attributes when you engage with a form.
Steps to reproduce
Using the 1Password browser extension (I've added some extra steps if you don't have this):
Expected
Warning: Extra attributes from the server: data-com.onepassword.iv, data-com.agilebits.onepassword.user-edited(Or whatever attributes you added)Actual:
Warning: Extra attributes from the server: Array(2)