closure-compiler
closure-compiler copied to clipboard
I think parseInt radix should be optional
I understand why this is. But if feels like a very old legacy issue...
https://github.com/google/closure-compiler/blob/b29d09f56b56e5b3ea78ba0317ae579b837fd821/externs/es3.js#L289-L300
Nowdays all browser use radix 10 as default and has done so for a long time. And the likelihood of someone starting the number with 0 and using IE8 is very slim. Many have stop supporting old browsers
* @param {number=} base
Created Google internal issue http://b/124375924
parseInt still infers the radix from the string contents when one is not provided.
In Chrome:
parseInt('0x20') === 32
parseInt('0x20', 10) === 0
This is a huge footgun if you are parsing user provided content. The only behavior that has changed is we no longer have to worry about an inferred octal radix.
Thought it would only be a footgun for things like padded zeros like 010
0x91 is clearly a hex value. I probably would always would want to do both octal or hex parsing then.
How about this:
If you really want to parse octal or hex based on the leader, then pass
undefinedas the base
if the 2nd argument is undefined can the compiler then remove it instead of changing it to void 0?
Still would prefer it to be optional
0x91is clearly a hex value
Yes it is - and it's 100% clear when it's a string literal like that. The problem comes when it's not a string literal:
const value = document.getElementById('myinput').value;
console.log(parseInt(value)); // What's my radix?
That would not be a problem if the input type is a number cuz that prevents the user from enter a x
as for a number input I would also have done input.valueAsNumber cuz 1e2 could be valid number too. and parsing 1e2 is not the same as parseInt('1e2') then you need to use Number('1e2')
<input id="myinput" type="number" min="0" max="100" step="1">
const value = document.getElementById('myinput').valueAsNumber;
console.log(value); // What's my radix?
And when the input is invalid like 1e the input value is empty "" and valueAsNumber is NaN
And if you want to you can always use the pattern attribute to restrict it even further
It would also be less error prone since you can first validate the hole form or just the input before proceeding
I realize it feels annoying to be forced to supply the second argument to parseInt(), but it avoids a footgun and leads to clearer (more explicit about what it's doing) code.
I don't think we're likely to change this anytime soon.
It is a barrier to compiling code not written with closure-compiler in mind, though, and longer term we'd like to remove such barriers. Of those barriers, this isn't the most important one, so again, probably not changing soon. Sorry.
Just noticed this issue, had created one at https://github.com/google/closure-compiler/issues/3548.
I realize it feels annoying to be forced to supply the second argument to
parseInt()
In our case, it is not as much the need to supply a second argument to parseInt(), but that when we do pass parseInt(..., undefined);, it results in Closure not optimizing this second argument away, but it generates code parseInt(..., void 0); in the output. Closure insists on adhering to a convention, but then does a sloppy job of minifying code when one does adopt that convention. Now that is what feels annoying, if something.
Maybe some interested party could add a Peephole pass to remove an unnecessary second argument to parseInt. I don't think our core team is likely to prioritize doing that, though.
Thinking more general, would it be a sensible optimization to always collapse ‘foo(1,2,undefined,undefined)’ into ‘foo(1,2)’? (there may be a need to disable such optimization in some cases, e.g. WebGL teximage2D function, but most of the time that feels like a safe optimization?)
Thinking more general, would it be a sensible optimization to always collapse ‘foo(1,2,undefined,undefined)’ into ‘foo(1,2)’? (there may be a need to disable such optimization in some cases, e.g. WebGL teximage2D function, but most of the time that feels like a safe optimization?)
Many non-native functions and methods distinguish the two cases; also the Number function
> Number(undefined)
< NaN
> Number()
< 0
the Math.max function
> Math.max(1, 2, 3, undefined)
< NaN
> Math.max(1, 2, 3)
< 3
and the Array.prototype.reduce method
> [].reduce(function () {}, undefined)
< undefined
> [].reduce(function () {})
< Uncaught TypeError: Reduce of empty array with no initial value
It seems like radix is now optional: https://github.com/google/closure-compiler/blob/71785a7e3e04be489510d4695870c7491af17c3f/externs/es6.js#L1942-L1953
I'm not sure when/how it changed though. Maybe this can be closed now?
No, it's still required. If it were optional, the declaration would look like this.
* @param {number=} radix