fix(build): honor project Babel configs

[james-elicx]

Summary

• honor project Babel configs before vinext's Vite 8 OXC JSX transform
• resolve Babel from the app or Next's bundled compiler
• match Next.js handling for transpilePackages, experimental.externalDir, and experimental.forceSwcTransforms
• provide environment-aware Babel caller metadata, including the Cloudflare worker target
• add focused production-build coverage for config precedence, empty output, external sources, transpiled packages, and forced default transforms

Parity

Fixes the failure in Next.js v16.2.6 test/e2e/babel/index.test.ts where Flow syntax configured through .babelrc was rejected by vinext's jsx-in-js transform before Babel could strip it.

Next.js references:

• packages/next/src/build/webpack-config.ts
• packages/next/src/build/babel/loader/get-config.ts
• test/e2e/babel/index.test.ts

Validation

• vp test run tests/pages-router.test.ts -t "honors a project Babel config"
• vp check packages/vinext/src/plugins/babel-config.ts packages/vinext/src/config/next-config.ts packages/vinext/src/index.ts tests/pages-router.test.ts
• local production build of the Next.js Babel/Flow fixture
• targeted Next.js deploy-suite attempt reached deployment but was unclassifiable due to local ENOSPC before assertions

Documentation

No documentation change is required: this restores compatibility with existing Next.js Babel configuration behavior and adds no vinext-specific public API.

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2360

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

commit: 6c2c399

[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
workers-cache	preview	production
web	preview	production

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #2360 (honor project Babel configs)

Reviewed against Next.js's actual Babel loader (packages/next/src/build/babel/loader/get-config.ts, get-babel-config-file.ts). Overall the approach is sound and the config-file selection ordering now matches Next.js exactly. A few correctness/robustness issues worth addressing:

1. fs.realpathSync.native(filename) can throw and crash the build (correctness)

packages/vinext/src/plugins/babel-config.ts:97 calls fs.realpathSync.native(filename) on any absolute id that passes the \.[cm]?[jt]sx? filter. If a plugin-generated module has an absolute-looking, .js-suffixed id that does not exist on disk, this throws ENOENT and aborts the whole build/dev server — but only when a Babel config is present, which makes it an easy-to-miss, config-conditional failure.

The repo already has a safe helper for exactly this: tryRealpathSync in build/ssr-manifest.ts:16 (returns null instead of throwing). Recommend using it here (and at :61/:70 for canonicalRoot) and bailing out when it returns null:

const canonicalFilename = tryRealpathSync(filename);
if (!canonicalFilename) return;

2. Within-root check ignores Windows path semantics (robustness)

:98-100 does a hand-rolled path.relative + startsWith("..") check. The codebase already has relativeWithinRoot(root, moduleId) (build/ssr-manifest.ts:28) which handles Windows drive-letter/UNC paths and the relativeId === ".." edge case. Reusing it keeps this consistent with the rest of vinext and avoids subtle cross-platform divergence.

3. existsSync for srcDir/pagesDir runs on every transform (hot-path perf)

:139-144 calls fs.existsSync twice for every matching source file on every transform. These paths are immutable for the lifetime of the build/server, so they should be computed once in configResolved (alongside canonicalRoot/configPath) and reused. This matches the AGENTS.md guidance to "hoist immutable ... lookup tables to module scope" and "avoid repeated hot-path work." With a Babel config present on a large app this is existsSync × 2 × every source file.

4. this.environment.config without optional chaining (minor consistency)

:120 reads this.environment.config directly while the rest of the codebase guards with this.environment?. (e.g. client-reference-dedup.ts:241, index.ts:2809). transform hooks populate this.environment in Vite 6+, so this is low-risk, but matching the existing convention is safer.

Things that are correct / verified

• Config-file ordering (BABEL_CONFIG_FILES) exactly matches Next.js get-babel-config-file.ts after commit 263cb3d.
• babelrc: false + explicit configFile matches Next.js's loader, which also disables .babelrc discovery and reads a single selected config. The test correctly proves babel.config.cjs is not auto-loaded when .babelrc is selected.
• No JSX double-transform: next/babel strips JSX before the downstream vinext:jsx-in-js / vite:oxc passes see the code, so lang: "jsx" only sees already-compiled output.
• Plugin ordering (enforce: "pre", placed before reactPluginPromise and vinext:jsx-in-js in the array) is correct for "Babel runs first."

Parity note (not a blocker)

Next.js's loader applies its own per-file plugins on top of the user config (transform-define for process.env.NODE_ENV / typeof window / process.browser, next-ssg-transform, next-page-config). This PR relies solely on native Babel + the user's config. Most of that is covered downstream in vinext (typeof-window plugin, OXC, etc.), but process.browser in particular is not defined anywhere in vinext and won't be replaced via this path either. Pre-existing gap, just flagging that the Babel path doesn't close it.

[ask-bonk]

Review posted to PR #2360. Summary of findings:

Review Summary

The PR's core approach is correct and matches Next.js's actual Babel loader well. The config-file selection ordering (after commit 263cb3d) exactly matches Next.js's getBabelConfigFile, and babelrc: false + explicit configFile correctly mirrors Next.js's loader behavior. The test legitimately verifies config selection.

Issues flagged:

1. fs.realpathSync.native(filename) can crash the build (babel-config.ts:97) — throws ENOENT on absolute, .js-suffixed ids that don't exist on disk, but only when a Babel config is present (config-conditional failure). The repo already has tryRealpathSync for exactly this.

2. Within-root check ignores Windows paths (:98-100) — hand-rolled path.relative check instead of the existing relativeWithinRoot helper that handles Windows drive-letter/UNC paths.

3. existsSync on every transform (:139-144) — immutable srcDir/pagesDir paths recomputed per-file; should be hoisted to configResolved per AGENTS.md hot-path guidance.

4. this.environment.config without optional chaining (:120) — minor inconsistency with the rest of the codebase.

Plus a non-blocking parity note that Next.js's per-file plugins (transform-define for process.browser etc.) aren't applied via this path, though most are covered downstream in vinext.

I verified the test passes and lint is clean on the new file.

github run

[james-elicx]

/bigbonk review for issues

[github-actions]

Performance benchmarks

Compared 6c2c399 against base de014d4 using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	117.1 KB	117.1 KB	⚫ -0.0%
Client entry size (gzip)	vinext	112.5 KB	112.5 KB	⚫ -0.0%
Dev server cold start	vinext	2.63 s	2.62 s	⚫ -0.2%
Production build time	vinext	3.14 s	3.14 s	⚫ +0.2%
RSC entry closure size (gzip)	vinext	93.0 KB	93.0 KB	⚫ +0.0%
Server bundle size (gzip)	vinext	156.9 KB	156.9 KB	⚫ +0.0%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[ask-bonk]

/bigbonk re-review — PR #2360 (honor project Babel configs)

Re-reviewed at 4a5eb22. The earlier bonk findings are all addressed: babel-config.ts now uses tryRealpathSync/relativeWithinRoot (:106-107), guards this.environment?.config (:127), and hoists srcDir/pagesDir into configResolved (:74-80). The new test passes and vp check is clean on the file. Approach matches Next.js's loader (config-file ordering, babelrc: false + explicit configFile). A few smaller items remain.

1. fs.realpathSync.native(root) at module scope can still throw (:62, :73) — minor

Both the initializer (:62) and configResolved (:73) call the throwing variant directly. :62 runs at plugin construction time (createBabelConfigPlugin() is invoked eagerly in the vinext() plugin array at index.ts:1184), and its value is immediately overwritten by configResolved. Low risk since process.cwd()/config.root virtually always exist, but for consistency with the hardening already applied elsewhere in this file, both should use the safe helper:

let canonicalRoot = tryRealpathSync(process.cwd()) ?? process.cwd();
// ...
canonicalRoot = tryRealpathSync(root) ?? root;

This also removes the redundant eager realpath at :62 doing real I/O during plugin array construction.

2. configPath is non-canonical while cwd is canonical — minor inconsistency

findBabelConfig(root) builds configPath from the non-canonical root (:74), but Babel is then called with cwd: canonicalRoot (:133). Babel resolves configFile to an absolute path so it still works, but mixing canonical and non-canonical roots is the exact footgun this file otherwise avoids. Recommend computing the config path from canonicalRoot (and likewise the configureServer watcher candidates at :83, which currently watch root-relative paths). If root is a symlinked checkout, the watcher could miss changes to the realpath'd config file.

3. Babel runs on all in-root source, broader than Next.js's loader — parity note (not a blocker)

The transform scopes to any in-root .[cm]?[jt]sx? file outside node_modules (:99-107). Next.js's babel-loader is only wired to page/app/src module rules, not arbitrary root files. In practice config files like next.config.js are loaded by vinext's own loader (not Vite's transform pipeline) so they won't hit this, but the scope is wider than Next.js. Worth a comment noting the intentional difference, or tightening to srcDir/pagesDir/appDir if you want exact parity.

Verified

• Config-file selection order in BABEL_CONFIG_FILES matches Next.js getBabelConfigFile; babel.config.cjs is correctly not loaded when .babelrc is selected (the test's poison babel.config.cjs proves it).
• babelrc: false + explicit configFile mirrors Next.js's loader.
• New test legitimately exercises both JSX-before-Babel ordering (StringLiteral rewrite survives the JSX transform) and plugin execution (the empty.js body-strip), and passes against symlinked node_modules — which also exercises the realpath/symlink handling.
• Lazy @babel/core resolution throws a clear, actionable error if a config is present but the dep is missing; reasonable.

Parity gap carried over (not introduced here)

process.browser is still not replaced via this Babel path (Next.js's loader injects transform-define per file). Pre-existing; flagging only so it isn't assumed closed by this PR.

Nothing here is blocking. Items 1–2 are cheap robustness/consistency fixes; item 3 is a documentation/scoping nuance.

[ask-bonk]

Review posted to PR #2360.

Summary

The author already addressed all of the prior /bigbonk findings in commit 4a5eb22 ("harden Babel transform paths") — tryRealpathSync, relativeWithinRoot, optional chaining on this.environment, and hoisting srcDir/pagesDir into configResolved. I verified the new test passes and vp check is clean on the file.

Remaining items I flagged (all non-blocking):

1. fs.realpathSync.native(root) at :62/:73 still uses the throwing variant — low risk (root always exists), but inconsistent with the hardening applied everywhere else in the file. :62 also does eager I/O at plugin-array construction that's immediately overwritten.

2. configPath/watcher candidates use non-canonical root while Babel cwd uses canonicalRoot — works because Babel resolves configFile absolutely, but for a symlinked checkout the dev-server config watcher could miss changes to the realpath'd file.

3. Babel scope is broader than Next.js's loader (any in-root source vs. page/app/src module rules) — parity note; in practice config files don't hit this transform.

Verified correct: config-file ordering matches Next.js getBabelConfigFile, babelrc: false + explicit configFile mirrors the loader, the test legitimately exercises ordering + plugin execution + symlink/realpath handling, and the missing-@babel/core error is clear and actionable.

github run

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk re-review — PR #2360 (honor project Babel configs)

Re-reviewed at 3e38d0b, which adds transpilePackages + experimental.forceSwcTransforms support on top of the previously-reviewed Babel transform. The new code is sound and I verified the test passes and vp check is clean on all three changed source files. A couple of small items, plus the carried-over robustness nits.

Verified correct (new code)

• forceSwcTransforms is wired correctly: experimental.forceSwcTransforms === true (next-config.ts:1530) flows into the plugin's getOptions() and short-circuits the transform (babel-config.ts:142). The new forced-swc test proves BABEL_CONFIG_BEFORE survives un-rewritten. Matches Next.js semantics (force SWC even when a custom Babel config is present).
• transpilePackages: the isPathInPackage fast path + resolvePackageRoot fallback (cached in transpilePackageRoots, reset per configResolved) correctly opts in-node_modules package code into the Babel pass. The transpile-package test exercises the vendor/node_modules/... path and the PACKAGE_BABEL_AFTER rewrite. The combined gate at :159-164 (in-root non-node_modules OR transpiled package) reads correctly.
• getOptions() timing: the closure reads nextConfig lazily at transform time; nextConfig is populated in the config hook (index.ts:1377) which always resolves before transform, so there's no stale-default race despite the eager createBabelConfigPlugin(...) call in the plugin array.
• Compiled-babel fallback: resolveBabelCore now falls back to next/dist/compiled/babel/core (:59). I confirmed against next@16.2.7 that this module exists and that import(pathToFileURL(...)) yields module.default.transformAsync (a function), so the module.default ?? module unwrap at :175 picks the right object. Works.

1. Error message is stale after the compiled-babel fallback — minor

The fallback now resolves next/dist/compiled/babel/core, but the missing-dep error (:169-172) still says only "Install @babel/core in the project." If @babel/core is unresolvable, the next step actually tries Next's compiled babel, so this error only fires when both are missing. Wording like "Install @babel/core (or ensure next is installed)" would be more actionable. Cosmetic.

2. fs.realpathSync.native(root) at :99/:111 still uses the throwing variant — carried over

Both the initializer (:99) and configResolved (:111) call fs.realpathSync.native directly, while the rest of this file uses the safe tryRealpathSync. Low risk (process.cwd()/config.root virtually always exist), and :99 is immediately overwritten by :111, but it's inconsistent with the hardening applied everywhere else here:

let canonicalRoot = tryRealpathSync(process.cwd()) ?? process.cwd();
// ...
canonicalRoot = tryRealpathSync(root) ?? root;

This also drops the redundant eager realpath I/O during plugin-array construction.

3. configPath/watcher built from non-canonical root while Babel cwd is canonical — carried over

findBabelConfig(root) (:112) and the configureServer watcher candidates (:122) use the non-canonical root, but Babel runs with cwd: canonicalRoot (:190). It works because configFile resolves to an absolute path, but for a symlinked checkout the dev-server watcher could miss changes to the realpath'd config file. Computing both from canonicalRoot would close the gap and keep the canonical/non-canonical usage consistent.

Parity note (not a blocker, carried over)

process.browser is still not replaced via this Babel path (Next's loader injects transform-define per file). Pre-existing gap; flagging only so it isn't assumed closed here.

Nothing blocking. Item 1 is cosmetic; items 2–3 are cheap consistency/robustness fixes that the new commit didn't pick up.

[ask-bonk]

Review posted to PR #2360.

Summary

Re-reviewed at 3e38d0b, which adds transpilePackages + experimental.forceSwcTransforms support on top of the previously-reviewed Babel transform. I ran the new test (honors a project Babel config before the JSX transform — passes) and vp check on all three changed source files (clean).

Verified correct (the new code the prior bonk reviews didn't cover):

• forceSwcTransforms correctly short-circuits the transform; the new forced-swc test proves it.
• transpilePackages opt-in logic (isPathInPackage fast path + cached resolvePackageRoot fallback + the combined in-root/transpiled gate) reads correctly and is exercised by the transpile-package test.
• getOptions() reads nextConfig lazily at transform time — no stale-default race despite eager plugin construction.
• The next/dist/compiled/babel/core fallback works: I confirmed against next@16.2.7 that the module exists and module.default.transformAsync is the right unwrap.

Items flagged (all non-blocking):

1. Stale error message — after adding the compiled-babel fallback, the missing-dep error still only says "Install @babel/core" (cosmetic).
2. fs.realpathSync.native(root) at :99/:111 still throwing variant vs. the safe tryRealpathSync used everywhere else (carried over).
3. configPath/watcher use non-canonical root while Babel cwd is canonical — symlinked-checkout watcher gap (carried over).

Plus the pre-existing process.browser parity note. Nothing blocks the merge.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk re-review — PR #2360 (honor project Babel configs)

Re-reviewed at bd7b78b (two new commits since the last bonk pass: dac1be8 "normalize Babel dependency paths" and bd7b78b "harden Babel config resolution"). All previously-flagged carry-over items are now fixed. I ran the test and vp check — both clean.

Previously flagged → now resolved

1. fs.realpathSync.native(root) throwing variant — fixed. Both the initializer (babel-config.ts:99) and configResolved (:111) now use tryRealpathSync(...) ?? root. No more eager throwing realpath during plugin-array construction.
2. configPath/watcher built from non-canonical root — fixed. findBabelConfig(canonicalRoot) (:112) and the configureServer watcher candidates (:122) now both derive from canonicalRoot, so a symlinked checkout watches/reads the realpath'd config file consistently with Babel's cwd: canonicalRoot.
3. Stale missing-dep error message — fixed. Now reads "Install @babel/core or ensure next is installed in the project." (:170-173), matching the actual @babel/core → next/dist/compiled/babel/core fallback.

Also in dac1be8: isPathInPackage and the in-handler node_modules exclusion were normalized to forward-slash matching (/node_modules/<pkg>/) instead of path.sep-based matching. This is more robust on Windows where Vite ids use forward slashes, and is consistent with relativeWithinRoot's normalization.

Verified correct

• Config-file ordering (BABEL_CONFIG_FILES) matches Next.js getBabelConfigFile; the test's poison babel.config.cjs proves .babelrc selection wins and the unselected config is never loaded.
• babelrc: false + explicit configFile mirrors Next.js's loader (single selected config, no .babelrc auto-discovery).
• forceSwcTransforms short-circuits the transform (:142); the forced-swc test proves BABEL_CONFIG_BEFORE survives un-rewritten.
• transpilePackages opt-in (isPathInPackage fast path + cached resolvePackageRoot fallback at :149-159, reset per configResolved) opts in-node_modules package code into the Babel pass; the transpile-package test exercises the vendor/node_modules/... path.
• getOptions() timing — reads nextConfig lazily at transform time; nextConfig is assigned in the config hook (index.ts:1393), which always runs before transform, so no stale-default race despite the eager createBabelConfigPlugin(...) in the plugin array.
• No JSX double-transform — enforce: "pre" + placement before the React plugin / jsx-in-js means next/babel strips JSX before downstream OXC passes see it.
• New test passes; vp check clean on all three changed source files.

Minor / non-blocking

• Canonical vs. non-canonical mix in the node_modules gate (:146-165): isProjectFile uses canonicalFilename (:148) while the node_modules exclusion uses normalizedFilename (non-canonical, :161) and isPathInPackage uses the raw filename (:150). This is internally consistent — the literal-path heuristics are paired with the relativeWithinRoot(packageRoot, canonicalFilename) realpath fallback — so I don't see a correctness bug, but it's a subtle mix worth a one-line comment for future readers.

Parity note (pre-existing, not introduced here)

process.browser is still not replaced via this Babel path (Next's loader injects transform-define per file). Flagging only so it isn't assumed closed by this PR.

Nothing blocking — the PR is in good shape.

[ask-bonk]

Review posted to PR #2360.

Summary

Re-reviewed at bd7b78b — two new commits since the last bonk pass (dac1be8, bd7b78b). I ran the new test (honors a project Babel config before the JSX transform — passes) and vp check on all three changed source files (clean).

All previously-flagged carry-over items are now resolved:

1. fs.realpathSync.native(root) at :99/:111 → now tryRealpathSync(...) ?? root
2. configPath/watcher → now derived from canonicalRoot (closes the symlinked-checkout watcher gap)
3. Stale missing-dep error message → now mentions both @babel/core and next

Plus dac1be8 normalized Windows path matching (/node_modules/ forward-slash instead of path.sep).

Verified correct: config-file ordering matches Next.js getBabelConfigFile, babelrc: false + explicit configFile mirrors the loader, forceSwcTransforms short-circuits correctly, transpilePackages opt-in works, getOptions() has no stale-default race, and no JSX double-transform.

Only remaining item (non-blocking): a subtle canonical/non-canonical path mix in the node_modules gate — internally consistent (no correctness bug), worth a clarifying comment. Plus the pre-existing process.browser parity note.

Nothing blocks the merge.

github run

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review posted to PR #2360.

Summary

Re-reviewed at f0ed247 ("support linked Babel sources") — one new commit since the last bonk pass at f09acb4. I ran the test (honors a project Babel config before the JSX transform — passes) and vp check on all three changed source files (clean).

New commit verified correct:

• The resolvePackageRoot rewrite (walking require.resolve.paths() instead of up from require.resolve(packageName)) is a genuine robustness improvement — it handles transpiled-only packages whose main/exports aren't resolvable as specifiers, plus symlinked/junctioned node_modules (the test uses a junction).
• includesExternalDirs = externalDir || transpilePackages.length > 0 now exactly mirrors Next's shouldIncludeExternalDirs (webpack-config.ts:484-485), and the updated external-source test faithfully reproduces it.

Items flagged:

1. isTranspiledPackage parity gap (minor): vinext checks only raw transpilePackages, but Next's node_modules exclusion uses finalTranspilePackages = raw + DEFAULT_TRANSPILED_PACKAGES. vinext has geist as a default, so a custom-.babelrc + geist project would diverge. (Note: includesExternalDirs correctly uses raw transpilePackages — different distinction.)
2. Client Fast Refresh dropped in dev (carried over, still unaddressed): the babel plugin runs enforce: "pre" before @vitejs/plugin-react, stripping JSX before its OXC refresh pass — so a .babelrc project silently loses Fast Refresh in vp dev. Live in dev, prod-only tests won't catch it. The most important item to decide on before merge.
3. @babel/core imported before the environmentConfig bail (minor): trivial reorder.

Plus carried-over non-blocking notes (canonical/non-canonical path mix needing a comment; process.browser not closed by this path).

Everything from prior passes re-verified clean. The new commit improves parity; items 1–2 are the ones worth a decision before merge.

github run

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk re-review — PR #2360 (honor project Babel configs)

Re-reviewed at 07a60f3, two new commits since the last bonk pass: 8cb7054 ("preserve Babel fast refresh") and 07a60f3 ("align Babel package selection"). All three items flagged in the previous pass are now resolved. I ran the test (honors a project Babel config before the JSX transform — passes) and vp check on all three changed source files (clean).

Previously flagged → now resolved

1. isTranspiledPackage ignored DEFAULT_TRANSPILED_PACKAGES — fixed in 07a60f3. The plugin's transpilePackages is now turbopackTranspilePackages (= raw transpilePackages + DEFAULT_TRANSPILED_PACKAGES) concatenated with optimizePackageImports (index.ts:1192-1195). I verified this exactly matches Next's finalTranspilePackages = (config.transpilePackages || []).concat(DEFAULT_TRANSPILED_PACKAGES) then appended optimizePackageImports (webpack-config.ts:353-358), which is the list Next uses for the node_modules exclude opt-in. So a custom-.babelrc + geist project now runs Babel over geist in both. Correctly keeps includeExternalDirs keyed off raw transpilePackages (index.ts:1189-1190), matching shouldIncludeExternalDirs = config.experimental.externalDir || !!config.transpilePackages (webpack-config.ts:373).

2. Client Fast Refresh dropped in dev — fixed in 8cb7054. The plugin now resolves next/dist/compiled/react-refresh/babel (:67-75) and injects it into the Babel plugins when isDev && !isServer (:207-210). I confirmed this mirrors Next's loader exactly: it injects the same next/dist/compiled/react-refresh/babel with skipEnvCheck: true, gated on hasReactRefresh (dev && client) (babel/loader/get-config.js:128-135).

3. @babel/core imported before the environmentConfig bail — fixed. The this.environment?.config bail (:177-178) now runs before the dynamic @babel/core import (:180+), so the import is skipped when there's no environment config.

One thing worth confirming before merge (dev-only, not covered by CI)

Possible double react-refresh injection in dev. This is the flip side of the 8cb7054 fix and is vinext-specific (Next doesn't run a second React plugin). The Babel plugin runs enforce: "pre" and now injects react-refresh/babel, then @vitejs/plugin-react (next in the array, index.ts:1198) runs its own OXC-based refresh pass over the Babel output. In Next there's exactly one transform, so this conflict can't arise. After Babel, JSX is already lowered to jsx()/createElement, so plugin-react's refresh pass may skip the file — but if both inject the $RefreshReg$/$RefreshSig$ boilerplate, you can get duplicate registration or broken HMR. The new tests are production-only (build(...), where isDev is false and refresh is never injected), so neither this path nor the fast-refresh fix itself is exercised by CI. Recommend a manual vp dev smoke test of a fixture with a .babelrc + a client component (edit-and-save → confirm HMR, not full reload, and no duplicate-refresh console errors). If plugin-react does double-process, consider gating one side off.

Minor / non-blocking

• transpilePackages: [] (explicitly empty) edge case. Next's shouldIncludeExternalDirs uses !!config.transpilePackages, and !![] === true, so explicitly setting an empty array flips external-dir inclusion on in Next. vinext uses .length > 0 (index.ts:1190), so the empty-array case stays false. The no-config case matches (Next leaves transpilePackages undefined → false). Extremely narrow; vinext's behavior is arguably more sensible, just noting the divergence.
• Canonical/non-canonical path mix in the node_modules gate (:155-167): isProjectFile uses canonicalFilename, the node_modules exclusion uses normalizedFilename (non-canonical), isPathInPackage uses raw filename. Internally consistent (the literal-path heuristics are paired with the relativeWithinRoot(packageRoot, canonicalFilename) realpath fallback) — no correctness bug, but a one-line comment would help future readers.
• process.browser is still not replaced via this Babel path (Next's loader injects transform-define per file). Pre-existing gap; flagging only so it isn't assumed closed here.

Re-verified clean at this SHA

• Config-file ordering (BABEL_CONFIG_FILES) matches Next's getBabelConfigFile; the poison babel.config.cjs proves .babelrc selection wins.
• babelrc: false + explicit configFile mirrors the loader.
• forceSwcTransforms short-circuits the transform (:151); the forced-swc test proves BABEL_CONFIG_BEFORE survives un-rewritten.
• serverTarget → "webworker" for Cloudflare matches Next's edge compiler target; client stays "web".
• resolvePackageRoot via require.resolve.paths() handles symlinked/junctioned node_modules (the test uses a junction) and transpiled-only packages.
• tryRealpathSync/relativeWithinRoot hardening, canonical watcher candidates, and the @babel/core → next/dist/compiled/babel/core fallback with the corrected error message are all in place.

Bottom line: the two new commits resolve every prior finding and the parity is now exact against Next 16.2.7's webpack-config. The only thing I'd want a human decision on is the dev double-refresh question (item above) — it's not blocking, but it's the one path the prod-only tests can't cover.

[ask-bonk]

Review posted to PR #2360.

Summary

Re-reviewed at 07a60f3 — two new commits since the last bonk pass (8cb7054 fast refresh, 07a60f3 package selection). I ran the test (honors a project Babel config before the JSX transform — passes) and vp check on all three changed source files (clean).

All three prior findings are now resolved, verified against Next 16.2.7's actual webpack-config.js and babel loader:

1. DEFAULT_TRANSPILED_PACKAGES parity gap — fixed. The plugin's transpilePackages is now turbopackTranspilePackages (raw + geist) + optimizePackageImports, exactly matching Next's finalTranspilePackages (webpack-config.ts:353-358). includeExternalDirs correctly stays keyed off raw transpilePackages (matching shouldIncludeExternalDirs at :373).
2. Dev Fast Refresh dropped — fixed. Now injects next/dist/compiled/react-refresh/babel with skipEnvCheck: true gated on isDev && !isServer, mirroring Next's loader (get-config.js:128-135).
3. @babel/core imported before the env bail — fixed (bail reordered above the import).

One item worth a human decision (not blocking): possible double react-refresh injection in dev — the flip side of the fast-refresh fix. The Babel plugin (enforce: "pre") now injects refresh, then @vitejs/plugin-react runs its own OXC refresh pass over the output. This is vinext-specific (Next has one transform). The new tests are production-only, so neither this path nor the fast-refresh fix is exercised by CI. Recommended a manual vp dev smoke test with a .babelrc + client component.

Plus minor non-blocking notes (the transpilePackages: [] truthiness edge case vs Next's !!, a canonical/non-canonical path-mix comment, and the pre-existing process.browser gap).

Nothing blocks the merge.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk re-review — PR #2360 (honor project Babel configs)

Re-reviewed at 68e1c0c, two new commits since the last bonk pass at 07a60f3: 2e061d0 ("skip Babel resource queries") and 68e1c0c ("normalize Babel dynamic metadata"). Both are sound. I ran the relevant tests and vp check — all clean.

2e061d0 — skip Babel resource queries — verified correct

• VITE_SPECIAL_QUERY_RE = /[?&](?:worker|sharedworker|raw|url)\b/ (babel-config.ts:19) is a byte-for-byte copy of Vite's own SPECIAL_QUERY_RE (confirmed in @voidzero-dev/vite-plus-core/.../logger.js:194). Using the authoritative regex is the right call for parity.
• This is a real bug fix, not just hardening. The transform strips the query (id.replace(/\?.*$/, "")) before tryRealpathSync, so a ?raw/?url import points at a file that does exist on disk — realpath wouldn't throw — but Babel-transforming the contents of a ?raw import would corrupt the raw-string payload (and ?worker/?url similarly should pass through untouched). Skipping them is correct.
• The guard is placed in the early bail alongside forceSwcTransforms / \0 (:156-163), so it short-circuits before any I/O. Good.
• The test exercises isViteSpecialQuery directly for ?raw, ?url&inline, ?worker, ?sharedworker (true) and ?__rsc (false). Reasonable unit coverage of the exported helper.

68e1c0c — normalize Babel dynamic metadata — verified correct

This handles the case where a project's Babel config runs Next's react-loadable plugin, which emits loadableGenerated: { webpack: () => [require.resolveWeak(...)], modules: [...] } into dynamic() calls. Previously vinext's shouldSkipCall bailed whenever any loadableGenerated was present, leaving Next's require.resolveWeak/webpack artifacts in the output (broken on Workers). Now it distinguishes Next-generated metadata (overwrite) from user-authored metadata (preserve).

• isNextBabelLoadableGenerated (:564) correctly fingerprints Next's output: an ObjectExpression with a webpack property (the strong signal), or a modules ArrayExpression containing a BinaryExpression or a string with " -> " (Next's module-id format). Both are signatures a hand-written config wouldn't produce.
• The preserve path is intact: a user-authored loadableGenerated: { modules: ["custom"] } has no webpack and no " -> ", so isNextBabelLoadableGenerated returns false → the call is skipped (verified by the still-passing preserves existing explicit loadableGenerated metadata test). The new replaces next/babel loadableGenerated metadata test proves the webpack/resolveWeak form is overwritten and resolveWeak is gone from the output.
• The overwrite uses the property node's absolute start/end and bails if either is null (:601-603), so it works regardless of which argument (first or second) carries the property. Shorthand/computed loadableGenerated keys are correctly excluded (propertyKeyName returns null for computed; a shorthand value is an Identifier, not an ObjectExpression, so isNextBabelLoadableGenerated → false → skip).

Carried-over (non-blocking, unchanged this pass)

• Possible double react-refresh injection in dev (from 8cb7054): the Babel plugin (enforce: "pre") injects react-refresh/babel, then @vitejs/plugin-react runs its own OXC refresh pass over the output. Next has a single transform so this can't arise there. The new tests are production-only, so this dev path still isn't covered by CI — a manual vp dev smoke test of a fixture with .babelrc + a client component (confirm HMR, not full reload, no duplicate-refresh errors) is the remaining thing worth a human check.
• transpilePackages: [] truthiness vs Next's !! (index.ts:1190 uses .length > 0) — extremely narrow, vinext's behavior is arguably more sensible.
• Canonical/non-canonical path mix in the node_modules gate (babel-config.ts:167-184) — internally consistent, no correctness bug, a one-line comment would help future readers.
• process.browser still not replaced via this Babel path (Next injects transform-define per file). Pre-existing; flagging only so it isn't assumed closed here.

Re-verified clean at this SHA

• honors a project Babel config before the JSX transform passes; next/dynamic preload metadata transform suite (23 tests) passes; vp check clean on all four changed source files.
• Config-file ordering, babelrc: false + explicit configFile, forceSwcTransforms short-circuit, transpilePackages opt-in (raw+geist+optimizePackageImports matching Next's finalTranspilePackages), serverTarget → "webworker" for Cloudflare, and the @babel/core → next/dist/compiled/babel/core fallback all remain correct.

Bottom line: both new commits are correct bug fixes (the ?raw/?url skip and the Next-vs-user loadableGenerated distinction) with appropriate test coverage. Nothing blocking. The only thing still worth a human decision before merge is the dev double-refresh question, which the prod-only tests can't cover.

[ask-bonk]

Review posted to PR #2360.

Summary

Re-reviewed at 68e1c0c — two new commits since the last bonk pass (07a60f3): 2e061d0 ("skip Babel resource queries") and 68e1c0c ("normalize Babel dynamic metadata"). I ran the Babel test, the next/dynamic preload metadata transform suite (23 tests), and vp check on all four changed source files — all clean.

Both new commits verified correct:

1. 2e061d0 (skip resource queries) — VITE_SPECIAL_QUERY_RE is a byte-for-byte copy of Vite's own SPECIAL_QUERY_RE (confirmed in the bundled vite-plus-core source). This is a real bug fix: the transform strips the query before realpath, so a ?raw/?url/?worker import points at a file that exists on disk, but Babel-transforming a raw-string import would corrupt it. Skipping is correct, placed in the early bail before any I/O.

2. 68e1c0c (normalize dynamic metadata) — correctly distinguishes Next's react-loadable output ({ webpack: () => [...resolveWeak...] }) which must be overwritten, from user-authored { modules: [...] } which must be preserved. The fingerprint (webpack property OR modules with BinaryExpression/" -> " strings) is sound, the overwrite uses absolute node positions, and shorthand/computed keys are safely excluded. Both the new replaces next/babel loadableGenerated metadata test and the still-passing preserves existing explicit loadableGenerated metadata test confirm both paths.

Non-blocking items (carried over, unchanged this pass): possible dev double react-refresh injection (prod-only tests can't cover it — worth a manual vp dev smoke test), the transpilePackages: [] truthiness nuance, the canonical/non-canonical path-mix comment, and the pre-existing process.browser gap.

Nothing blocks the merge.

github run

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk re-review — PR #2360 (honor project Babel configs)

Re-reviewed at da5cb86, two new commits since the last bonk pass at 68e1c0c: 8dc139b ("preserve custom dynamic metadata") and da5cb86 ("narrow Babel metadata detection"). Both tighten the loadableGenerated fingerprint to avoid clobbering user-authored metadata. I verified the new detection against Next 16.2.7's actual react-loadable babel plugin source (next/dist/build/babel/plugins/react-loadable-plugin.js), ran the next/dynamic preload metadata transform suite (25 tests) and the Babel pages-router test — all pass, vp check clean on both changed source files.

New commits verified correct

8dc139b — webpack form now requires the require.resolveWeak signature. Previously any loadableGenerated with a webpack property was treated as Next-generated (hasObjectProperty(value, "webpack")); now isNextBabelWebpackCallback requires the value to be an arrow/function returning a non-empty ArrayExpression whose every element is a require.resolveWeak(...) call. I confirmed this is exactly the shape Next emits (react-loadable-plugin.js:126-130): webpack: () => [require.resolveWeak(importArg), ...]. The block-body branch (single ReturnStatement) is defensive for transpiled variants — Next only emits the expression-body form, so this is a correct superset. The new preserves custom webpack loadableGenerated metadata test (webpack: () => ["custom-chunk"]) proves a user's non-resolveWeak webpack callback is now preserved.

da5cb86 — modules form now requires the Next " -> " binary-expression signature. The previous check treated any BinaryExpression element as Next-generated; now isNextBabelModuleElement requires operator +, a left string literal ending in " -> ", and a string right operand. I confirmed against react-loadable-plugin.js:119: Next emits t.binaryExpression('+', t.stringLiteral(<srcDir-relative-path> + ' -> '), node). The string-literal fast path (stringValue.includes(" -> ")) covers the constant-folded variant. The new preserves custom binary-expression loadableGenerated metadata test (modules: [prefix + "dynamic-widget"]) proves an arbitrary + expression is no longer mistaken for Next output.

Both narrowings move toward Next parity: Next's loader skips generation entirely when loadableGenerated already exists (react-loadable-plugin.js:86-88), so the only metadata vinext should overwrite is the metadata Next's own babel pass injected upstream. Tighter fingerprints reduce false positives. The replaces next/babel loadableGenerated metadata test still proves the genuine require.resolveWeak form is overwritten and resolveWeak is stripped.

Minor / non-blocking

1. No overwrite test for the Next modules binary-expression form. There's a test that preserves a non-Next modules binary expression, and one that overwrites the webpack form, but none that overwrites a genuine Next modules: ["<path> -> ./x"] form (the dev/server path, react-loadable-plugin.js:123-124). The detection is exercised by the preserve test, but a positive overwrite test for the modules shape would close the symmetry and guard against future regressions on the dev/server branch.

2. modules right-operand assumed to be a string. isNextBabelModuleElement requires nodeStringValue(element.right) !== null, i.e. a string-literal import specifier. Next builds the right operand from importArguments[0].node (:117-119), which is a string literal for import("./x") but could be a template/expression for import(./x). In that case the element would not be recognized and the require.resolveWeak-free modules form would pass through un-rewritten. Extremely narrow; Next's own preload would behave similarly, so this is parity-neutral, just noting the assumption.

3. Hand-written Next-shaped metadata is overwritten (parity divergence). Next preserves any existing loadableGenerated regardless of shape (:86-88). vinext now overwrites it whenever it matches the Next fingerprint, so a user who hand-writes loadableGenerated: { webpack: () => [require.resolveWeak("./x")] } gets it overwritten where Next would keep it. This is unavoidable — vinext can't distinguish "Next's babel pass generated this" from "user typed the identical thing" since the user's babel config runs first. The fingerprint is the best achievable heuristic; flagging only as a documented, intentional tradeoff.

Carried over (unchanged this pass, non-blocking)

• Possible double react-refresh injection in dev (from 8cb7054): the Babel plugin (enforce: "pre") injects react-refresh/babel, then @vitejs/plugin-react runs its own OXC refresh pass over the output. The new tests are production-only, so this dev path still isn't covered by CI — a manual vp dev smoke test of a fixture with .babelrc + a client component remains the one thing worth a human check.
• Canonical/non-canonical path mix in the babel-config node_modules gate — internally consistent, a one-line comment would help future readers.
• process.browser still not replaced via this Babel path (Next injects transform-define per file). Pre-existing; flagging only so it isn't assumed closed here.

Re-verified clean at this SHA

• Config-file ordering (BABEL_CONFIG_FILES) matches Next's getBabelConfigFile; babelrc: false + explicit configFile mirrors the loader.
• forceSwcTransforms short-circuits the transform; the forced-swc test proves it.
• transpilePackages opt-in uses raw+geist+optimizePackageImports matching Next's finalTranspilePackages; includeExternalDirs correctly keys off raw transpilePackages.
• serverTarget → "webworker" for Cloudflare matches Next's edge compiler target.
• tryRealpathSync/relativeWithinRoot hardening, canonical watcher candidates, and the @babel/core → next/dist/compiled/babel/core fallback with the corrected error message are all in place.

Bottom line: the two new commits are correct, well-targeted bug fixes that improve parity (verified against Next 16.2.7's actual react-loadable plugin). Nothing blocking. Item 1 (an overwrite test for the modules form) is the cheapest follow-up; the dev double-refresh question remains the one path the prod-only tests can't cover.

[ask-bonk]

Review posted to PR #2360.

Summary

Re-reviewed at da5cb86 — two new commits since the last bonk pass (68e1c0c): 8dc139b ("preserve custom dynamic metadata") and da5cb86 ("narrow Babel metadata detection"). Both tighten the loadableGenerated fingerprint so vinext only overwrites Next-babel-generated metadata, not user-authored metadata. I verified the new detection against Next 16.2.7's actual react-loadable plugin source, ran the metadata suite (25 tests) + the Babel pages-router test (all pass), and vp check is clean.

Both new commits verified correct:

• 8dc139b — the webpack form now requires the full () => [require.resolveWeak(...)] signature (matching react-loadable-plugin.js:126-130), so a user's webpack: () => ["custom-chunk"] is preserved.
• da5cb86 — the modules form now requires Next's "<path> -> " + node binary-expression signature (matching :119), so an arbitrary prefix + "x" is no longer mistaken for Next output.

Both move toward Next parity (Next skips generation when loadableGenerated already exists), and the genuine-overwrite test still passes.

Items flagged (all non-blocking):

1. No positive overwrite test for the Next modules binary-expression form (the dev/server path) — detection is only covered by a preserve test.
2. modules right-operand assumed to be a string literal — narrow edge for template-literal imports.
3. Hand-written Next-shaped metadata gets overwritten where Next would preserve it — unavoidable heuristic limitation, worth documenting as intentional.

Plus carried-over notes: the dev double-refresh question (prod-only tests can't cover it), the canonical/non-canonical path-mix comment, and the pre-existing process.browser gap. Nothing blocks the merge.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk re-review — PR #2360 (honor project Babel configs)

Re-reviewed at 7c8ef94. The one new commit since the last bonk pass (da5cb86) is 7c8ef94 "test(config): cover Babel config defaults" — it just adds externalDir: false / forceSwcTransforms: false assertions to the resolveNextConfig default-shape test. I ran that test (passes), the Babel feature test (honors a project Babel config before the JSX transform — passes), the loadableGenerated cases (pass), and vp check on both changed source files (clean). No source changes since the last pass, so the prior verification all still holds.

Two items I want to surface that earlier passes didn't, plus the standing non-blocking notes.

1. getOptions() allocates on every transform even when no Babel config is present — hot-path (new)

transform.handler calls getOptions() as the very first statement (babel-config.ts:155), before the !configPath bail (:156-163). The factory in index.ts:1187-1196 builds a fresh options object and spreads two arrays into a new transpilePackages array on every call:

transpilePackages: [
  ...(nextConfig?.turbopackTranspilePackages ?? []),
  ...(nextConfig?.optimizePackageImports ?? []),
],

Because the plugin's transform filter matches every .[cm]?[jt]sx? file regardless of whether a Babel config exists, this allocation runs for every JS/TS module in every build/dev session even when the project has no Babel config at all — i.e. the default case for ~all vinext users. configPath is a stable closure value known before getOptions() is needed.

Cheap fix: bail on configPath first, then read options:

async handler(code, id) {
  if (!configPath || id.startsWith("\0") || isViteSpecialQuery(id)) return;
  const options = getOptions();
  if (options.forceSwcTransforms) return;
  // ...
}

This keeps the common no-Babel path allocation-free and matches AGENTS.md's "avoid repeated hot-path work" / "filter before JS runs" guidance. Not blocking, but it's the one concrete perf regression introduced for the majority of users who don't have a Babel config.

2. Next's babelIncludeRegexes default node_modules opt-ins aren't matched — parity note (new, not blocking)

Next's babel-loader excludes node_modules except finalTranspilePackages and a built-in babelIncludeRegexes set (webpack-config.js:165-170, used in the shouldExclude calc at :369):

/next[\\/]dist[\\/](esm[\\/])?shared[\\/]lib/,
/next[\\/]dist[\\/](esm[\\/])?client/,
/next[\\/]dist[\\/](esm[\\/])?pages/,
/[\\/](strip-ansi|ansi-regex|styled-jsx)[\\/]/

So Next runs the user's Babel config over next/dist/{shared/lib,client,pages}, strip-ansi, ansi-regex, and styled-jsx even without listing them in transpilePackages. vinext's gate excludes all /node_modules/ unless the file is in a listed transpiled package (babel-config.ts:181-187), so these are skipped. In practice this is harmless for vinext (it doesn't compile Next internals through this path, and styled-jsx is handled separately), so I'd treat it as a documented intentional divergence rather than something to implement — just flagging it isn't covered.

Carried over (unchanged, non-blocking)

• Possible double react-refresh injection in dev. The Babel plugin (enforce: "pre") injects next/dist/compiled/react-refresh/babel when isDev && !isServer (:219-222), then @vitejs/plugin-react runs its own OXC refresh pass over the Babel output. Next has a single transform so this can't arise there. All the new tests are production-only (build(...), isDev false), so neither the fast-refresh fix nor this interaction is exercised by CI — a manual vp dev smoke test of a fixture with .babelrc + a client component (confirm HMR not full reload, no duplicate $RefreshReg$ errors) remains the one path tests can't cover.
• Canonical/non-canonical path mix in the node_modules gate (:167-187): isProjectFile uses canonicalFilename, the node_modules exclusion uses normalizedFilename, isPathInPackage uses raw filename. Internally consistent (literal-path heuristics paired with the relativeWithinRoot(packageRoot, canonicalFilename) realpath fallback) — no correctness bug, a one-line comment would help future readers.
• process.browser still isn't replaced via this Babel path (Next injects transform-define per file). Pre-existing; flagging only so it isn't assumed closed here.

Re-verified clean at this SHA

• Config-file ordering (BABEL_CONFIG_FILES) matches Next's getBabelConfigFile; babelrc: false + explicit configFile mirrors the loader.
• forceSwcTransforms short-circuits; transpilePackages opt-in uses raw+geist+optimizePackageImports — I re-confirmed against the installed next/dist/build/webpack-config.js:352-356 that this exactly matches finalTranspilePackages, and that includeExternalDirs correctly keys off raw transpilePackages (matching shouldIncludeExternalDirs).
• serverTarget → "webworker" for Cloudflare matches Next's edge compiler target.
• loadableGenerated fingerprint correctly overwrites the genuine require.resolveWeak / " -> " Next forms and preserves user-authored webpack: () => [...] / prefix + "x" metadata; tryRealpathSync/relativeWithinRoot hardening and the @babel/core → next/dist/compiled/babel/core fallback are all in place.

Bottom line: no source changed since the last pass, so the PR remains in good shape. The only newly-surfaced item worth acting on is #1 (the per-transform allocation on the no-Babel-config common path) — a small, cheap fix. #2 and the carried-over notes are non-blocking. The dev double-refresh interaction is still the one thing the prod-only tests can't cover.

[ask-bonk]

Review posted to PR #2360.

Summary

Re-reviewed at 7c8ef94. The only commit since the last bonk pass (da5cb86) is a test-only commit adding externalDir/forceSwcTransforms default assertions — no source changed. I re-ran the Babel feature test, the loadableGenerated cases, and the new config-defaults test (all pass), and vp check is clean.

Two newly-surfaced items the prior 9 passes missed:

1. Hot-path allocation on the no-Babel-config common path (worth fixing). getOptions() is called as the first statement in transform.handler, before the !configPath bail, and the factory in index.ts allocates a fresh options object + spreads two arrays into a new transpilePackages array on every JS/TS module — even for the ~all users with no Babel config. Cheap fix: bail on configPath first, then read options. Matches AGENTS.md "avoid repeated hot-path work."

2. Next's babelIncludeRegexes default node_modules opt-ins aren't matched (parity note, not blocking). Next runs the user Babel config over next/dist/{shared/lib,client,pages}, strip-ansi, ansi-regex, styled-jsx even without transpilePackages. vinext skips them. Harmless for vinext in practice — flagging as documented divergence.

Verified correct (re-confirmed against the installed next/dist/build/webpack-config.js): finalTranspilePackages mapping (raw+geist+optimizePackageImports), includeExternalDirs keying off raw transpilePackages, serverTarget → "webworker", config-file ordering, and the loadableGenerated overwrite/preserve fingerprints.

Carried over (non-blocking): the dev double-refresh interaction (prod-only tests can't cover it — manual vp dev smoke test recommended), the canonical/non-canonical path-mix comment, and the pre-existing process.browser gap.

The only newly actionable item is #1 (a small perf fix). Nothing blocks the merge.

github run

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk re-review — PR #2360 (honor project Babel configs)

Re-reviewed at 6c2c399 ("perf(build): avoid unused Babel options"), the one new commit since the last bonk pass at 7c8ef94. It directly fixes the hot-path allocation flagged as the only actionable item last pass. I re-ran vp check on all four changed source files (clean), the Babel feature test (honors a project Babel config before the JSX transform — passes), and the loadableGenerated cases (6 pass).

Previously flagged → now resolved

Per-transform allocation on the no-Babel-config common path — fixed in 6c2c399. transform.handler now bails on !configPath || id.startsWith("\0") || isViteSpecialQuery(id) before calling getOptions() (babel-config.ts:155-159), so the getOptions() factory — which spreads turbopackTranspilePackages + optimizePackageImports into a fresh array on every call (index.ts:1192-1195) — no longer runs for the ~all users who have no Babel config. getOptions() has no side effects and forceSwcTransforms is still checked immediately after, so behavior is unchanged. Pure perf win, matches AGENTS.md "avoid repeated hot-path work."

Minor / non-blocking (carried over)

1. No positive overwrite test for the Next modules binary-expression form. isNextBabelModuleElement (dynamic-preload-metadata.ts:572-579) detects Next's dev/server react-loadable shape (t.binaryExpression('+', t.stringLiteral("<path> -> "), node)), but the test suite only preserves a non-Next prefix + "x" expression — there's no test that overwrites a genuine ["<path> -> ./x"] form and strips it. The webpack form has both preserve and overwrite tests; the modules form is missing the overwrite half. Cheapest follow-up to close the symmetry and guard the dev/server branch against regression.

2. Possible double react-refresh injection in dev (from 8cb7054): the Babel plugin (enforce: "pre") injects next/dist/compiled/react-refresh/babel when isDev && !isServer (:215-218), then @vitejs/plugin-react (next in the array, index.ts:1198) runs its own OXC refresh pass over the Babel output. Next has a single transform so this can't arise there. All the new tests are production-only (build(...), isDev false), so neither the fast-refresh fix nor this interaction is exercised by CI — a manual vp dev smoke test of a fixture with .babelrc + a client component (confirm HMR not full reload, no duplicate $RefreshReg$ errors) remains the one path tests can't cover.

3. Canonical/non-canonical path mix in the node_modules gate (babel-config.ts:163-183): isProjectFile uses canonicalFilename, the node_modules exclusion uses normalizedFilename (non-canonical), isPathInPackage uses raw filename. Internally consistent (literal-path heuristics paired with the relativeWithinRoot(packageRoot, canonicalFilename) realpath fallback) — no correctness bug, a one-line comment would help future readers.

4. process.browser still isn't replaced via this Babel path (Next injects transform-define per file). Pre-existing; flagging only so it isn't assumed closed here.

Re-verified clean at this SHA

• Config-file ordering (BABEL_CONFIG_FILES) matches Next's getBabelConfigFile; babelrc: false + explicit configFile mirrors the loader.
• forceSwcTransforms short-circuits the transform (:159); the transpilePackages opt-in uses raw+geist+optimizePackageImports matching Next's finalTranspilePackages, and includeExternalDirs correctly keys off raw transpilePackages.
• serverTarget → "webworker" for Cloudflare matches Next's edge compiler target; client stays "web".
• resolvePackageRoot via require.resolve.paths() handles symlinked/junctioned node_modules and transpiled-only packages.
• tryRealpathSync/relativeWithinRoot hardening, canonical watcher candidates, and the @babel/core → next/dist/compiled/babel/core fallback with the corrected error message are all in place.
• loadableGenerated fingerprint correctly overwrites the genuine require.resolveWeak / " -> " Next forms and preserves user-authored webpack: () => [...] / prefix + "x" metadata.

Bottom line: the new commit resolves the last actionable finding. Item 1 (an overwrite test for the modules form) is the cheapest remaining follow-up; the dev double-refresh interaction is still the one path the prod-only tests can't cover. Nothing blocking.

[ask-bonk]

Review posted to PR #2360.

Summary

Re-reviewed at 6c2c399 ("perf(build): avoid unused Babel options") — the one new commit since the last bonk pass at 7c8ef94. I ran vp check on all four changed source files (clean), the Babel feature test (passes), and the loadableGenerated cases (6 pass).

The prior pass's only actionable finding is now resolved:

• The per-transform allocation on the no-Babel-config common path is fixed. transform.handler now bails on !configPath/\0/special-query before calling getOptions() (babel-config.ts:155-159), so the array-spreading factory no longer runs for the ~all users without a Babel config. getOptions() has no side effects, so behavior is unchanged — pure perf win.

Items flagged (all non-blocking, mostly carried over):

1. No positive overwrite test for the Next modules binary-expression form — the dev/server react-loadable shape's detection is only covered by a preserve test, not an overwrite test (the webpack form has both). Cheapest follow-up.
2. Possible dev double react-refresh injection — the flip side of the 8cb7054 fix; prod-only tests can't cover it. Manual vp dev smoke test recommended.
3. Canonical/non-canonical path-mix comment.
4. Pre-existing process.browser gap.

Re-verified correct against Next 16.2.7: config-file ordering, finalTranspilePackages mapping, includeExternalDirs keying off raw transpilePackages, serverTarget → "webworker", the @babel/core → compiled-babel fallback, and the loadableGenerated overwrite/preserve fingerprints.

Nothing blocks the merge.

github run