feat: import optional deps on demand from the user project - #4542
Conversation
Drops all optional peer dependencies except `vite`, resolving each from `nitro.options.rootDir` via `utils/dep.ts` instead: - `rollup` — new `build/rollup/_import.ts` helper - `xml2js` — IIS `web.config` merging - `@vercel/queue`, `zephyr-agent` — already used `importDep` `dotenv`, `giget` and `jiti` are optional deps of the bundled `c12`, so a bare specifier inside `dist/` cannot reach the user project. They get a bundle-time alias to a shim under `src/shims/` that goes through `importDep` as well. `vite` stays an optional peer: `build/vite/` extends its `DevEnvironment` class and re-exports its types from the public `.d.ts`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change adds optional dependency configuration and build-time shims. Rollup and IIS utilities now resolve dependencies from the user project through ChangesOptional dependency loading
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change updates optional dependency loading while preserving existing behavior, and no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
Removes all optional peer dependencies except
vite, resolving each from the user project on demand viasrc/utils/dep.ts(importDep/ensureDep, which resolves fromnitro.options.rootDirand offers to install what is missing).Direct imports →
importDeprollupawait import("rollup")×2src/build/rollup/_import.tshelperxml2jsawait import("xml2js")rootDirin the IISweb.configmerge@vercel/queueimportDep— the peer entry was redundantzephyr-agentimportDepdotenv/giget/jitiThese are optional peers of
c12, which Nitro bundles — so a bareimport("jiti")insidedist/cannot reach the user'snode_modulesunder pnpm's isolated layout. They now get a bundle-timeresolve.aliasto a shim undersrc/shims/that goes throughimportDeptoo:dotenvis only reachable on runtimes withoutnode:util.parseEnv(below Nitro's supported Node range), andgigetonly for remoteextends:— both keep working, they just install on demand now.vitestays an optional peersrc/build/vite/dev.tsextends vite'sDevEnvironmentclass andsrc/types/build.ts/src/build/vite/*re-export vite types from Nitro's public.d.ts, so dropping the peer entry would break type resolution for consumers. Left as-is to keep the risk down.Other
build.config.ts: externals no longer derive the full list frompkg.peerDependencies(which would have silently started bundlingrollup); explicitoptionalDepslist instead, minus the shimmed ones. Same for thetracedPackageshook.dotenv+jitimoved todevDependencies(they were peer-only) so this repo's own tests and typecheck still resolve them.AGENTS.md: documents the convention.Testing
pnpm lint,pnpm typecheck,pnpm buildclean.pnpm test:rolldown913 passed.pnpm test:rolluphas 3 failures intest/unit/import-attributes.test.ts— pre-existing onmain(verified by stashing), caused byresolveBuilderprompting for an install against a temprootDirinstead of usingutils/dep.ts, which has anisTestguard. Not touched here.🤖 Generated with Claude Code