Skip to content

Conversation

@BowlingX
Copy link
Collaborator

@BowlingX BowlingX commented Jul 13, 2025

This adds support for zod version 4. Still WIP.

Will fix #22

@lostfictions
Copy link
Owner

lostfictions commented Jul 13, 2025

Thanks so much for getting this started!

Before we go further, do you think this approach is better than if we simply cut a major version that only supports 4.x and clearly document which release to keep using for 3.x? I feel like znv is pretty stable/mature now, and personally I don't see a problem with this approach -- it would avoid a lot of complexity and having to abstract out logic instead of keeping things compact/simple (which to me is a big aim of this library). It also doesn't preclude us having a 3.x branch that we can make critical fixes to down the line if necessary.

@BowlingX
Copy link
Collaborator Author

I am fine with that and agree that it would make it a lot simpler. I already had to separated the 2 implementations anyway, because it's impossible to merge the ts types due to the complexity. I will adjust it then in the upcoming week.

@BowlingX BowlingX marked this pull request as ready for review July 17, 2025 21:40
@BowlingX
Copy link
Collaborator Author

@lostfictions if you have some time this week. I think this state is acceptable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I adjusted the formatter a bit to closer match what zod's prettifyError outputs. We did not had any test before how the output looks like, so the test covers that.

// - to ensure that no other consumer of zod in the codebase has set a default
// error map that might override our formatting
// - to return slightly friendlier error messages in some common scenarios.
export const errorMap: ZodErrorMap = (issue, ctx) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

zod 4 deals different with error mapping (see https://zod.dev/v4/changelog?id=changes-error-map-precedence). I think the defaults of the formatter are good enough here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In case we have global error configuration, they would be applied here, but that also gives the chance to localize the error messages.

https://zod.dev/error-customization#global-error-customization

@BowlingX BowlingX requested a review from lostfictions July 19, 2025 20:14
@klippx
Copy link

klippx commented Jul 31, 2025

I would love to try an alpha release of this, if you have the means to do it. We have a large mono-repo using znv and zod and might find something.

Also, suggestion about versioning: Perhaps this is a good point that you abandon the 0.x branch where every minor is a potential breaking change. Just plan this feature for the final 1.0 release using zod ^4 branch perhaps. And keep bugfixing related to 0.x and zod 3 in a 0.5.x branch...

@BowlingX
Copy link
Collaborator Author

BowlingX commented Aug 1, 2025

Also, suggestion about versioning: Perhaps this is a good point that you abandon the 0.x branch where every minor is a potential breaking change. Just plan this feature for the final 1.0 release using zod ^4 branch perhaps. And keep bugfixing related to 0.x and zod 3 in a 0.5.x branch...

Yes, releasing this as a major version was the plan.

@BowlingX
Copy link
Collaborator Author

@lostfictions anything I should change?

@reslear
Copy link
Contributor

reslear commented Sep 20, 2025

hi @lostfictions friendly ping :)

totakoko added a commit to betagouv/france-chaleur-urbaine that referenced this pull request Sep 26, 2025
- Remove znv dependency (incompatible with Zod v4)
- Implement custom parseEnv function using Zod v4 safeParse
- Maintain same API and functionality as znv
- Handle boolean and number type coercion from environment strings
- Provides better error messages with variable context

BREAKING CHANGE: znv dependency removed in favor of native Zod v4 implementation

Related to: lostfictions/znv#23
@marco-arnold
Copy link

^^ 👍

Copy link
Owner

@lostfictions lostfictions left a comment

Choose a reason for hiding this comment

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

Hi, thanks for all your work on this and sorry to take such a long time! I've had some personal stuff happen over the past few months and OSS contributions were one of the things I haven't been able to keep up with.

I'd be happy to transfer znv over to some sort of organization (is there an existing "zod-userland" org or something similar?) if that makes maintenance and collaboration easier.

I personally haven't had time to directly use Zod 4 yet or run this PR locally, but I've left a few review comments about things that jumped out to me looking at the code. I don't think my review should block merging, so if folks would like to get this out the door quickly @BowlingX I think you should have general repo collaborator privileges, feel free to use your own judgment.


