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

support for Maybe<T> special case #91

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

eturino
Copy link

@eturino eturino commented Jul 26, 2022

  • we can configure if Maybe represents nullable, optional, or both.
  • we can define which generic types to use as Maybe (e.g. Maybe, InputMaybe, etc). Can be multiple.
  • when ts-to-zod encounters a generic type in the list of "Maybe"s, it skips the schema generation for them.
  • when it encounters them as being used, it makes a call to maybe() function.
  • the maybe function is defined depending on the nullable/optional config.

This is useful to work in conjunction with other codegen tools, like graphql codegens.

e.g.

// config
/**
 * ts-to-zod configuration.
 *
 * @type {import("./src/config").TsToZodConfig}
 */
module.exports = [
  {
    name: "example",
    input: "example/heros.ts",
    output: "example/heros.zod.ts",
    maybeTypeNames: ["Maybe"],
  }
];

// input

export type Maybe<T> = T | null | "whatever really"; // this is actually ignored

export interface Superman {
  age: number;
  aliases: Maybe<string[]>;
}

// output

export const maybe = <T extends z.ZodTypeAny>(schema: T) => {
  return schema.nullable();
};

export const supermanSchema = z.object({
    age: z.number(),
    alias: maybe(z.array(z.string()))
});

Configuration:

By default, this feature is turned off. When adding the list of type names to be considered 'Maybe's, we turn it on.
Maybe is nullable and optional by default, unless specified otherwise.

We can set this in CLI options...

  • maybeOptional: boolean, defaults to true
  • maybeNullable: boolean, defaults to true
  • maybeTypeName: string, multiple. List of type names.

…as well as in the ts-to-zod config file.

  • maybeOptional: boolean
  • maybeNullable: boolean
  • maybeTypeNames: string[]. list of type names.

Why

Maybe<T> is a common pattern, particularly used by the TS generated by other codegen tools (e.g. https://www.graphql-code-generator.com/). While parsing Maybe<T> as a generic can be problematic, in the vast majority of cases it is used for marking something as nullable, optional, or both.

By adding this support, we can both skip the Maybe util generic type in the zod generation, as well as interpret it according to our config. This will allow us to use ts-to-zod from the output of the graphql-codegen straight away.

- we can configure if Maybe represents nullable, optional, or both.
- we can define which generic types to use as Maybe (e.g. Maybe, InputMaybe, etc). Can be multiple.
- when ts-to-zod encounters a generic type in the list of "Maybe"s, it skips the schema generation for them.
- when it encounters them as being used, it makes a call to `maybe()` function.
- the `maybe` function is defined depending on the nullable/optional config.

This is useful to work in conjunction with other codegen tools, like graphql codegens.

e.g.

```ts
// config
/**
 * ts-to-zod configuration.
 *
 * @type {import("./src/config").TsToZodConfig}
 */
module.exports = [
  {
    name: "example",
    input: "example/heros.ts",
    output: "example/heros.zod.ts",
    maybeTypeNames: ["Maybe"],
  }
];

// input

export type Maybe<T> = T | null | "whatever really"; // this is actually ignored

export interface Superman {
  age: number;
  aliases: Maybe<string[]>;
}

// output

export const maybe = <T extends z.ZodTypeAny>(schema: T) => {
  return schema.nullable();
};

export const supermanSchema = z.object({
    age: z.number(),
    alias: maybe(z.array(z.string()))
});
```

Configuration:

By default, this feature is turned off. When adding the list of type names to be considered 'Maybe's, we turn it on.
Maybe is nullable and optional by default, unless specified otherwise.

We can set this in CLI options...

- `maybeOptional`: boolean, defaults to true
- `maybeNullable`: boolean, defaults to true
- `maybeTypeName`: string, multiple. List of type names.

…as well as in the ts-to-zod config file.
- `maybeOptional`: boolean
- `maybeNullable`: boolean
- `maybeTypeNames`: string[]. list of type names.
@fabien0102
Copy link
Owner

Noiceeeeeeeeee, this looks amazing! Sorry I totally missed the notification for this PR, let me give a try locally and review this 👍

@fabien0102 fabien0102 self-requested a review August 26, 2022 14:47
@fabien0102
Copy link
Owner

First of all, testing your PR makes me realize that I forgot the Set (#94), so thanks for this 👍

Now let's talk about the PR itself. The usecase is 100% relevant and we definitely need to fix this!

This said, I would prefer to try to parse the Maybe<T> properly instead of configure it.

My main issue is to have to map manually everything and having the command fail if you miss something (helpful to spot bugs but in this case I will go for an easier error message to avoid confusion, and this will be quite hard to achieve 😅)

image

Since parsing generics are actually quite hard, we can start by covering the cases of this Maybe (and throw for cases not handled)

Anyway, amazing PR, with unit tests, documentation, nice description, really amazing 😍 (I feel really guilty to not approving it immediately 🙈 )

I'm keeping this open for now, I will try to see if we can parse this Maybe<T> 😁

@fabien0102 fabien0102 self-assigned this Aug 26, 2022
@eturino
Copy link
Author

eturino commented Aug 28, 2022

fair enough! Thanks for moving it forward!

One thing I would suggest though is to cover both Maybe and InputMaybe which are the ones used by the graphql codegen, or to allow a config to set which Maybes to parse (the names)

@eturino
Copy link
Author

eturino commented Feb 2, 2023

@fabien0102 did you manage to get this working? I don't have a lot of bandwidth at the moment but I could try to help if not, or at least see if I can rebase the PR

@fabien0102
Copy link
Owner

Oh my god, I’m so out of date on this repository! Same as you, not a lot of bandwidth, I will try to have a look this week, but I should really not promise on any date 🙃

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

2 participants