Add support for BigInts and BigInt-based TypedArrays

[j-f1]

Fixes #56. ~Depends on #183~

In the JavaScriptKit package, this PR:

• adds a JSBigInt class that makes it easy to convert between BigInt and Int64/UInt64 values.
  • the Wasm-JS API assumes all i64 values are signed, so I have to hack around that behavior for UInt64 values
  • It would be nice to support standard operations (+-*/ etc) on JSBigInt instances; however, we currently don’t have a way to invoke arbitrary operators in JS from Swift, and there are no function-based equivalents of the operators that we could access instead.
• Automatically casts BigInt values to the requested integer type when using .fromJSValue() (this may cause a crash if the BigInt is outside of the bounds of the requested type, I think)
• Removes support for converting Int64 and UInt64 to JSValue

In addition, a new package (tentatively named JavaScriptKit_I64) includes:

• Support for JSTypedArray<Int64> and JSTypedArray<UInt64>
• Support for converting Int64 and UInt64 to a BigInt JSValue
  • they can still be constructed from a regular number; should I add a way to convert them to a Double value?
• Support for constructing any integer type from a BigInt JSValue
• Support for getting the int64Value and uInt64Value of a JSBigInt
  • …should this API be replaced with Int64(bigInt)/UInt64(bigInt)?
• Support for constructing a JSBigInt from an Int64 or UInt64

also, it:

• changes the integer/float conformances to ConstructibleFromJSValue to reduce boilerplate code (using a Swift 5.5 feature)
• fixes JSSymbol’s JSValue conversion APIs (oops!)
• changes the type for the internal setTimeout function to be a Double (since at least Node.js doesn’t support BigInt timeouts)
• removes JSValueKind.invalid since it is unused
• Uses an assertNever helper function in TypeScript to ensure switches are exhaustive

[github-actions[bot]]

Time Change: +279.25ms (3%)

Total Time: 8,247.5ms

Test name	Duration	Change
Object heap/Increment and decrement RC	2,714.75ms	+226.25ms (8%)	🔍

ℹ️ View Unchanged

| Test name | Duration | Change | | :--- | :---: | :---: | | Serialization/Write JavaScript number directly | 170.25ms | +0.75ms | | Serialization/Write JavaScript string directly | 175.5ms | +2.25ms (1%) | | Serialization/Swift Int to JavaScript | 2,541ms | +19.5ms (0%) | | Serialization/Swift String to JavaScript | 2,646ms | +30.5ms (1%) |

_{performance-action}

[MaxDesiatov]

This needs conflicts to be resolved after JSSymbol merge. That would reduce the diff as well, making it easier to review. Thanks!

[kateinoigakukun]

Can we have a separate module for BigInt support instead of having a new feature flag JAVASCRIPTKIT_WITHOUT_BIGINTS? I think it would be more straightforward than using ifdefs. Unlike JAVASCRIPTKIT_WITHOUT_WEAKREFS, BigInt is relatively independent of the whole runtime mechanism.

BTW symbol support should be also enabled by an explicit option? We need to define the minimum supported ECMAScript edition, and guard for new features introduced after the edition.

[j-f1]

Can we have a separate module for BigInt support instead of having a new feature flag JAVASCRIPTKIT_WITHOUT_BIGINTS?

I don’t think that would work, since it would have to be deeply integrated with JSValue. Any arbitrary JS API can take or return a BigInt, so I’m not sure how a separate module would work.

BTW symbol support should be also enabled by an explicit option? We need to define the minimum supported ECMAScript edition, and guard for new features introduced after the edition.

I don’t think so, because Symbol was added in ES2015, which is above our minimum supported language version (since we use e.g. arrow functions).

~However, I think it would make sense to make the global symbol properties implicitly unwrapped optionals (i.e. JSSymbol!) to allow people to feature-detect them if they want to.~ I already did that and forgot, haha

[kateinoigakukun]

I don’t think that would work, since it would have to be deeply integrated with JSValue.

Yes, you are right here. The current library design cannot allow this approach to work, so we need to redesign JSValue if we take the approach. I think it's a good time to redesign JSValue for extensibility and semantics mismatches.

I want to propose a change of JSValue to a protocol type.

// In JavaScriptKit module
public protocol JSValue {
  // de/serialize from/to RawJSValue
  static func fromRawJSValue(_ rawJSValue: RawJSValue) -> Self
  func toRawJSValue() -> RawJSValue
}
extension JSObject: JSValue {
  static func fromRawJSValue(_ rawJSValue: RawJSValue) -> JSObject { ... }
  ...
}
extension JSString: JSValue { ... }
// In JavaScriptBigIntSupport module
extension JSBigInt: JSValue { ... }

The problems of the current JSValue design are:

1. Supporting a new value type would be always a breaking change because JSValue is public enum. And even though JSValue is an exhaustive enum, most of the users don't expect it and don't use switch jsValue. See usecases of switch-case of JSValue in the real world

2. Since JSValue is a value type, there is a semantics mismatch in nested property assignment with JavaScript. See for more info: https://github.com/swiftwasm/JavaScriptKit/issues/132#issuecomment-881986603

3. The central declaration of all possible types makes it difficult to introduce a new value type optionally like this PR in a natural way. Controlling declaration appearance by conditional compilation flags is really hacky way and introduces many complexities. All libraries depending on JSKit have to test itself with all combination of conditional feature flags?

Of course, protocolization has some runtime performance penalties as a tradeoff.

However, I think it would make sense to make the global symbol properties implicitly unwrapped optional

It makes sense to me 👍

[j-f1]

semantics mismatch

Couldn’t this be fixed by marking the setter as nonmutating?

The central declaration of all possible types makes it difficult to introduce a new value type optionally like this PR in a natural way.

I’m wondering whether we should just enable BigInts by default, and throw at runtime when unsupported operations occur (like instantiating a new BigInt).

Supporting a new value type would be always a breaking change because JSValue is public enum

Hopefully that doesn’t happen very frequently, and I don’t agree that breaking changes are always bad.

I want to propose a change of JSValue to a protocol type.

I think this is a potential footgun. People would be incentivized to conform their own types to JSValue (and Swift doesn’t have any way to prevent that), which would not behave like they’d expect (for instance, there’s no way to hook into the importing mechanism). Right now there is a 1:1 mapping between JSValue cases and JavaScriptValueKind cases.

If a new value type is added, we need to support it in JSKit directly (including in the runtime). There is no way for a user of this package to do that without forking.

[kateinoigakukun]

@j-f1

Couldn’t this be fixed by marking the setter as nonmutating?

Wow, it looks like the exact solution for the issue. I forgot that feature 😅

I’m wondering whether we should just enable BigInts by default, and throw at runtime when unsupported operations occur (like instantiating a new BigInt).

Even though throwing errors at runtime, imports declarations having i64 in their signatures appear in the wasm module because they are unconditionally linked, so can't instantiate it on no BigInt support host.

I think this is a potential footgun.

On second thought, I agree that making it a protocol is a bad idea as you said. Then, how about just making it a struct type instead?

// In JavaScriptKit
public struct JSValue {
  @_spi(JavaScriptKitPrivate)
  public let inner: RawJSValue

  public var boolean: Bool? { ... }
  public static func boolean(_ value: Bool) -> JSValue { ... }
  ...
}

