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

Allow for ZodObject to be passed directly to client/server keys instead of an object of ZodTypes #169

Open
kpervin opened this issue Jan 23, 2024 · 14 comments · May be fixed by #272
Open
Labels
PRs Accepted Feel free to pick this up and make a PR

Comments

@kpervin
Copy link

kpervin commented Jan 23, 2024

Currently in @t3-oss/env-nextjs you are meant to define your env as follows:

const env = createEnv({
  server: {
    FOO: z.string().required()
  },
  client: {
    BAR: z.string().required()
  }
});

However, there are times where you might want to define these ZodObjects outside of your env.mjs file and simply pass them:

/* External file*/
export const ServerSchema = z.object({
  FOO: z.string().required()
});
export const ClientSchema = z.object({
  BAR: z.string().required()
});


/* env.mjs */
const env = create schema({
  server: ServerSchema,
  client: ClientSchema
});

This however does not work and the resulting env has type Record<string, {}> because it wasn't expecting a ZodObject. It would be nice if we could pass pre-defined ZodObjects to these keys and retain type safety.

@kpervin
Copy link
Author

kpervin commented Jan 23, 2024

Closing as you can just use Schema.shape, however it would be good to include that in the docs.

/* External file*/
export const ServerSchema = z.object({
  FOO: z.string().required()
});
export const ClientSchema = z.object({
  BAR: z.string().required()
});


/* env.mjs */
const env = create schema({
  server: ServerSchema.shape,
  client: ClientSchema.shape
});

@kpervin kpervin closed this as completed Jan 23, 2024
@shkreios
Copy link

I believe there's significant value in considering the capability to pass a complete Zod schema, as opposed to merely an object structure, in our configuration. This feature would offer greater flexibility, particularly in complex scenarios where multiple conditional validations are necessary.

Example

Take, for instance, an app that allows a different set of authentication providers. In such cases, it's crucial to ensure that correlated environment variables are collectively defined. For example, if GOOGLE_CLIENT_ID is provided, it's imperative that GOOGLE_CLIENT_SECRET is also present, and not just marked as optional.

Example schema

const GoogleAuthSchema = z.object({
  GOOGLE_CLIENT_ID: z.string(),
  GOOGLE_CLIENT_SECRET: z.string(),
});

const ResendSchema = z.object({
  RESEND_API_KEY: z.string(),
});

const ServerSchema = z
  .object({
    NODE_ENV: z.enum(["development", "test", "production"]).default("development"),
    NEXTAUTH_SECRET: process.env.NODE_ENV === "production" ? z.string() : z.string().optional(),
  })
  .and(z.union([GoogleAuthSchema, ResendSchema]));

Resulting env type

type ServerSchemaType = {
  NODE_ENV: "development" | "test" | "production";
  NEXTAUTH_SECRET?: string | undefined;
} & (
  | {
      GOOGLE_CLIENT_ID: string;
      GOOGLE_CLIENT_SECRET: string;
    }
  | {
      RESEND_API_KEY: string;
    }
);

@shkreios
Copy link

I opened a new issue #176

@kpervin
Copy link
Author

kpervin commented Jan 29, 2024

@shkreios I could simply reopen this one, if you'd like

@shkreios
Copy link

@shkreios I could simply reopen this one, if you'd like

If thats possible, yes feel free to, maybe you can copy the new issue text into an updated request section or so

@kpervin kpervin reopened this Jan 30, 2024
@wfl-junior
Copy link

Any updates here? I would appreciate this feature, as I want to utilize super refine to validate some variables to be required only if another one is a certain value.

@juliusmarminge
Copy link
Member

juliusmarminge commented Feb 7, 2024

feel free to open a PR - iirc the reason why i did an object was cause inferring the shape didn't work properly with z.object() .

Don't try to make it backwards compat, lib is still on 0.x so we can make a breaking change that requries everyone to use z.object()

@helmturner
Copy link

helmturner commented Aug 25, 2024

If there's a technical limitation to z.object, I wonder if there's an innovative workaround.

I'm thinking, what if there were another parameter: discriminatedUnions (name up for debate, picked something illustrative).

E.g. given this createEnv call:

export const env = createEnv({
  runtimeEnv: process.env,
  emptyStringAsUndefined: true,
  server: {
	NODE_ENV: z.enum(['development', 'test', 'production']),
	url: z.string(),
	otherStuff: z.string().optional(),
  },
  discriminatedUnions: {
    NODE_ENV: {
	  development: {
        url: z.literal('http://localhost'),
      },
      test: {
        url: z.literal('https://test.example.com'), 
      },
      production: {
        url: z.literal('https://example.com'),
        otherStuff: z.string(),
      },
    }
  }
});

When parsed, the resulting environment object would have this type:

```typescript
type Env = 
| {
    NODE_ENV: 'production',
    url: z.literal('https://example.com'),
    otherStuff: string,
  }
| (
  & { otherStuff?: string | undefined }
  & (
    | {
        NODE_ENV: 'development',
        url: 'http://localhost',
      }
    | {
       NODE_ENV: 'test',
       url: 'https://test.example.com',
      }
    )
)

(The convoluted way of representing that type is intentional, intended to represent the type I would expect to see given how this would likely need to be implemented, e.g. with stuff like Exclude<Obj1, keyof Obj2> & Obj2[keyof Obj1 & keyof Obj2]

In essence, this would effectively mean parsing the environment once with the top-level validation, then creating a discriminated union parser (non-strict) that a runs secondary validation and unions the resulting types.

The need for secondary validation is somewhat mitigated by using the more performant ZodDiscriminatedUnion.

Worth noting:

We would need type magic to validate that the top-level keys of the "discriminatedUnions" object are valid, and we would need additional type magic to ensure that the keys of each "discriminatedUnion" object are assignable to the type asserted by the primary validation schema, and to ensure that the sub-schemas provided represent valid subsets of the primary validation schema.

@juliusmarminge
Copy link
Member

If there's a technical limitation to z.object, I wonder if there's an innovative workaround.

No it should be doable. I just recall it being a pain validating the shape's keys when the input was a ZodObject. Should be doable but I don't see it as enough a benefit to justify the time it would take to dig into it now.

If anyone has time and need this feature feel free to submit a PR

@feder240516
Copy link

I think I may be able to make it work, though it will take some time (correct type inference is hard).

Btw, this issue only talks about server and client, should shared also be changed?

@juliusmarminge
Copy link
Member

Yea everything.

@helmturner
Copy link

@feder240516 Any way I can help? I'm pretty nifty with type inference, and could certainly use this for work!

@feder240516
Copy link

@helmturner I think I've finished with the core package, however the library also includes a next package, and that's where I'm currently having problems. I'll polish and upload my partial PR tonight (it's 7AM here) so you can give it a look

@feder240516 feder240516 linked a pull request Oct 8, 2024 that will close this issue
2 tasks
@feder240516
Copy link

feder240516 commented Oct 8, 2024

@helmturner I've just uploaded a draft PR, as you can see next package doesn't work, but core package works like a charm

Edit: While uploading and polishing some details, I found the origin of the issue, so I think the PR is now complete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PRs Accepted Feel free to pick this up and make a PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants