Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
niloc132 opened this issue Jul 31, 2019 · 2 comments
Open

Comments

@niloc132
Copy link
Contributor

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):

  • In the same way as integer-entities.txt, provide a way to mark some parameter as @DoNotAutobox, so that if truly any value is allowed, an int/float/etc is passed directly to JS without boxing. It might be reasonable to have this be the default, since native methods aren't likely to specially handle java.lang.Integer, etc.
  • In the extern diffs, specify that this method might take Object, or might take Number, allowing the regular integer-entities.txt functionality to work here.
  • Permit jsinterop.generator.visitor.IntegerEntitiesConverter in google/jsinterop-generator to act on any supertype of Double/double, so that overloads are generated automatically.
@jDramaix
Copy link
Member

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
Copy link
Contributor

zbynek commented Feb 20, 2024

PR in Closure created

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants