-
-
Notifications
You must be signed in to change notification settings - Fork 882
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
Upgrading ts_rs to 10.0.0 #5163
Conversation
fn transparent() -> bool { | ||
true | ||
} | ||
} |
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.
This one's no longer necessary, as ts-rs now has a URL derive impl.
@@ -39,19 +40,3 @@ impl From<String> for SensitiveString { | |||
SensitiveString(t) | |||
} | |||
} | |||
|
|||
#[cfg(feature = "full")] | |||
impl TS for SensitiveString { |
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.
Not needed anymore either.
FederationError { | ||
#[cfg_attr(feature = "full", ts(optional))] | ||
error: Option<FederationError>, | ||
}, |
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.
Had to make these named structs, because tuple structs don't support the derives.
This is really extremely verbose now. As these |
I agree I really don't like it, but I don't think there's a way around it, and its what ts-rs is recommending now (see that linked thread above). It could be done with some post-generated regex, but my regex skills aren't good enough to do inline conversions from |
I assume there would be some existing tool which can rewrite this. |
I looked into eslints, and there doesn't seem to be a good way to enforce it. I'll test some other things out though. |
I checked and there's really no way around it. This is just something we have to accept with ts-rs. |
Was able to remove some custom derives for this also.
Unfortunately this does require adding
ts(optional)
directives for everyOption
type that derivests::TS
. Context Aleph-Alpha/ts-rs#364I went through the generated types and verified that none output
null |
. I've added a woodpecker test for this also.