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.

Sign up for GitHub

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

Persist default values when skipping validation #291

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+45 −5
Open

Persist default values when skipping validation #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions packages/core/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { TypeOf, ZodError, ZodObject, ZodType } from "zod";
import { object } from "zod";
import type { TypeOf, ZodError, ZodObject, ZodType, AnyZodObject } from "zod";
import { object, ZodDefault } from "zod";

export type ErrorMessage<T extends string> = T;
export type Simplify<T> = {
Expand Down Expand Up @@ -212,6 +212,16 @@ export type CreateEnv<
>
>;

const getDefaults = <Schema extends AnyZodObject>(schema: Schema) => {
return Object.fromEntries(
Object.entries(schema.shape).map(([key, value]) => {
if (value instanceof ZodDefault)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh i actualy like this, but don't think it'll be supported with this change: #299 ://

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately no - you could possibly add a getDefault option that receives a given schema and returns a default if it can get one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an alternative would be something like

createEnv({
  server: {
    FOO: v.optional(v.string)
  },
  runtimeEnv: {
    FOO: process.env.FOO ?? "DEFAUTL"
  }
})

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd bet for @EskiMojo14 solution, as the goal should be to avoid declaring the same thing twice ;)

Copy link
Contributor

@EskiMojo14 EskiMojo14 Jan 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've had a think and a play around, and ended up with this:

import { createEnv } from "@t3-oss/env-core";
import { v } from "valibot";

export const env = createEnv({
  server: {
    SKIP_AUTH: v.optional(v.boolean(), false),
    EMAIL: v.optional(v.pipe(v.string(), v.email()), "[email protected]"),
    PASSWORD: v.optional(v.pipe(v.string(), v.minLength(1)), "password"),
    SECRET_NUMBER: v.pipe(
      v.string(),
      v.transform((s) => parseInt(s, 10)),
      v.number(),
    )
  },
  // ...
  skipValidation: true,
  getUnvalidatedEnv: (env, schema) => ({
    ...v.getDefaults(schema),
    ...env,
    SECRET_NUMBER: typeof env.SECRET_NUMBER === "string" ? parseInt(env.SECRET_NUMBER, 10) : v.getDefault(schema.entries.SECRET_NUMBER),
  }),
});

Requiring a getUnvalidatedEnv callback when using skipValidation allows for the user to decide how to get the env shape they're expected to have, and we can check it against what the schema normally returns. The user can of course still bury their head in the sand, it's just a little more inconvenient to do so:

export const env = createEnv({
   // ...
  skipValidation: true,
  getUnvalidatedEnv: env => env as any
});

I can't open the PR yet since it's based on the branch for #313, but once that's finalised I'll get that open so we can have a place to discuss it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EskiMojo14 really liked your solution, but just wondering if we should introduce new property that's required, I think that it could be optional and by default, we'll return all the default values, wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Bartek532 done - had the same thought

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! @EskiMojo14 could you open the PR for this? We can then review together with @juliusmarminge ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#313 isn't merged, so if I opened the PR right now it would be on my fork not this one.

return [key, value._def.defaultValue()];
return [key, undefined]
})
)
}

export function createEnv<
TPrefix extends TPrefixFormat,
TServer extends TServerFormat = NonNullable<unknown>,
Expand All @@ -232,9 +242,6 @@ export function createEnv<
}
}

const skip = !!opts.skipValidation;
// biome-ignore lint/suspicious/noExplicitAny: <explanation>
if (skip) return runtimeEnv as any;

const _client = typeof opts.client === "object" ? opts.client : {};
const _server = typeof opts.server === "object" ? opts.server : {};
Expand All @@ -247,6 +254,17 @@ export function createEnv<

const allClient = client.merge(shared);
const allServer = server.merge(shared).merge(client);

const skip = !!opts.skipValidation;
// biome-ignore lint/suspicious/noExplicitAny: <explanation>
if (skip) {
return {
...getDefaults(allServer),
...runtimeEnv,
} as any;
}


const parsed = isServer
? allServer.safeParse(runtimeEnv) // on server we can validate all env vars
: allClient.safeParse(runtimeEnv); // on client we can only validate the ones that are exposed
Expand Down
22 changes: 22 additions & 0 deletions packages/core/test/smoke.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,3 +620,25 @@ describe("extending presets", () => {
});
});
});

describe("skipping validation", () => {
test("returns values when present", () => {
const env = createEnv({
server: { BAR: z.string().default("bar") },
runtimeEnv: { BAR: "foo" },
skipValidation: true,
});

expect(env).toMatchObject({ BAR: "foo" });
});

test("returns defaults when values missing", () => {
const env = createEnv({
server: { BAR: z.string().default("bar") },
runtimeEnv: {},
skipValidation: true,
});

expect(env).toMatchObject({ BAR: "bar" });
});
});