Skip to content
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

Allow for flattening of generic parameters #336

Merged
merged 4 commits into from
Jun 28, 2024
Merged

Conversation

NyxCode
Copy link
Collaborator

@NyxCode NyxCode commented Jun 27, 2024

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

  • I have followed the steps listed in the Contributing guide.
  • If necessary, I have added documentation related to the changes made.
  • I have added or updated the tests related to the changes made.

Copy link
Collaborator

@gustavo-shigueo gustavo-shigueo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@NyxCode
Copy link
Collaborator Author

NyxCode commented Jun 27, 2024

damn, that was fast! :D

@gustavo-shigueo
Copy link
Collaborator

damn, that was fast! :D

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

@NyxCode
Copy link
Collaborator Author

NyxCode commented Jun 27, 2024

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

@gustavo-shigueo
Copy link
Collaborator

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
Copy link

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
Copy link
Collaborator Author

NyxCode commented Jun 27, 2024

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
Copy link
Collaborator Author

NyxCode commented Jun 27, 2024

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

@NyxCode NyxCode merged commit af15ff9 into main Jun 28, 2024
16 checks passed
@NyxCode NyxCode deleted the flatten-generic-parameter branch June 28, 2024 00:08
@gustavo-shigueo
Copy link
Collaborator

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

@gustavo-shigueo
Copy link
Collaborator

Hey @NyxCode, since this was release on crates.io, I'll make a corresponding GitHub release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants