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

Option to rename source of TS trait #274

Merged
merged 11 commits into from
Mar 20, 2024

Conversation

aumetra
Copy link
Contributor

@aumetra aumetra commented Mar 19, 2024

Goal

The goal of this PR is to provide an option which allows for re-exports of the ts_rs::TS trait.
Similar to serde's #[serde(crate = "other_serde")] attribute.

Motivation is us evaluating ts-rs for generating TypeScript types directly without an intermediate JSON Schema step. We generate the derive annotations via an own procedural macro, which is why we need to re-export the crate.

Changes

This PR introduces the new crate_rename attribute to the derive macro.
The invocation looks similar to this:

#[derive(TS)]
#[ts(crate_rename = "::my_crate::ts_rs")]

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.

(Will add the documentation if this is actually a feature that would be accepted)

@aumetra aumetra marked this pull request as draft March 19, 2024 17:17
@aumetra
Copy link
Contributor Author

aumetra commented Mar 19, 2024

This PR is missing like 90% of all renames. So far from complete.
Will complete and clean it up once it is clear whether this would be accepted.

macros/src/attr/enum.rs Outdated Show resolved Hide resolved
macros/src/attr/struct.rs Outdated Show resolved Hide resolved
@escritorio-gustavo
Copy link
Contributor

Hey, thanks for the PR! I've left a couple of changes I'd like to see here, apart from those, I don't see an issue with adding this feature.

@aumetra
Copy link
Contributor Author

aumetra commented Mar 19, 2024

I've left a couple of changes I'd like to see here, apart from those, I don't see an issue with adding this feature

Great! Then I will check on how to sync the same crate name across all the different code paths and will take care of your requested changes.

@escritorio-gustavo
Copy link
Contributor

Great! Then I will check on how to sync the same crate name across all the different code paths

A few days ago I added a commit to add :: before the ts_rs path. Check out the changes in #267 to see all the places that will need updating

@NyxCode
Copy link
Collaborator

NyxCode commented Mar 19, 2024

I'm open to adding this as well. 👍

@NyxCode
Copy link
Collaborator

NyxCode commented Mar 19, 2024

Interestingly, this would come in handy in #276, allowing to #[derive(TS)] within ts-rs by using #[ts(crate = "")]

@aumetra
Copy link
Contributor Author

aumetra commented Mar 20, 2024

Oh, you got there before I did. Cool!
For a test, do you think introducing a separate test crate, such as ts-test-rename would be sensible and then importing it as a dependency via

ts-rename = { package = "ts-rs", path = "../ts-rs" }

That way you can be 100% sure the rename code works. The test I made is a bit.. lackluster

@escritorio-gustavo
Copy link
Contributor

escritorio-gustavo commented Mar 20, 2024

Oh, you got there before I did. Cool! For a test, do you think introducing a separate test crate, such as ts-test-rename would be sensible and then importing it as a dependency via

ts-rename = { package = "ts-rs", path = "../ts-rs" }

That way you can be 100% sure the rename code works. The test I made is a bit.. lackluster

This is a very good idea! You can do that inside the e2e directory. Checkout e2e/workspace and e2e/dependencies for examples of how to set this up

@aumetra aumetra marked this pull request as ready for review March 20, 2024 14:48
@NyxCode
Copy link
Collaborator

NyxCode commented Mar 20, 2024

Great work guys, awesome!
I've taken the liberty to make crate_rename private and replace its use with a getter, crate_rename() -> Path. I didn't love that we had these unwrap()s everywhere.

@NyxCode
Copy link
Collaborator

NyxCode commented Mar 20, 2024

Interestingly, this would come in handy in #276, allowing to #[derive(TS)] within ts-rs by using #[ts(crate = "")]

While this doesn't allow #[ts(crate = "")], #[ts(crate = "crate")] works perfectly fine.

NyxCode added a commit that referenced this pull request Mar 20, 2024
@escritorio-gustavo
Copy link
Contributor

Great work guys, awesome! I've taken the liberty to make crate_rename private and replace its use with a getter, crate_rename() -> Path. I didn't love that we had these unwrap()s everywhere.

Nice! I didn't like having so many unwraps either, glad to see them gone!

@escritorio-gustavo escritorio-gustavo merged commit 77f224e into Aleph-Alpha:main Mar 20, 2024
7 checks passed
@aumetra aumetra deleted the rename-crate branch March 20, 2024 17:23
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