fix(config): keep packages externalized when loading TypeScript next.config

[james-elicx]

Summary

Fixes a production-build regression introduced by #1701. The app-router-playground example (and any app whose next.config.ts imports a CJS config plugin) fails to build with:

[vinext] Failed to load next.config.ts: require is not defined
  at @next/mdx/index.js:22  (require.resolve('./mdx-js-loader'))

See the failing deploy: https://github.com/cloudflare/vinext/actions/runs/26746903446/job/78825508039

Root cause

#1701 added resolve.noExternal: true while loading a TypeScript next.config so that a baseUrl-local file could shadow an installed package of the same name. noExternal: true de-externalizes every installed package and forces it through Vite's module runner. CJS config plugins like @next/mdx call require / require.resolve at runtime (inside the returned config factory), and the runner doesn't provide require for those de-externalized modules — so they throw require is not defined.

This wasn't caught before merge because deploy-examples is skipped for fork PRs, and the added "imports ESM and CommonJS packages" test only used trivial module.exports = "..." packages that never call require at runtime.

Fix

Drop the blanket noExternal: true. Installed packages stay externalized (so CJS config plugins keep working), and the native Vite 8 resolve.tsconfigPaths resolver still resolves baseUrl-local bare imports that have no installed package of the same name — which is the actual Next.js tsconfig-extends fixture behavior.

The only behavior we give up is shadowing an installed package with a baseUrl-local file of the same name. No Next.js fixture exercises that case; it was an extra invariant added in #1701.

Behavior matrix

Scenario	Before (#1701)	After
next.config.ts imports bar, only local bar.ts exists	local	local
next.config.ts imports bar, only package bar exists	package	package
next.config.ts imports bar, both local bar.ts and package bar exist	local	package
next.config.ts imports a CJS plugin that calls require.resolve at runtime (@next/mdx)	throws	works

Tests

• Updated the shadowing test to assert the installed package wins.
• Added a regression test for an @next/mdx-style CJS plugin that calls require.resolve at runtime (verified red before this change, green after).
• Documented the package-precedence behavior in the README limitations entry.

Validation

• vp test run tests/next-config.test.ts — 167 pass
• vp build in examples/app-router-playground — builds successfully (was failing on main)

[james-elicx]

/bigbonk review

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/vinext@1710

commit: 972f086

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
web	preview	production

[ask-bonk]

Review: fix(config) keep packages externalized when loading TypeScript next.config

Verdict: Looks good. Correct, well-tested fix that restores Next.js parity.

I verified the change end-to-end:

Root cause and fix are sound

The diff drops the noExternal: true block added in #1701. That option de-externalized every installed package and forced it through Vite's module runner, which doesn't provide require/require.resolve — so CJS config factories like @next/mdx threw require is not defined at runtime. Keeping packages externalized is the right call, and resolve.tsconfigPaths still resolves the actual Next.js tsconfig-extends fixture case (a baseUrl-local file with no installed package of the same name).

Verified the regression claims locally

• On main (with noExternal: true), the new require.resolve regression test fails with exactly ReferenceError: require is not defined at fake-mdx/index.js:6 — the reported deploy failure. ✅ red before
• On the PR branch, the same test passes. ✅ green after
• The updated shadowing test confirms the documented matrix change: main returns local, PR returns package. The behavior change (installed package wins over baseUrl-local file) is real, intentional, and accurately documented.
• Full tests/next-config.test.ts: 167 pass.
• vp check on both changed files: clean (format, lint, types).

Behavior change is acceptable and documented

The only regression is losing the ability to shadow an installed package with a baseUrl-local file of the same name. As the PR notes, no Next.js fixture exercises that case — it was an extra invariant introduced in #1701. The README limitation entry and inline comments accurately describe the new precedence (package wins). This matches Next.js behavior (their SWC pipeline externalizes node_modules too).

Test quality

• The new regression test reproduces the exact failure mode (require.resolve inside a CJS factory invoked at config-load time), not a trivial module.exports string — this directly closes the gap that let #1701 slip through.
• Existing CJS/ESM package import coverage (line 664+) still passes, confirming no collateral regression.

Minor / non-blocking

• None. The comments in next-config.ts (lines 832-835, 852-859) are updated to match the new behavior and read clearly.

Nice tight fix with a genuine red/green regression test.

[ask-bonk]

Review posted to PR #1710.

Summary

I reviewed PR #1710 (fix(config): keep packages externalized when loading TypeScript next.config) and approved it.

What the PR does: Removes the noExternal: true block introduced in #1701, which had de-externalized every installed package and routed it through Vite's module runner — breaking CJS config plugins like @next/mdx that call require.resolve at runtime.

Verification I performed:

• Confirmed the new regression test fails on main with the exact reported error (ReferenceError: require is not defined at fake-mdx/index.js), and passes on the PR branch — genuine red/green.
• Confirmed the shadowing-behavior test change is real: main returns local, PR returns package, matching the documented behavior matrix.
• Ran the full tests/next-config.test.ts (167 pass) and vp check on both changed files (clean).
• Confirmed the behavior change (installed package wins over a baseUrl-local file) is intentional, documented in the README + inline comments, and aligns with Next.js's own externalization behavior. No Next.js fixture exercises the dropped case.

No blocking issues found. The fix is correct, minimal, and backed by a meaningful regression test that closes the gap which let #1701 slip through.

github run