-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Reconsider usage of bigint in generated JS code #4246
Comments
I think the desire was to make an application only fail at runtime so if the usage of Do we actually need
I agree that a policy around this should be established. Unfortunately I have no idea where we stand or what we should do about this. A more informed opinion from somebody actually knowledgeable about JS/TS and the ecosystem would be much appreciated. |
That might be the case. I did some digging to find how how Safari 13 even supports the WebAssembly JS API when it doesn't support bigint. The answer is that an old version of the JS API spec (wayback) just doesn't support Note that this only affects the JS API of WebAssembly. You can use
On a related note, this also means that changing the WASM ABI of any type to use
Maybe we could start by documenting which browser versions are supported? The browser support page doesn't say. Going back to safari, they added wasm support in Safari 11 (released Sep 2017), but thanks to Luckily, there's this handy page, so we "just" have to pick out what's relevant to WBG. Knowing exactly what browsers we currently support would probably also be good to know to inform such a policy. |
Not trying to hijack this issue, but worth noting despite claimed support Safari 15 has issues with
|
This isn't part of Wasm v1.0, its a proposal called "JS BigInt to Wasm i64 Integration". You can find it on the Wasm feature page as well.
Uff, true!
We shouldn't confuse
Indeed, Safari's v15 implementation of the reference type proposal was faulty (don't remember the details), we should pretend that support was added in Safari v16. |
Okay, so taking this all together: For documentation, we need one table that explains which stable Rust versions with the target We might have to amend the Rust table in the future for other target and nightly tricks, but at least the target feature browser support table should be rather unchanging.
I think proposals that are purely about the JS API raise an interesting issue regarding browser support. What #3164 established is essentially partial browser support. From my understanding, things like target feature (e.g. The question is: should this "all or nothing" mantra for supporting certain browser versions apply to all WASM features, or should we support partial browser support? On that note: are there other JS-API-focused WASM features for which partial support would also apply? I quickly scanned through the list of features, but couldn't find anything that sounded like it. |
What should actually be required of us is to specify which browsers we support considering no target features are enabled and note that we don't control what target features are enabled by the Rust compiler and it does already enable some target features by default. Anything more, including what you wrote, would be very appreciated, but isn't absolutely necessary.
I would like to see some substantial use-case for partial browser support, otherwise I remain unconvinced. But its already here, so we can only change it with the next breaking change. There are some cases where this is required however, e.g. WBG does some
I'm not aware of one. |
Summary
#3164 set the precedent that WBG should not emit bigint literals in generated JS code, and I wanted to ask whether this could be reconsidered and #3164 be reverted.
I'm asking for this, because (1) this is a blocker for #4222 in its current form, and (2) #3164 didn't actually solve anything.
What #3164 tried to achieve
The motivation behind #3164 was to solve a specific esbuild error. Here's the error:
This error means that esbuild wasn't able to convert the generated JS code of WBG into JS code supported by all configured target environments. In particular, safari 13 doesn't support bigint. So esbuild correctly errors out, as it cannot fulfill the request to support bigints (specifically bigint literals) in safari 13.
The "solution" #3164 implemented was to replace the bigint literal
0n
withBigInt(0)
, so the constant is created at runtime instead.BigInt(0)
is simple JS and so the transpiler doesn't need to do anything and so won't error.However, the approach taken in #3164 does nothing to fix the compatibility issue. Even
BigInt(0)
isn't supported in safari 13. The example function in the issue will still not work in safari 13. The only thing that changed is that this compatibility problem is no longer detected by esbuild. #3164 did not fix the underlying issue, it hid it. Even the other code example given in #3164 doesn't work on safari 13, it's just that esbuild didn't detect that and so didn't emit an error.There's also no point in emitting
BigInt(value)
for older browsers, because it will always fail at runtime or (if polyfilled) be unusable. As the author of esbuild explains in this issue, polyfilling bigint is pretty much impossible. And even if you somehow hack together a spotty polyfill, it wouldn't help. The WebAssembly JS API uses bigint fori64
, and a polyfill wouldn't be compatible with that.So all #3164 did was to make esbuild misrepresent which target environments are actually supported.
IMO, users that encounter this error should either:
If users still insist on not using bigint literals, then I think they should use a transpiler for that. 64-bit integer types require bigint, and I don't think it's WBG's job to hide this fact.
Why this blocks support for
u128
(#4222)To support
u128
with the ABI implemented in #4222 (pass oneu128
as twou64
), we need to perform bit-shifts on bigints. This operation is impossible to polyfill, so #4222 requires that bigint be supported.However, #3164 sets the precedent that bigint doesn't need to be supported. The only ABI for
u128
that would be compatible with a hypothetical bigint polyfill is the same ABI asjs_sys::BitInt
: a string of decimal digits. For performance reasons, I would like to avoid this.ES version policy
Similar to a MSRV, it would probably be a good idea to have a similar policy for ES versions. Right now, I'm not sure which features I am allowed to use in generated JS code.
We extensively use ES2015 features, so I would assume that the minimum supported ES version is at least ES2015. However, we also use bigints which are ES2020, but we also apparently don't target ES2020 because of #3164. So what ES version do we target?
The text was updated successfully, but these errors were encountered: