-
Notifications
You must be signed in to change notification settings - Fork 71
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
Handling imports of generated types #148
Handling imports of generated types #148
Conversation
Hello @dan-blank, @fabien0102 👋, Following the discussion in #135, I fiddled a bit and decided to go for this solution of Zod Schemas. |
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## main #148 +/- ##
==========================================
+ Coverage 97.33% 97.57% +0.23%
==========================================
Files 14 15 +1
Lines 676 782 +106
Branches 275 313 +38
==========================================
+ Hits 658 763 +105
- Misses 18 19 +1
|
43e6762
to
fd31327
Compare
This worked pretty well for me when testing in my project. One issue I ran into is that I use proto-loader-gen-types for generating some of my types from protos. I found that it doesn't work when there are parenthesis around the types which is how proto-loader-gen-types generates types. Example: // DoesntWork.ts
import type { BoolValue } from "google/protobuf/BoolValue";
export interface Broken {
bool: (BoolValue | null);
}
// DoesWork.ts
import type { BoolValue } from "google/protobuf/BoolValue";
export interface Works {
bool: BoolValue | null;
}
// google/protobuf/BoolValue.ts
export interface BoolValue {
'value': (boolean);
} |
Hey @anthony-dandrea, The issue you mentioned was not directly related to this PR, but it's an easy fix. You can cherry pick the commit in your branch while waiting for an approval. I'll rebase the import-related branches when it's the fix is merged. |
503db51
to
a288570
Compare
d9738e5
to
158d72d
Compare
Can you please explain the difference between this PR and #164 ? |
Hey @schiller-manuel, As far as I see, it seems that they are multiple PRs to handle import: Those 3 are not from me, and they felt like too much changes in one pass to me.
I took a different approach with #135 + #148 (the current one): 👉 #135 aimed at handling 👉 the current PR #148 adds a very simple way of handling multiple file generation, by relying on the 👉 in my mind, a third PR would have finished the work to automate filesystem browsing and generate all zod files in the right order based on How to move onI see several alternatives to move on with my 2 PRs:
Any thoughts are welcome! |
4f2d581
to
f638a8f
Compare
Hello @schiller-manuel, @fabien0102, I rebased this PR and it's (finally) ready for merging. Next step would be to build the list "on the fly" (from a folder passed as argument for instance). |
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.
LGTM!
"Next step would be to build the list "on the fly" (from a folder passed as argument for instance).
Should we wait to release a new version until this has been implemented?
I don't think I will have time in the next few weeks to deep dive on this but would love to at one point! |
Thanks a lot for your work! |
Why
Following #135 (once it's merged), this PR adds support for automatic resolution of ts-to-zod-generated schemas, based on the config file provided: if the current input file imports a type from a file referenced in the
ts-to-zod.config.js
file, then the generated file will automatically resolve the reference to the corresponding schema.This opens up the possibility of handling multiple files in one generation (by adding them to the config file first for instance).