-
Notifications
You must be signed in to change notification settings - Fork 116
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
Replace Record<K, V>
with { [key: K]: V }
#277
Conversation
ts-rs/src/lib.rs
Outdated
@@ -863,15 +863,15 @@ impl<K: TS, V: TS, H> TS for HashMap<K, V, H> { | |||
type WithoutGenerics = HashMap<Dummy, Dummy>; | |||
|
|||
fn ident() -> String { | |||
"Record".to_owned() | |||
"{ [key: any]: any }".to_owned() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually have no idea what this method should return. Maybe it should panic!
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, it should panic. This method is only ever used if the type is derive
d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's right!
We only use ident()
for constructing Dependency
, and we only do that for type which can be exported (which are those that are derived).
Awesome, thank you! While playing with this a bit, I found one potential edge-case: type A<K, V> = { [key: K]: V } which would be generated by However, So right now, I can't think of any regression this would cause. Are there any edge-cases you're concerned about? |
As far as I can tell, I think this is a minor change (e.g 8.0.1), and shouldn't break anything. |
Goal
Allow recursive
HashMap
types by changing their inlining fromRecord<K, V>
to{ [key: K]: V }
Closes #134
Changes
The
impl
block forHashMap
was alteredChecklist