rustwasm / wasm-bindgen

Facilitating high-level interactions between Wasm modules and JavaScript
https://rustwasm.github.io/docs/wasm-bindgen/
Apache License 2.0
7.78k stars 1.07k forks source link

Allow `typescript_type` on struct fields. #3953

Open prideout opened 5 months ago

prideout commented 5 months ago

Motivation

If you annotate struct fields with #[wasm_bindgen(typescript_type = "EntityId")], wasm-bindgen just ignores it, because typescript_type is meant for type declarations in extern "C".

However sometimes you might want to patch the .d.ts declaration emitted by a struct field.

For example, let's say your Rust app has a newtype called EntityId with no associated methods. It just wraps a u64. You probably do not want to expose such a simple type as a JavaScript class. Happily, wasm-bindgen already lets you do this! Here's what you can do:

impl WasmDescribe for EntityId {
  fn describe() {
    <u64 as WasmDescribe>::describe();
  }
}

impl IntoWasmAbi for EntityId {
  type Abi = u64;

  fn into_abi(self) -> Self::Abi {
    self.0 .0
  }
}

impl FromWasmAbi for EntityId {
  type Abi = u64;

  unsafe fn from_abi(js: Self::Abi) -> Self {
    Self(EntityId(js))
  }
}

This is neat because you can make a pub field in your Rust struct that has type EntityId, but it'll be exposed as a nice simple bigint. No need for a class definition and garbage collectible object, etc.

However, experienced TypeScript developers like myself often want strong typing for their id types. We do things like:

export type Brand<T, Brand extends string> = T & {
  readonly [B in Brand as `__${B}_brand`]: never;
};

export type EntityId = Brand<bigint, "entity">;

If the .d.ts file were patched so that bigint is replaced with EntityId, it would still work just fine; it's actually the same ABI. However it would have the benefits of strong typing.

Proposed Solution

I would be happy to make a PR with the above changes.

Liamolucko commented 5 months ago

I think it might be better to put the annotation on the type (i.e. EntityId) rather than the field - with the current proposal, it wouldn't be possible to override the type in a scenario like this:

#[wasm_bindgen]
impl Foo {
    #[wasm_bindgen(getter)]
    pub fn bar(&self) -> EntityId { EntityId(5) }
}

You should be wary of manually using IntoWasmAbi / FromWasmAbi / WasmDescribe like that, though. As the docs for wasm_bindgen::convert say:

This is mostly an internal module, no stability guarantees are provided. Use at your own risk.

prideout commented 5 months ago

Ah you're proposing adding it to a type that is not extern "C"? Yes I think that could work! In fact maybe it's already supported? I can try the experiment later tonight.

prideout commented 5 months ago

The problem with using this attribute on my type definition is that a wrapper class is generated, which I'm trying to avoid. The situation mentioned by @Liamolucko can be handled by creating a new extern "C" type with a different name, e.g. JsEntityId, and returning that from a Rust function. However that isn't very ergonomic because I'd like my function to have both Rust clients and JS clients.

I just discovered that my desire to avoid wrapper classes for simple newtypes is an existing issue: #1474.

In an ideal world, we could have #1474 and use typescript_type on it, in which case I agree with @Liamolucko that I would have no need for it on struct fields.