google / elemental2

Type checked access to browser APIs for Java code.
Apache License 2.0
151 stars 38 forks source link

JsNumber.toFixed either needs more overloads or needs @DoNotAutobox #129

Closed niloc132 closed 1 week ago

niloc132 commented 5 years ago

Discovered via https://stackoverflow.com/questions/57244451/bug-jsnumber-tofixed-returns-different-values-in-superdev-and-js

When calling JsNumber.toFixed with an int value (as would seem to be the sanest option), the int is boxed to Integer, which isn't treated as a number by the underlying ToInteger -> ToNumber -> ToPrimitive (see https://www.ecma-international.org/ecma-262/5.1/#sec-15.7.4.5 etc).

Adding elemental2.core.JsNumber.toFixed.digits to core's integer-entities.txt only results in this logged message at build time:

SEVERE: Unused integer entities: [elemental2.core.JsNumber.toFixed.digits]

Some proposed solutions that seem to make sense on face value (though I haven't tried to fully implement any of them yet):

jDramaix commented 5 years ago

I think we should fix the extern file for this method. It should accept a Number and not an Object.

Then we can use interger-entities.txt to turn the parameter to int.

zbynek commented 8 months ago

PR in Closure created

zbynek commented 4 months ago

@niloc132 with the Closure PR merged, the signature in next Elemental release should be toFixed(double), which is consistent with existing toPrecision(double) and toExponential(double). Both of those methods work OK with integer argument both in compiled and super-dev mode. Since (1/3).toFixed(5.5) is technically valid JS, maybe declaring digits to be an integer is not needed and this issue can be closed?

zbynek commented 1 week ago

Fixed in 1.2.3.