dbeckwith / rust-typescript-type-def

Generate TypeScript type definitions for Rust types.
30 stars 13 forks source link

Support for third-party crates #32

Open WesleyAC opened 7 months ago

WesleyAC commented 7 months ago

Because of the Rust orphan rule, it's difficult to use this crate to define types for other libraries. Doing so requires using newtypes, which can be cumbersome. It would be ideal to make this easier. I see two potential solutions:

Potentially doing both could be good, so that people can avoid the compile-time hit from a procmacro, while still supporting more niche libraries without having to upstream a feature flag first.

Is that a path forward that seems reasonable to you? I'm specifically opening this because I want support for the types in webauthn_rs, but I imagine that this is somewhat widely applicable.

WesleyAC commented 7 months ago

Related: #2, #28 — seems like this comes up for uuid::Uuid a lot, so adding a feature to support that seems quite reasonable to me.

dbeckwith commented 7 months ago

Yeah it's tough to figure out the best way to do this. On the one hand, I don't want to have to write TypeDef impls for every other library out there, so I'm tempted to say that those libraries should be providing their own TypeDef impls for their own types, much like many do with serde (typically behind a crate feature flag). But I think that only really applies because serde is so well-known. The library authors could easily turn that around and be like "I've never heard of typescript-type-def, why should I bother writing impls for every trait-defining library out there?" When at least one of the type or the trait are well-known, it's fairly obvious where the impl should go, but where's the middle ground when both of them are obscure and the orphan rule requires that the impl go in either the library defining the type or the library defining the trait? Not sure there's really a good answer to this in general.

Despite all that, I think your first suggestion of providing TypeDef impls for common libraries here under feature flags seems like the most practical.

It's not possible to "implement a procmacro to automatically write the TypeDef implementation for anything that implements Serialize or Deserialize" because the type definition needs to know how the type is serialized, which comes from the code inside the Serialize impl, which you can't inspect in a proc-macro. Even if you could, analyzing that serialization code to determine what data type is produced when the format is JSON seems intractable. When I write TypeDef impls for foreign types, I have to either look at a real value that was serialized to see what JSON type it is or read the impl Serialize source code and analyze it myself to figure out what type is produced (not hard for a human who's good at Rust, hard for a Rust proc-macro).

In summary, I think I'll take a look at what common types are supported by other libraries like rusqlite that need to serialize things and add feature-flagged impls for those.

dbeckwith commented 7 months ago

Another thought: you mentioned that the newtype pattern is "cumbersome", and although I think you're not wrong due to the fact that it requires a bit of boilerplate to re-implement all the behavior you need on the newtype, I think it's very powerful in giving you complete control over how your newtype is serialized and Typescript-typed and all that. In large projects I think this is usually the way to go when you have specific requirements. If you decide to use newtypes, the feature mentioned in the other issues you linked should help with easily implementing TypeDef on those.

jesseditson commented 3 weeks ago

FWIW I've solved for this in my own dependency tree (I have a private lib that depends on an OSS one) by using the typescript flag - you can see some of the implementation code here:

https://github.com/search?q=repo:jesseditson/archival%20typescript&type=code

The cargo package and the related definition-emitting tests are also behind this flag, and it's quite clear. I think it's also a somewhat fair assumption (although not true in all cases, e.g. clients that generate ts stubs for node.js API endpoints) that the libs that will be doing this will support the wasm32-unknown-unknown target - so perhaps that's an interesting datapoint for narrowing what the extent of support looks like.

In any case - I could see it pretty easily being a community-level support task (similar to serde support) that resolves this, we just would need to generally agree on:

  1. what the feature is called
  2. Possibly, where to emit d.ts files and what to call them

I'd be more than happy to conform to a community standard for either/both answers if and when such a standard exists. Perhaps a solution could be just to publish in this lib what those answers should typically be, and we can do the normalization step via PRs on various repos when we find that they diverge?

jesseditson commented 3 weeks ago

One main thing I could see as a stumbling block here is recursive types and mismatches on JSON serialization - in my experience these are the things that I've had to make internal decisions on that I wouldn't necessarily feel are obvious or universally acceptable.

The main serialization stumbling block that I imagine being confusing is that HashMap becomes Map in the default wasm_bindgen serializer and this library emits a Record type - so I've had to change the serializer defaults when emitting code. I think this is pretty isolated to implementation code so probably doesn't actually matter, but I would anticipate a lot of people hitting that when writing interop in wasm targets.

Recursive types are more of a library-level issue, here's an example how I currently solve for that, which is obviously quite brittle and verbose:

https://github.com/jesseditson/archival/blob/65c4421163a793a79f117ddc938cce766d4ceaf7/src/fields/field_type.rs#L39-L70

This issue is just https://github.com/dbeckwith/rust-typescript-type-def/issues/18, so it's already tracked, but probably would be something that would need to be resolved before a universal standard could be reasonable.