t3-oss / t3-env

https://env.t3.gg
MIT License
2.78k stars 86 forks source link

Upgrade jiti to v2 in docs and nextjs example #271

Closed ChristianIvicevic closed 1 month ago

ChristianIvicevic commented 1 month ago

jiti v2 has been released and has deprecated the previous default import and recommends to use ESM style imports of files. This PR just updates the docs to reflect that as well as the nextjs example, but doesn't change the setup used within the repository itself, in case you want to do that in a separate PR.

vercel[bot] commented 1 month ago

@ChristianIvicevic is attempting to deploy a commit to the t3-oss Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
t3-env ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 30, 2024 3:06am
Teekola commented 1 month ago

It seems like using jiti with a higher version than in the example (1.21.0) will cause a webpack warning.

Next config file

// next.config.mjs

import createJiti from "jiti";
import { fileURLToPath } from "node:url";

const jiti = createJiti(fileURLToPath(import.meta.url));

// Import env here to validate during build. Using jiti we can import .ts files
jiti("./src/env/server");
jiti("./src/env/client");

Warning that is logged on both locale development and on Vercel. On locale development this is logged even on npm run dev, on Vercel this is during build.

<w> [webpack.cache.PackFileCacheStrategy/webpack.FileSystemInfo] Parsing of /vercel/path0/node_modules/jiti/lib/jiti.mjs for build dependencies failed at 'import(id)'.
<w> Build dependencies behind this expression are ignored and might cause incorrect cache invalidation.

Switching to the older jiti version (1.21.0) made the warning disappear.

ChristianIvicevic commented 1 month ago

That's presumably due to the breaking changes of using the old API and not the new API they have introduced. Using to the code that I added in this PR I have no issues.

import { fileURLToPath } from 'node:url'
import { createJiti } from 'jiti'
const jiti = createJiti(fileURLToPath(import.meta.url))
await jiti.import('./src/env')

Am I understanding your comment correctly that this is an argument to update to the new version?

ubinatus commented 1 month ago

Just sharing this, if helps. In my case (monorepo), the current upgrade of jiti to v2 triggered an error because the "env/client" and "env/server" imported zod as import { z } from "zod"; instead of import * as z from "zod";. When used the latter, the error was gone.

Teekola commented 1 month ago

Just sharing this, if helps. In my case (monorepo), the current upgrade of jiti to v2 triggered an error because the "env/client" and "env/server" imported zod as import { z } from "zod"; instead of import * as z from "zod";. When used the latter, the error was gone.

Oh yes, this caused the problem! Thanks!

Edit: tried updating the imports, and that did not solve the error :/

ubinatus commented 1 month ago

Just sharing this, if helps. In my case (monorepo), the current upgrade of jiti to v2 triggered an error because the "env/client" and "env/server" imported zod as import { z } from "zod"; instead of import * as z from "zod";. When used the latter, the error was gone.

Oh yes, this caused the problem! Thanks!

Edit: tried updating the imports, and that did not solve the error :/

Hmm. I had my main env.ts extending multiple .env.ts from other internal packages (that used either @t3-oss/env-core or @t3-oss/env-nextjs). Changing the zod import for all the those **/env.ts files made it work.

However, I'm now seeing a warning log when either running the dev server or building nextjs

<w> [webpack.cache.PackFileCacheStrategy/webpack.FileSystemInfo] Parsing of /Users/ubinatus/Work/monorepo/node_modules/.pnpm/jiti@2.1.1/node_modules/jiti/lib/jiti.mjs for build dependencies failed at 'import(id)'.
<w> Build dependencies behind this expression are ignored and might cause incorrect cache invalidation.
juliusmarminge commented 1 month ago

See https://github.com/unjs/jiti/issues/329#issuecomment-2409035681

Not sure it's worth it to update to v2 given the issues here. Next 15 will have .ts support ootb and you wont need Jiti at all.

ChristianIvicevic commented 1 month ago

Sounds plausible given .ts support is coming - I'll go ahead and close this PR for now. In the unlikely event it becomes relevant again it can reopened anyways.