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

Handling imports of generated types #148

Merged
merged 44 commits into from Feb 6, 2024

Conversation

tvillaren
Copy link
Collaborator

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.

//ts-to-zod.config.js
/**
 * ts-to-zod configuration.
 *
 * @type {import("./src/config").TsToZodConfig}
 */
module.exports = [
  {
    name: "villain",
    input: "src/villain.ts",
    output: "src/generated/villain.zod.ts",
    getSchemaName: (id) => `z${id}`
  },
   {
    name: "hero",
    input: "src/example/heros.ts",
    output: "src/example/heros.zod.ts",
  },
];

// heros.ts (input)
import { Villain } from "../villain";

export interface Hero {
  name: string;
  nemesis: Villain;
}

// heros.zod.ts (output)
import { zVillain } from "../generated/villain.zod";

export const heroSchema = z.object({
  name: z.string(),
  nemesis: zVillain;
});

This opens up the possibility of handling multiple files in one generation (by adding them to the config file first for instance).

@tvillaren
Copy link
Collaborator Author

Hello @dan-blank, @fabien0102 👋,

Following the discussion in #135, I fiddled a bit and decided to go for this solution of Zod Schemas.
Happy to discuss this.

@codecov-commenter
Copy link

codecov-commenter commented Jul 24, 2023

Codecov Report

Merging #148 (d9738e5) into main (8d3b297) will increase coverage by 0.23%.
The diff coverage is 98.71%.

❗ 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     
Files Changed Coverage Δ
src/utils/fixOptional.ts 95.00% <95.00%> (ø)
src/core/generate.ts 97.20% <100.00%> (+1.08%) ⬆️
src/core/generateZodSchema.ts 96.25% <100.00%> (+0.05%) ⬆️
src/core/validateGeneratedTypes.ts 97.61% <100.00%> (ø)
src/utils/importHandling.ts 100.00% <100.00%> (ø)
src/utils/traverseTypes.ts 100.00% <100.00%> (ø)

@anthony-dandrea
Copy link

anthony-dandrea commented Aug 3, 2023

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);
}

@tvillaren
Copy link
Collaborator Author

Hey @anthony-dandrea,

The issue you mentioned was not directly related to this PR, but it's an easy fix.
The fix is there: #152

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.

@tvillaren tvillaren force-pushed the feat-handle-zod-imports branch 2 times, most recently from 503db51 to a288570 Compare September 8, 2023 15:56
@schiller-manuel
Copy link
Collaborator

Can you please explain the difference between this PR and #164 ?
Would love to get all those PRs merged.

@tvillaren
Copy link
Collaborator Author

tvillaren commented Feb 2, 2024

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 import statement in one file only. It does work, as I've been using this for months now on my company project.
However, due to validation errors that I haven't been able to fix (see #186), it would be kind of a breaking change in the current state, as non-zod imported types would only pass validation in most cases (the generated zod validators usually work in that case).

👉 the current PR #148 adds a very simple way of handling multiple file generation, by relying on the ts-to-zod.config.js configuration file to describe all the files in the "ZOD filesystem". I did that as a first step, to rely on existing bricks and not do a big bang refactoring like other approaches above.
Again, this work in my project. I haven't rebased to the latest ts-to-zod though because of the validation errors in #135

👉 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 import statements and all. Haven't looked at it yet (but the need in my company project is going to arise to if we can move on with the rest, it would be great 😉)

How to move on

I see several alternatives to move on with my 2 PRs:

  1. Find a solution to external imports validation issues, as discussed on [Project] Issue while adding more tests for validateGeneratedType #186, implement in Add import handling for external classes #135, and then integrate to Handling imports of generated types #148
  2. Drop the "external import" bit and focus only on "internal imports" of files which are also converted to zod schemas. This would mean a bit of rework, it would add support for multiple generated files (through ts-to-zod.config.js for now), but limited to referencing other "internal" types.
  3. Acknowledge that we don't fully support external imports and merge both PRs (after rebasing).

Any thoughts are welcome!

@tvillaren tvillaren changed the title [DO NOT MERGE] Handling imports of generated types Handling imports of generated types Feb 6, 2024
@tvillaren
Copy link
Collaborator Author

Hello @schiller-manuel, @fabien0102,

I rebased this PR and it's (finally) ready for merging.
Now ts-to-zod fully supports imports of types that are also converted into zod schema on the condition that all files are listed in the configuration file.

Next step would be to build the list "on the fly" (from a folder passed as argument for instance).

Copy link
Collaborator

@schiller-manuel schiller-manuel left a 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?

@tvillaren
Copy link
Collaborator Author

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!
Let's merge, I know some people (including me) are using this branch in their project.

@schiller-manuel schiller-manuel merged commit 1a879b1 into fabien0102:main Feb 6, 2024
2 checks passed
@schiller-manuel
Copy link
Collaborator

Thanks a lot for your work!

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.

None yet

4 participants