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

Removing the type guards? #293

Closed
scotttrinh opened this issue Jan 15, 2021 · 5 comments
Closed

Removing the type guards? #293

scotttrinh opened this issue Jan 15, 2021 · 5 comments

Comments

@scotttrinh
Copy link
Collaborator

Saw the note about removing check, but if it is simply safeParse().success, why can we not implement it that way in the base type? I assume this has already been tried, but if there is a problem with that, it would be helpful to know what the problem is so that I don't accidentally reproduce the issue in my own usage.

@scotttrinh
Copy link
Collaborator Author

Wrote this little wrapper that seems to do the right thing:

function check<T>(schema: ZodSchema<T>, data: unknown): data is T {
  const parsed = schema.safeParse(data);
  return parsed.success;
}

— But still not clear why this can't be how ZodType#check is implemented to avoid having to import this helper when using a ZodSchema

@hixus
Copy link

hixus commented Jan 31, 2021

Actually it's probably because transformers transform types. For example:

const countLength = z.transformer(
  z.string(),
  z.number(),
  (myString) => myString.length
);

const originalString = "foobar"
if (countLength.check(originalString)) {
 console.log('originalString is number', typeof originalString) // would result wrong typecheck
}

@scotttrinh
Copy link
Collaborator Author

Ahh, that makes sense. So not only do we want to avoid the type guard in this situation, we should always use the parsed data directly to avoid accidentally asserting something about a type before it has been transformed.

So to build off of your example:

const countLength = z.transformer(
  z.string(),
  z.number(),
  (myString) => myString.length
);

const originalString = "foobar";
const parsed = countLength.safeParse(originalString);
if (!parsed.success) {
  throw new Error("parsing error! oh no!");
}

parsed.data; // number

@scotttrinh
Copy link
Collaborator Author

@colinhacks

Do you want me to take a swing at adding to the v3 docs now that @hixus has helped enlighten me on this issue? I know it's a bit early to be documenting the API itself, but since this is more of a pattern, and I imagine the basic premise will hold throughout the refactor, seems like a worthwhile time to guide people toward to right usage maybe?

At any rate, the issue is definitely "resolved", so I'll close this now. Thanks again @hixus for helping me to understand!

@ChristoRibeiro
Copy link

Finally I used the same way as @scotttrinh did. It works nicely with next-rest.

Full context here: joeltg/next-rest#1

// zod-check.ts
import { ZodSchema } from "zod"

export function check<T>(schema: ZodSchema<T>) {
  function is(data: unknown): data is T {
    return schema.safeParse(data).success
  }
  return { is }
}
// pages/api/widgets/[id].ts
...
import { ZodSchema } from "zod"
import { check } from "./zod-check"

const getRequestHeaders = z.object({ accept: z.literal("application/json") })
const getRequestBody = z.void()

export default makeHandler("/api/widgets/[id]", {
  GET: {
    headers: check(getRequestHeaders).is,
    body: check(getRequestBody).is,
    exec: async ({ params }) => {
      // ... method implementation
      return {
        headers: { "content-type": "application/json" },
        body: { id: params.id, isGizmo: false },
      }
    },
  },
})

Wrote this little wrapper that seems to do the right thing:

function check<T>(schema: ZodSchema<T>, data: unknown): data is T {
  const parsed = schema.safeParse(data);
  return parsed.success;
}

— But still not clear why this can't be how ZodType#check is implemented to avoid having to import this helper when using a ZodSchema

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

No branches or pull requests

3 participants