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

Restore existing type inference behaviour of z.ZodObject #1122

Merged
merged 2 commits into from May 12, 2022

Conversation

bentefay
Copy link
Contributor

@bentefay bentefay commented May 9, 2022

Fixes the issue documented here: #1121

I'm pretty sure my PR introduced this issue: #1117

In almost all cases z.flatten<{ property: (T | undefined) & T }> flattens to { property: T }. However, this does not appear to be the case for some very specific types like this one:

type Opaque<Type, Token = unknown> = Type & { tag: Token };

  enum FACE {
    QUEEN = "QUEEN",
    KING = "KING",
  }

  enum SUIT {
    HEARTS = "HEARTS",
    DIAMONDS = "DIAMONDS",
  }

  type FaceCard<Face extends FACE> = Opaque<`${Face}_${SUIT}`, "FaceCard">;

  type Hand = z.objectUtil.flatten<
    z.ZodObject<{
      card: z.ZodType<FaceCard<FACE.QUEEN>, z.ZodTypeDef, string>;
    }>["_output"]
  >;

In this case Hand is flattening to:

type Hand = {
    card: (FaceCard<FACE.QUEEN> | undefined) & FaceCard<FACE.QUEEN>;
}

Fortunately, I now better understand the problem I was trying to fix in my PR, and it is actually possible to write z.ZodObject in a way that preserves both behaviours.

It also has the benefit of being easier to understand:

  type optionalKeys<T extends object> = {
    [k in keyof T]: undefined extends T[k] ? k : never;
  }[keyof T];

  type requiredKeys<T extends object> = {
    [k in keyof T]: undefined extends T[k] ? never : k;
  }[keyof T];

  export type addQuestionMarks<T extends object> = Partial<
    Pick<T, optionalKeys<T>>
  > &
    Pick<T, requiredKeys<T>>;

Unfortunately I couldn't see a way to write a test for this, as:

type Hand = {
    card: (FaceCard<FACE.QUEEN> | undefined) & FaceCard<FACE.QUEEN>;
}

and

type Hand = {
    card: FaceCard<FACE.QUEEN>
}

are equal as far as util.AssertEqual is concerned.

Fixes this issue: colinhacks#1121

Specifically, `{ property: (T | undefined) & T }` does not seem to
flatten to `{ property: T }` in the case that T is a complex string literal.
@bentefay bentefay changed the title Fix addQuestionsMarks type helper so it resolves to expected type in more cases Restore existing type inference behaviour of z.ZodObject (while also working with IDE features) May 11, 2022
@bentefay bentefay changed the title Restore existing type inference behaviour of z.ZodObject (while also working with IDE features) Restore existing type inference behaviour of z.ZodObject May 11, 2022
@colinhacks
Copy link
Owner

colinhacks commented May 12, 2022

Sounds good to me. That test may be possible if we switched to using something like https://github.com/dsherret/conditional-type-checks which is a little smarter than the current util.AssertEqual trick, but I've been procrastinating on that. Not vital at the moment.

@colinhacks colinhacks merged commit c4740a6 into colinhacks:master May 12, 2022
MrAwesome pushed a commit to MrAwesome/zod that referenced this pull request May 20, 2022
…1122)

* Fix addQuestionsMarks so it resolves correctly in more cases

Fixes this issue: colinhacks#1121

Specifically, `{ property: (T | undefined) & T }` does not seem to
flatten to `{ property: T }` in the case that T is a complex string literal.

* Ran lint on languageServerFeatures.test.ts
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