// In JavaScriptBigIntSupport
public extension JSValue {
  public var bigint: Bool? { ... }
  public static func bigint(_ value: JSBigInt) -> JSValue {
    return JSValue(inner: RawJSValue(kind: .bigint, payload1: ..., payload2: ...)
  }
}

[j-f1]

Even though throwing errors at runtime, imports declarations having i64 in their signatures appear in the wasm module because they are unconditionally linked, so can't instantiate it on no BigInt support host.

Ah, that's unfortunate. Since some browsers support BigInt without the corresponding i64 support, maybe we could have JSBigInt unconditionally enabled, and then allow disabling the two swjs_* APIs that enable translation to and from i64?

[kateinoigakukun]

Since some browsers support BigInt without the corresponding i64 support, maybe we could have JSBigInt unconditionally enabled, and then allow disabling the two swjs_* APIs that enable translation to and from i64?

Then, how can we express UInt64: ConvertibleToJSValue conformance? If no i64 support, 64bit ints can't be precisely convertible to BigInt, so we can't provide that conformance in that case. It means the protocol conformance appears conditionally based on the conditional feature flag. In addition, I think we can't provide any operation of BigInt at all without i64 support.

Again, I think it's more straightforward to enable the i64 support requirement by import BigIntSupport instead of enabling/disabling it by conditional compilation flags because of the fixed declaration set. Could you clarify the benefit of the conditional compilation flags approach?

Hopefully that doesn’t happen very frequently

I think that is an optimistic prediction. IMO, it will happen several times in the future because these new features are additive changes and are backward compatible at the JS source level.

[j-f1]

Then, how can we express UInt64: ConvertibleToJSValue conformance

with the compiler flag! If you specify WITHOUT_BIGINT, you don’t get the Int64/UInt64 conformances (since they depend on JSBigInt(_: Int64)/JSBigInt(unsigned: UInt64) which require Wasm i64 support.

Again, I think it's more straightforward to enable the i64 support requirement by import BigIntSupport instead of enabling/disabling it by conditional compilation flags because of the fixed declaration set.

Looking to the future, I can imagine that we’d drop the WITHOUT_BIGINT (and WITHOUT_WEAKREFS) flags once usage of supported browsers is high enough. Making BigInts be a separate module means that we will be ~forever beholden to supporting the BigIntSupport module.

I think that is an optimistic prediction. IMO, it will happen several times in the future because these new features are additive changes and are backward compatible at the JS source level.

Yes, it will definitely happen in the future; however, of all of the (stage 0–3) proposals, there seems to be only one that adds new typeof types (records & tuples). I really don’t expect there to be more than a couple new types per decade.

[kateinoigakukun]

WITHOUT_BIGINT

At first, I think it is unacceptable for users to add new restrictions on applications that don't use BigInt. The new restrictions will be added by just upgrading JavaScriptKit. I recommend that it should be disabled by default.

In fact, I have some regrets about making WeakRefs enabled by default. However, since it has many benefit to all users of the core closure feature, the new restriction was a trade-off for ease of use. But in the case of BigInt, the benefit of default enable is very limited.

Making BigInts be a separate module means that we will be ~forever beholden to supporting the BigIntSupport module.

We can merge the BigIntSupport into the main JavaScriptKit module after the BigInt support becomes sufficiently widespread. We can also keep the BigIntSupport module as an empty module for compatibility, but even in this case, don't need to maintain it forever.

[MaxDesiatov]

Making BigInts be a separate module means that we will be ~forever beholden to supporting the BigIntSupport module.

We can merge the BigIntSupport into the main JavaScriptKit module after the BigInt support becomes sufficiently widespread. We can also keep the BigIntSupport module as an empty module for compatibility, but even in this case, don't need to maintain it forever.

+1 here for the eventually empty module. I personally find compiler flags a bit hard to use with Swift and feel like module imports would be more approachable. There may be a period of time where module import is required until we see some solid browser adoption, but then we can deprecate the module and keep it as empty for backwards compatibility. That's assuming there are no other issues with the separate module strategy.

[j-f1]

The main downsides I see are:

• everyone’s runtime would still have to include BigInt support
• and the main JSKit Swift module would have to know about BigInts in order to support optionally receiving them from the runtime (how would it be able to instantiate a JSBigInt object?)
• whatever approach we go with for enabling additional “cases” to be added to whatever non-enum type JSValue ends up as will be theoretically accessible by any package. This would be very confusing because only the main JSKit package and the BigInt support package would actually be able to properly add cases to JSValue (since the RawJSValue objects passed over from JS are immediately parsed into JSValues inside of JSKit before they’re exposed to any user code)

We can merge the BigIntSupport into the main JavaScriptKit module after the BigInt support becomes sufficiently widespread

What is a good metric for “sufficiently widespread?” BigInt support itself is around 89% right now, and I could find no statistics (either by usage or by browser version) for the i64-to-BigInt translation support. I worry that this means we’ll leave this extra module requirement in place for longer than necessary, complicating the user story.

[kateinoigakukun]

@j-f1

• everyone’s runtime would still have to include BigInt support

1. Unlike having i64 imports in Wasm binary, having BigInt support in runtime doesn't add any new restriction. What are your concerns?

• and the main JSKit Swift module would have to know about BigInts in order to support optionally receiving them from the runtime (how would it be able to instantiate a JSBigInt object?)

2. The main JSKit module needs to know not the JSBigInt type itself but only how to convert it to/from RawJSValue (JSValue).

• whatever approach we go with for enabling additional “cases” to be added to whatever non-enum type JSValue ends up as will be theoretically accessible by any package.

3. That's theoretically possible, but normally impossible because those semi-public APIs can be hidden by @_spi.

• I worry that this means we’ll leave this extra module requirement in place for longer than necessary, complicating the user story.

4. The conditional compilation flag strategy will also face the same problem of the "good metric" because it needs to remove the WITH/WITHOUT_BIGINT. So this point doesn't affect the decision of architecture strategy I think. The user story complication discussion is not a part of the architecture decision but a part of the default enable/disable of BigInt support. So I want to confirm that you agree on the direction of disabling the BigInt support by default.

Overall, I think we need to have an implementation of the separate module strategy for further discussion. Let me prepare it this week.

[j-f1]

1. My concern is that the functionality supporting BigInts would be split up between two separate packages.
2. Right, but in order to do that, it would need to be able to access the JSBigInt type. So that would have to be defined inside of JavaScriptKit.
3. interesting! That could definitely provide a way to move just the i64-based parts of the BigInt API (and other potential future APIs, I guess)…

How does this sound?

• define JSBigInt inside of JavaScriptKit, allowing it to be natively used with JSValue.
• JSBigInt would only include the APIs that allow interacting with BigInts as strings (i.e. init(_: String) [which I need to implement] and description), as well as APIs that interact with BigInts opaquely (like clamped(bitSize: Int, signed: Bool))
• There would be a new library (calling this JSKitI64 for now but I put literally zero thought into that name). It would:
  • extend JSBigInt to allow initializing it with Int64/UInt64
  • allow converting JSBigInt values to Int64/UInt64
  • Enable support for JSTypedArray over Int64/UInt64
• Finally, there would be an _CJSKitI64 package providing the bindings to the i64-based wasm exports.

We could declare the JSObject(id:) initializer (plus JSObject/id) as SPI, allowing JSKitI64 to interact with the bridge itself.

4. …now that I’ve written that I’m much more comfortable with not including the i64 stuff by default, if we go with that approach or something similar.

[kateinoigakukun]

1. My concern is that the functionality supporting BigInts would be split up between two separate packages.

Ok, now I understand.

2. Right, but in order to do that, it would need to be able to access the JSBigInt type. So that would have to be defined inside of JavaScriptKit.

Technically, it can be avoidable by protocol abstraction, I think... (but don't need it if we go the newly proposed strategy)

3. That could definitely provide a way to move just the i64-based parts of the BigInt API (and other potential future APIs, I guess)

It looks like a feasible and reasonable approach to isolate only i64-to-BigInt related things. I have no objection to this approach. Let's go ahead!

4. …now that I’ve written that I’m much more comfortable with not including the i64 stuff by default, if we go with that approach or something similar.

😄

[github-actions[bot]]

Time Change: -39ms (0%)

Total Time: 19,157ms

Test name	Duration	Change
Serialization/Write JavaScript number directly	454ms	+36ms (7%)	🔍
Serialization/Write JavaScript string directly	471ms	-28ms (5%)	✅

View Unchanged

| Test name | Duration | Change | | :--- | :---: | :---: | | Serialization/Swift Int to JavaScript | 6,142ms | +26ms (0%) | | Serialization/Swift String to JavaScript | 6,437ms | +144ms (2%) | | Object heap/Increment and decrement RC | 5,652ms | -216ms (3%) |

[j-f1]

I would especially appreciate feedback on the new package name (JavaScriptKit_I64)

[kateinoigakukun]

Module name candidates:

• JavaScriptKitBigIntIntegrationSupport
• JavaScriptBigIntIntegrationSupport
• JavaScriptKitBigIntSupport
• JavaScriptBigIntSupport

Note: The proposal name was JS-BigInt-integration

[MaxDesiatov]

How's about JavaScriptKitI64Support?

[MaxDesiatov]

Ah, sorry, just saw your link to the proposal. 😅 Then I personally vote for JavaScriptBigIntSupport, this seems the least verbose, but still obvious enough for me.

[j-f1]

For JavaScriptBigIntSupport, my concern is that people won’t think BigInts are supported at all by vanilla JSKit, but maybe that isn’t such a big deal and we should just be clear in the README in which cases you need it.

[MaxDesiatov]

Is this ready for review by any chance?

[j-f1]

Yep!

[kateinoigakukun]

I prefer JavaScriptBigIntSupport with README note. Other things are LGTM

[MaxDesiatov]

I wonder if we have to disable Safari WasmTransformer pass when JavaScriptBigIntSupport target is linked? I'm currently seeing WebAssembly.Module doesn't validate: control flow returns with unexpected type. I32 is not a I64, in function at index 89699 error when playing with https://github.com/swiftwasm/DOMKit/pull/18

[MaxDesiatov]

I can also confirm that the issue is not reproducible with 95d0c4cd78b48ffc7e19c618d57c3244917be09a, which is prior to the merge commit for this PR.

[j-f1]

That sounds good! Would we need to do anything on the JS side to correctly handle the previous APIs that now return BigInts?

[MaxDesiatov]

I haven't verified yet if disabling the pass fixes the issue with the latest JSKit main, this is only my current theory that could explain the breakage.