-
Notifications
You must be signed in to change notification settings - Fork 67
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
asIntN and asUintN should both throw RangeError when n
is less than zero
#39
Comments
n
is less than zeron
is less than zero
Thanks for the report. I'm actually not sure we want to fix this. There are several places where JSBI kind of assumes that you know what you're doing, and in the interest of performance doesn't check for nonsensical inputs. I'm open to opinions here. Is the above a reasonable implementation policy? Or is it more useful to be 100% error-checking compatible with native BigInts for nonsensical, exception-throwing cases, even if that costs performance and bundle size? |
I agree we should be consistent and either add validation everywhere (which would be bad for performance) or continue to assume that the user knows what they're doing. Perhaps the way forward here is to document this philosophy a little bit more clearly, and provide a list of examples such as the one in OP's post. WDYT? |
These would be wonderful lol! In my experience (or more like lack of experience with BigInt stuff) I am having a hard time comparing this JSBI and the four below:
As also discussed in web3/web3.js#2171 |
Sure, we can add more documentation and examples. @joshxyzhimself , would you like to draft something that you think would be useful? Regarding JSBI's general motivation, we've tried to sum that up in the existing README.md. In short, it's meant to be a transitionary solution until native BigInt support is available in all browsers, which significantly informs its design, e.g. JSBI intentionally doesn't support anything that can't be easily mapped onto native BigInt behavior. If there is demand for a fully type-checked version, I would be fine with adding a layer that does that (e.g. duplicate all |
JSBI.BigInt('-0o0') does not throw |
With native BigInt,
With jsbi,
https://runkit.com/embed/tjflr2xtmtou
The text was updated successfully, but these errors were encountered: