Aleph-Alpha / ts-rs

Generate TypeScript bindings from Rust types
MIT License
989 stars 99 forks source link

Allow for flattening of generic parameters #336

Closed NyxCode closed 1 week ago

NyxCode commented 1 week ago

Goal

Generic parameters currently can't be flattened:

#[derive(TS)]
#[ts(export)]
struct Item<D> {
    id: String,
    #[ts(flatten)]
    inner: D,
}

Serde happily accepts this, and we should too.

The three test cases work, and they are compatible with what serde does with them. There may be edge-cases I currently can't think of, though.
However, this change definitely wont break any existing code.

Changes

Tiny change: we just implement fn inline_flattened() for the dummy generic types we generate, instead of throwing an error.

Checklist

NyxCode commented 1 week ago

damn, that was fast! :D

gustavo-shigueo commented 1 week ago

damn, that was fast! :D

I literally logged into GitHub as the PR was being created lmao

NyxCode commented 1 week ago

I think this is what @murl-digital wanted in #335, though it might be unrelated if I misunderstood.

gustavo-shigueo commented 1 week ago

I think this is what @murl-digital wanted in #335, though it might be unrelated if I misunderstood.

Looking thorugh the issue, I think this is what they need as well

murl-digital commented 1 week ago

I think this is what @murl-digital wanted in #335, though it might be unrelated if I misunderstood.

i clarified what i wanted in the issue just in case, but i think this would fit my usecase (i'll have to check, though)

one potential issue is handling trait bounds, but i think i can just use a concrete type or something.

NyxCode commented 1 week ago

one potential issue is handling trait bounds, but i think i can just use a concrete type or something.

Since your definition of struct Item doesn't make use of the fact that D: Document, I'd consider if you can just remove that bound. Normally, you only need such a bound on a struct definition if you're using an associated type on Document.
Even std::hashmap::HashMap<K, V> doesn't constrain K or V, but only the impl<K: Hash, V> HashMap<K, V> { ... } block does.

NyxCode commented 1 week ago

Hm, maybe this should be a patch release instead..

gustavo-shigueo commented 1 week ago

Hm, maybe this should be a patch release instead..

This would fit very well in a 9.1.0, maybe we could even squeeze #316 in with it