A few quick thoughts, not strictly related to this PR: if we'd like to make a breaking 1.0.0 release for Zod 4 support, it might also make sense (again, not necessarily in this PR) to clean up some other stuff as well. Some functionality has been added to Zod since 2022 that makes some of the bits in znv less necessary. For example, I think Date as a schema type should probably be deprecated; Zod now has z.iso.datetime() which is less footgun-prone and supports several useful customizations/refinements, so users should just use that directly (and transform to Date as a final step themselves if so desired). There might be other coercions that can similarly be removed?

By the same token, I think you have a great idea using meta() for description fields -- I think we can probably just get rid of the DetailedSpec format and use the new Zod registry functionality plus meta() for stuff like description and deprecation status, right? It might make some of the hairier type wrangling in parse-env.ts a lot simpler -- no conditional-type hacks to reach into an object and extract the Zod schema needed. A breaking release for Zod 4 support seems like the time to do it!

I don't want to foist all of this extra work on you, since you've already done a great job with this PR only to have it sit unreviewed all this time (sorry again 😔 ). I should have a bit more time and energy to contribute now, so just musing about next steps to get a 1.0 out the door!

Comment on lines 82 to 98
export const throwIfUnknown = (typeName: string) => {
throw new Error(
[
`Zod type not supported: ${typeName}`,
"You can use `z.string()` or `z.string().optional()` instead of the above type.",
"(Environment variables are already constrained to `string | undefined`.)",
].join("\n"),
);
};

export const throwIfCurrentlyUnsupported = (typeName: string) => {
throw new Error(`Zod type not yet supported: "${typeName}" (PRs welcome)`);
};

export const throwIfWillNeverBeSupported = (typeName: string) => {
throw new Error(`Zod type not supported: ${typeName}`);
};
Copy link
Owner

@lostfictions lostfictions Oct 6, 2025

Choose a reason for hiding this comment

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

These functions in particular seem like misnomers! 😄 I would assume a function named throwIfX, when called, checks a conditional X. Again, would personally prefer if they just stayed inline unless there's a good reason not to do that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved them inline

return (arg) => arg;
switch (def.type) {
case "pipe":
return getPreprocessorByZodType(def.in as zCore.$ZodTypes);
Copy link
Owner

Choose a reason for hiding this comment

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

Seems like there's a fair bit of additional as-casting now in this file and especially in parse-env.ts -- this makes it feel to me like there's some kind of mismatch going on with the internal types, even if the user-facing interface remains correct at the boundaries. It's not the end of the world, but is it necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will check this case again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I adjusted the types a bit and removed some of the castings.

BLA: z.string().default("bar"),
FOO: z.number(),
DO_NOT_USE_THIS_KEY: z.string().meta({
deprecated: true,
Copy link
Owner

Choose a reason for hiding this comment

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

I think this is a great idea! Though checking deprecated in metadata is not actually implemented here unless I'm missing something, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's used now :)

@BowlingX
Copy link
Collaborator Author

BowlingX commented Dec 3, 2025

@lostfictions could you check again, I kept the DetailedSpec for now, I could also remove it, but maybe we could for now just deprecate it, so the migration is smoother?

Copy link
Owner

@lostfictions lostfictions left a comment

Choose a reason for hiding this comment

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

LGTM! Great work and thanks for the revisions... I'm sorry again for all the delays on my side.

Personally I think I'm in favor of removing DetailedSpec for the next release -- when users upgrade znv to the next version they'll have to upgrade to Zod 4 anyway, and that may come with breaking changes of its own. To me it seems less annoying to do one migration than to have to migrate Zod and then migrate znv again later? Open to other opinions though.

Regardless, I think we can merge this and put out a preview release before making any final changes!

@BowlingX
Copy link
Collaborator Author

BowlingX commented Dec 6, 2025

@lostfictions awesome ok, thank you :) Yeah, I guess it makes sense to remove it.

@BowlingX BowlingX merged commit f38d9aa into master Dec 6, 2025
3 checks passed
@BowlingX
Copy link
Collaborator Author

@lostfictions As the branch is merged, maybe we can create a prerelease / rc?

@klippx
Copy link

klippx commented Jan 7, 2026

Patiently waiting for an rc or beta version...

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.

Support for Zod 4 (now in beta)

5 participants