[james-elicx]

Fixes #1506.

Failing test: test/e2e/babel/index.test.ts — "Babel > Should compile a page with flowtype correctly".

Adds Babel flowtype page compilation support (changes in packages/vinext/src/index.ts + tests/babel-flowtype.test.ts).

Note: recovered from a worker that completed the edit but was cut off before committing; relying on CI (Check/Vitest/E2E) + bonk for verification.

[pkg-pr-new]

Open in StackBlitz

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

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

commit: 1569302

[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

[ask-bonk]

/bigbonk review — PR #1870 (Babel flowtype page compilation)

Verified the new test passes locally (vp test run tests/babel-flowtype.test.ts → 2 passed). Compared against the Next.js reference fixture (test/e2e/babel/). A few correctness and parity issues stand out.

1. Flow-pragma regex produces false positives (correctness — blocking)

/^\s*\/\/\s*@flow\b/m uses the m flag, so ^ matches the start of any line and \s* can span newlines. It matches // @flow anywhere in the file, not just the leading pragma. Flow/Babel only honor a pragma at the top of the file, so this is too loose:

// matches today (all false positives):
const doc = `\n// @flow\n`;                  // inside a template literal
function f() { /* ... */ }
// @flow annotations are cool                // mid-file comment, line 4

Both node -e checks returned true. Consequence: any .js file that happens to contain a line starting with // @flow is routed to the Babel path instead of OXC. When @babel/core is not resolvable (transformWithFlowBabel returns null), the plugin returns null, OXC is skipped, and since vite:oxc excludes .js, raw JSX in that file is never compiled — a regression vs. current behavior.

Suggest anchoring to the very start of the file (after optional leading whitespace/BOM, no m flag) and also accepting the block-comment form, which Flow/Babel both support:

// matches a leading `// @flow` or `/* @flow */` / `/** @flow */` only
/^\uFEFF?\s*(?:\/\/[^\n]*@flow\b|\/\*[\s\S]*?@flow\b)/.test(code)

(At minimum, drop the m flag and anchor to file start.)

2. Block-comment @flow pragma is not detected (parity)

Independent of #1: the regex only handles // @flow. The /* @flow */ and /** @flow */ forms are valid and common (note Next.js's own vendored files use * @flow). Those files will hit OXC and throw Flow is not supported, the exact error this PR targets.

3. PR-time test cannot verify the actual fix

@babel/core is not a dependency of this repo, so in the vitest environment transformWithFlowBabel always takes the return null branch. The test only asserts "OXC is not called" — it never exercises Babel actually stripping Flow types. The test comment and PR body defer real verification to the deploy-suite test/e2e/babel/index.test.ts, but nextjs-deploy-suite.yml runs on main/schedule/workflow_dispatch, not on PRs. So nothing on this PR verifies the end-to-end behavior. Consider adding @babel/core + @babel/preset-flow as devDependencies and writing a test that asserts the Flow annotations are actually stripped and the component still renders.

4. Comment claims JSX gets compiled by Babel — depends on preset (parity)

The doc comment on transformWithFlowBabel states "JSX in the file is also compiled so vite:oxc does not re-parse JSX syntax later." That is only true if the project's Babel config includes a JSX-compiling preset. The Next.js reference .babelrc is ["next/babel", "@babel/preset-flow"] — next/babel does the JSX compile. With only @babel/preset-flow (as the test fixture code and comments imply is typical), JSX is left intact, and downstream vite:oxc excludes .js, so the leftover JSX won't be handled. Also worth noting: next/babel won't resolve in a vinext project, so users porting the Next.js fixture verbatim will hit a Babel config error. The comment should be tempered, or the fallback should re-run the Babel output through OXC for JSX.

5. Minor

• transformWithFlowBabel resolves @babel/core per-call (createRequire + req(...)) on every matching file. Fine functionally, but a small cache (module-level memo of the resolved module / a "not found" flag) would avoid repeated createRequire/resolve work across files in a build.
• Returning null silently when Babel produces no code (e.g. misconfigured preset) means the file falls through to no transform; the "more actionable error later" only materializes if a later pass actually parses it. For a file that matched the pragma but whose JSX isn't compiled, you may get a confusing downstream error rather than a clear one.

Summary

Direction is reasonable, but #1 is a real regression risk (false-positive routing skips OXC), and #2–#3 mean the stated fix isn't fully covered by either the regex or the PR-time tests. Recommend tightening the pragma detection to a leading-pragma anchor (incl. block-comment form) and adding a PR-time test that actually runs Babel.

[ask-bonk]

Posted the review on PR #1870. Summary of findings:

Blocking / correctness

1. False-positive flow-pragma regex — /^\s*\/\/\s*@flow\b/m uses the m flag and \s* spanning newlines, so it matches // @flow anywhere in a .js file (template literals, mid-file comments), not just the leading pragma. Such files get routed to the Babel path; when @babel/core isn't resolvable they fall through with OXC skipped, leaving raw JSX uncompiled — a regression. Verified with node -e.

Parity
2. Block-comment pragma missed — /* @flow */ / /** @flow */ aren't detected and will still hit OXC's Flow is not supported error.
4. JSX-compile claim is preset-dependent — the doc comment claims Babel compiles JSX, but with only @babel/preset-flow (as the test implies) JSX is left intact; vite:oxc then excludes .js. Also next/babel from the real Next.js fixture won't resolve in vinext.

Coverage
3. PR-time test can't verify the fix — @babel/core isn't a repo dep, so the test always hits the return null branch and never exercises Babel. The referenced deploy-suite e2e runs on main/schedule, not PRs, so nothing on this PR verifies end-to-end behavior.

Minor
5. Per-call createRequire/resolve of @babel/core (could memoize); silent null on empty Babel output can yield confusing downstream errors.

I cleaned up the .nextjs-ref clone usage and restored the working tree to a clean state.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #1870 (Babel flowtype page compilation)

The follow-up commit af2c374 does a solid job addressing the prior round of findings. Verified locally:

• vp test run tests/babel-flowtype.test.ts → 22 passed
• vp check on the three changed files → clean (format + lint + types)
• vp test run tests/build-optimization.test.ts → 87 passed

Resolved since last review

• Virtual module imports break esbuild dependency optimization when vinext is installed from npm #1 (false-positive pragma): ✅ The loose /^\s*\/\/\s*@flow\b/m regex is gone. hasFlowPragma is now a proper leading-comment-block tokenizer (BOM → hashbang → whitespace/comments loop → first real token), mirroring hasReactDirective. Verified that @flow in template literals, strings, and mid-file comments all correctly return false.
• Published npm package ships sourcemaps pointing to missing source files #2 (block-comment pragma): ✅ /* @flow */ and /** @flow */ are now detected.
• ci: Bonk should fail faster #4 (JSX-compile claim): ✅ Real fix, not just a comment tweak — after Babel strips Flow types, the code re-runs transformWithOxc({ lang: "jsx" }), so JSX is compiled regardless of whether the project's Babel preset compiles JSX. The doc comment now matches behavior.
• fix: inline source content in sourcemaps to prevent missing source warnings #5 (perf / silent null): ✅ @babel/core resolution is memoized per projectRoot (incl. a cached "not found" miss), and empty Babel output now throws a clear actionable error instead of silently returning null.

Remaining issues

1. PR-time tests still don't exercise the actual Babel transform (coverage — non-blocking, same as prior #3)

@babel/core is not a dependency of this repo, so in the vitest environment transformWithFlowBabel always takes the _babelCoreCache.set(projectRoot, null); return null branch. The 22 new tests only exercise hasFlowPragma (detection) in isolation — none of them runs Babel, strips a Flow type, or asserts the OXC re-pass output. The only end-to-end coverage is the referenced test/e2e/babel/index.test.ts, which lives in the Next.js repo and is run against vinext only by nextjs-deploy-suite.yml (confirmed: on: schedule / workflow_dispatch, not PRs). So no check on this PR verifies that Flow types are actually stripped and the page renders.

Consider adding @babel/core + @babel/preset-flow as devDependencies and a test that drives transformWithFlowBabel (or the plugin transform) on the mycomponent.js fixture, asserting the Flow annotations are gone and the JSX is compiled. That would turn the most important behavior from "verified nightly on main" into "verified on every PR."

2. Source-map chain is broken across the two-stage transform (minor, new)

index.ts:666 requests Babel sourcemaps (sourceMaps: true) but result.map is discarded; only oxcResult.map is returned (index.ts:687). The returned map describes JSX-compiled-output → post-Babel-output, not → original Flow source. For a Flow .js page, stack traces / breakpoints will map to intermediate Babel output rather than the original source. Chaining the two maps (Babel map then OXC map, e.g. via @ampproject/remapping) would preserve original-source mapping. Acceptable to defer, but worth a tracking note.

3. Doc/comment nit

flow-pragma.ts:47 and :60 use /@flow\b/, while the test comment at tests/babel-flowtype.test.ts:140 describes it as /\b@flow\b/. Behavior is correct (@flowtype → false, verified), just a comment mismatch.

Summary

The blocking regression from the prior review (#1) is fixed, and #2/#4/#5 are genuinely resolved. The implementation is in good shape. The one item I'd push on before merge is #1 (real Babel coverage on PRs) — right now the actual fix is only exercised by a nightly suite, so a regression in transformWithFlowBabel would land green on PRs. The sourcemap-chain and comment nits are minor.

[ask-bonk]

Posted the review on PR #1870. Summary:

Resolved since the prior review (follow-up commit af2c374):

• Virtual module imports break esbuild dependency optimization when vinext is installed from npm #1 false-positive pragma — the loose /m-flag regex is replaced by a proper leading-comment-block tokenizer (hasFlowPragma). Verified template literals / strings / mid-file comments all return false.
• Published npm package ships sourcemaps pointing to missing source files #2 block-comment pragma — /* @flow */ and /** @flow */ now detected.
• ci: Bonk should fail faster #4 JSX-compile — actually fixed: after Babel strips Flow types, an OXC lang:"jsx" re-pass compiles JSX regardless of preset.
• fix: inline source content in sourcemaps to prevent missing source warnings #5 perf / silent null — @babel/core resolution memoized per root; empty Babel output now throws a clear error.

Verified locally: 22/22 flowtype tests pass, vp check clean on all 3 files, build-optimization tests pass.

Remaining issues raised:

1. Coverage (non-blocking, carries over from prior fix: exclude vinext from optimizeDeps to prevent virtual module resolution errors #3): @babel/core isn't a repo dep, so in vitest transformWithFlowBabel always hits the return null branch — the 22 tests only exercise detection, never the actual Babel transform. The end-to-end test lives in the Next.js repo and runs only via nextjs-deploy-suite.yml (schedule/dispatch, confirmed not on PRs). Suggested adding @babel/core + @babel/preset-flow devDeps and a test that drives the real transform.
2. Sourcemap chain broken (minor, new): Babel's result.map is requested but discarded; only the OXC map is returned, so maps point to post-Babel output rather than original Flow source.
3. Doc nit: comment says /\b@flow\b/ but code uses /@flow\b/ (behavior correct).

The blocking regression from the previous round is fixed; the main thing I'd push on before merge is getting real Babel coverage to run on PRs.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #1870 (Babel flowtype page compilation)

Reviewed the latest state including the new commit a6df73a (sourcemap chaining + doc fix). Verified locally:

• vp test run tests/babel-flowtype.test.ts → 22 passed
• vp check on all 3 changed files → clean (format + lint + types)
• Confirmed via a runtime probe that transformWithOxc(..., inMap) actually composes the Babel map: the final map's sources traces back to original.flow.js (original Flow source), so the commit-a6df73a sourcemap fix is real, not cosmetic.

Confirmed resolved

• The sourcemap-chain issue from the prior round is genuinely fixed — babelMap is now passed as inMap and OXC composes it. ✅
• Doc/comment nit (/@flow\b/ vs /\b@flow\b/) is corrected in both flow-pragma.ts and the test. ✅
• The earlier blocking false-positive pragma regex remains fixed (hasFlowPragma is a proper leading-comment tokenizer; template literals / strings / mid-file comments all return false). ✅

Remaining issues

1. No PR-time test exercises the actual Babel transform (coverage — non-blocking, carries over)

@babel/core / @babel/preset-flow are still not dependencies of this repo (confirmed not in root or packages/vinext/package.json). In the vitest environment transformWithFlowBabel therefore always hits the _babelCoreCache.set(projectRoot, null); return null branch. The 22 new tests reference transformWithFlowBabel 0 times — they only exercise hasFlowPragma (detection) in isolation. Nothing on this PR runs Babel, strips a Flow annotation, or asserts the OXC re-pass output.

The only end-to-end coverage is test/e2e/babel/index.test.ts, which lives in the Next.js repo and is run against vinext only by nextjs-deploy-suite.yml — confirmed on: schedule (cron) / workflow_dispatch, not pull_request. So a regression in transformWithFlowBabel (or in the new sourcemap chaining) would land green on every PR and only surface in the nightly run on main.

Recommend adding @babel/core + @babel/preset-flow as devDependencies and a test that drives transformWithFlowBabel (or the plugin transform) over the mycomponent.js-style fixture, asserting (a) Flow annotations are stripped, (b) JSX is compiled, and (c) the returned map's sources points at the original file. That converts the most important behavior — including the brand-new sourcemap chaining — from "verified nightly" to "verified per-PR."

2. "OXC will surface Flow is not supported" comment is inaccurate for the null (babel-missing) path (minor)

When @babel/core can't be resolved, transformWithFlowBabel returns null and the vinext:jsx-in-js transform returns without calling transformWithOxc. But Vite 8's built-in vite:oxc excludes .js (exclude: /\.js$/, per the comment at index.ts:1121), so the Flow .js file is then not parsed by OXC at all — it flows through with raw Flow + JSX intact and errors later in the rolldown/RSC analysis pass, not as a clean "Flow is not supported" from OXC. The behavior (some error occurs) is acceptable, but the comments at index.ts:639-641 and index.ts:657 overstate that OXC produces the clear error. Consider tempering the comment, or — better — emitting an explicit actionable error when a Flow pragma is detected but @babel/core is missing (e.g. "install @babel/core + @babel/preset-flow to compile Flow pages"), which would be a clearer DX than a downstream parser error.

Summary

No new blocking issues. The prior blocking regression stays fixed, and commit a6df73a's sourcemap chaining is correct (verified). The one item worth pushing on before merge is #1: the actual Babel transform (now including sourcemap composition) is still only exercised by a nightly suite, so add a PR-time test backed by @babel/core devDeps. #2 is a minor comment/DX nit.

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[james-elicx]

Review addressed in 6479cee — both remaining findings from the latest /bigbonk review:

#1 (PR-time coverage of the actual Babel transform):

• Extracted transformWithFlowBabel into packages/vinext/src/utils/flow-babel.ts so it can be tested in isolation.
• Added a workspace fixture tests/fixtures/babel-flowtype/ that declares @babel/core + @babel/preset-flow + @babel/plugin-syntax-jsx as its own deps (the way a real user project would) with a .babelrc and the Next.js-style mycomponent.js Flow fixture.
• tests/babel-flowtype.test.ts now drives the real two-stage transform: asserts Flow annotations are stripped via the fixture's .babelrc, the OXC re-pass compiles the leftover JSX (automatic runtime), and the composed sourcemap's sources/sourcesContent trace back to the original Flow source — so the sourcemap chaining from a6df73a is now verified on every PR, not just nightly. Also covers the memoized @babel/core-not-found path. 26 tests pass.
• Interesting wrinkle surfaced by the new test: @babel/preset-flow alone cannot parse JSX, so a Flow+JSX project's Babel config necessarily either compiles JSX (next/babel) or parses-but-keeps it (@babel/plugin-syntax-jsx). The fixture uses the latter so the OXC re-pass is meaningfully exercised; doc comments corrected to match.

#2 (babel-missing path / inaccurate "OXC will surface Flow is not supported" comment):

• When a leading @flow pragma is detected but @babel/core is not resolvable, the plugin now falls through to the regular OXC pass instead of skipping the transform entirely. Plain JS that merely mentions @flow in a leading comment still compiles; genuine Flow syntax fails OXC's parse and is rethrown as an actionable error ("install @babel/core and @babel/preset-flow…") with the original parse error as cause — no more confusing downstream rolldown/RSC analysis error.

Also fixed while in there: Babel's root/cwd are now pinned to the project root so .babelrc resolution works when the Vite process is launched from a different directory (Babel's default babelrcRoots only honors .babelrc files in its root package — previously the Babel path would silently load no config in monorepo setups).

Local verification: vp test run tests/babel-flowtype.test.ts → 26 passed; vp check → clean; knip → clean.

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #1870 (Babel flowtype page compilation)

Reviewed the latest state including commit 6479cee (real per-PR Babel coverage + actionable babel-missing error). The two remaining items from the prior round are genuinely addressed.

Verified locally:

• vp install (frozen) → fixture node_modules materialized; @babel/core resolves from tests/fixtures/babel-flowtype/
• vp test run tests/babel-flowtype.test.ts → 26 passed
• vp check on all changed source/test files → clean (format + lint + types)
• vp test run tests/build-optimization.test.ts → 87 passed

Resolved since last review

• Virtual module imports break esbuild dependency optimization when vinext is installed from npm #1 (actual Babel transform now runs on PRs): ✅ The fixture tests/fixtures/babel-flowtype/ is a real workspace package declaring @babel/core + @babel/preset-flow + @babel/plugin-syntax-jsx. transformWithFlowBabel was extracted to utils/flow-babel.ts and the three new tests drive the real two-stage transform: Flow annotations stripped via the fixture .babelrc, JSX compiled by the OXC re-pass (react/jsx-runtime), and the composed sourcemap's sources/sourcesContent traced back to the original Flow source. Confirmed the .github/actions/setup composite runs vp install by default before the Vitest job, so the fixture deps are present and these tests run per-PR — the brand-new sourcemap chaining is now verified on every PR, not just nightly.
• Published npm package ships sourcemaps pointing to missing source files #2 (babel-missing path / inaccurate comment): ✅ When a leading @flow pragma is detected but @babel/core is unresolvable, the plugin now falls through to the regular OXC pass. Plain JS that merely mentions @flow still compiles; genuine Flow syntax fails OXC's parse and is rethrown as an actionable [vinext] ... install @babel/core and @babel/preset-flow error with the original parse error as cause (index.ts:1124-1133). No more confusing downstream rolldown/RSC error.
• Babel root/cwd are pinned to projectRoot, so .babelrc resolution works in monorepo setups where the Vite process launches from a different directory.

Notes / minor (non-blocking)

1. Fixture deps must be installed for the new tests to pass. I reproduced a clean-checkout failure where all 3 real-transform tests returned null because tests/fixtures/babel-flowtype/node_modules wasn't materialized; a frozen vp install fixed it and 26/26 then pass. CI's setup action runs vp install by default so this is covered, but it does mean these tests are sensitive to install state (e.g. a --no-install job would skip-by-null silently rather than fail loudly). Acceptable.
2. _babelCoreCache null (not-found) entries persist for the process lifetime keyed by projectRoot. Fine for a build; in a long-lived dev server, installing @babel/core mid-session wouldn't be picked up without a restart. Worth a one-line note in the doc comment if you want.
3. Transform input id mismatch (cosmetic): transformWithFlowBabel is called with cleanId (query stripped) while the fallback transformWithOxc uses raw id (index.ts:1105 vs 1115). No functional impact (OXC keys the map on filename), just slight inconsistency.

Summary

Both remaining findings from the prior reviews are resolved: the actual Babel transform (including sourcemap composition) is now exercised per-PR via a real-deps fixture, and the babel-missing path produces an actionable error instead of skipping the transform. Implementation is clean, checks pass, no blocking issues. The notes above are minor.

[ask-bonk]

Posted the review on PR #1870.

Summary

The latest commit 6479cee resolves both remaining findings from the prior /bigbonk rounds. I verified everything locally.

Resolved:

• Virtual module imports break esbuild dependency optimization when vinext is installed from npm #1 — Real Babel transform now runs per-PR. tests/fixtures/babel-flowtype/ is a real workspace package declaring @babel/core + @babel/preset-flow + @babel/plugin-syntax-jsx, and transformWithFlowBabel was extracted to utils/flow-babel.ts. The 3 new tests drive the actual two-stage transform: Flow stripping via the fixture .babelrc, JSX compiled by the OXC re-pass, and the composed sourcemap tracing back to the original Flow source. I confirmed .github/actions/setup runs vp install by default before Vitest, so these tests run on every PR — closing the coverage gap that was previously "nightly only."
• Published npm package ships sourcemaps pointing to missing source files #2 — Babel-missing path. When @flow is detected but @babel/core is unresolvable, the plugin falls through to OXC and rethrows genuine Flow parse failures as an actionable install message (with the original error as cause).

Verification: 26/26 flowtype tests pass, 87/87 build-optimization tests pass, vp check clean on all changed files.

Notable wrinkle I hit: On a fresh checkout the 3 real-transform tests failed with null because the fixture's node_modules wasn't materialized; a frozen vp install fixed it. CI covers this via its setup action, but it means these tests skip-by-null rather than fail-loud in a --no-install job. I flagged that plus two minor cosmetic notes (_babelCoreCache not-found entries persisting for the process lifetime; cleanId vs id input mismatch between the Babel and fallback OXC calls).

No blocking issues. The implementation is clean and the prior coverage gap is genuinely closed.

github run

[james-elicx]

Addressed all three minor notes from the last review in df5774c:

1. Tests skip-by-null on a non-installed checkout → now fail loud. The transformWithFlowBabel describe block gains a beforeAll that resolves @babel/core from the fixture root (tests/fixtures/babel-flowtype) and, when it isn't materialized, throws an actionable error telling you to run vp install — instead of three opaque "expected null not to be null" failures.

2. _babelCoreCache no longer pins a not-found result for the process lifetime. Resolution misses are no longer cached, so installing @babel/core mid-dev-session takes effect on the next transform without a restart. Successful resolutions stay memoized, so the hot path (Babel installed, per-module transform) is unchanged; the re-attempt cost is a failed require.resolve and only applies to @flow-pragma files in a project without Babel — which either fail the build anyway (genuine Flow syntax) or are rare pragma-comment-only plain-JS files. Locked in by a new test that "installs" @babel/core after a miss and asserts the next call picks it up.

3. cleanId vs id mismatch fixed. The fallback transformWithOxc call in vinext:jsx-in-js now receives cleanId (query params stripped), matching the filename given to transformWithFlowBabel, so sourcemap filenames and parse errors are consistent across both paths.

Bonus robustness fix found while verifying: vp test launches workers with NODE_PATH pointing at pnpm's hoisted .pnpm/node_modules store. On a fully-hoisted install that makes @babel/core resolvable from any directory, so the babel-missing tests (which rely on an empty temp dir) were environment-sensitive — green in CI, red on a local pnpm install. The two babel-missing tests now neutralize the NODE_PATH fallback (via Module._initPaths(), restored in finally) so they are deterministic regardless of installer hoisting.

Verification: vp test run tests/babel-flowtype.test.ts 27/27, vp test run tests/jsx-in-js.test.ts tests/jsx-in-js-node-modules.test.ts 3/3, vp check clean.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #1870 (Babel flowtype page compilation)

Reviewed the full current state through df5774c. Verified locally:

• vp install (frozen) → fixture node_modules materialized
• vp test run tests/babel-flowtype.test.ts → 27 passed
• vp check on flow-babel.ts, flow-pragma.ts, index.ts, babel-flowtype.test.ts → clean (format + lint + types)

This is the strongest state the PR has been in. All prior /bigbonk findings are genuinely resolved, and the recent df5774c notes are addressed well:

• hasFlowPragma (utils/flow-pragma.ts) is a proper leading-comment tokenizer mirroring hasReactDirective — template literals, strings, and mid-file @flow all correctly return false; // @flow, /* @flow */, /** @flow */, BOM, and hashbang-prefixed pragmas are detected; @flowtype is correctly rejected via \b.
• Two-stage transform (utils/flow-babel.ts): Babel strips Flow types via the project .babelrc, then an OXC lang:"jsx" re-pass compiles leftover JSX; the Babel map is chained into OXC as inMap so the composed sourcemap traces to original Flow source. The fixture (tests/fixtures/babel-flowtype/) is a real workspace package declaring its own @babel/* deps, so the actual transform + sourcemap composition is now exercised per-PR, not just nightly.
• Babel-missing fallthrough: when @flow is detected but @babel/core is unresolvable, the plugin falls through to OXC and rethrows genuine Flow parse failures as an actionable install message with the original error as cause (index.ts:1127-1136). The _babelCoreCache only memoizes hits (misses re-attempt, so mid-session installs take effect), and root/cwd are pinned to projectRoot for monorepo .babelrc resolution.
• The beforeAll fail-loud guard works: on this clean checkout the 3 real-transform tests failed with the actionable "run vp install" error rather than opaque null assertions; after a frozen install all 27 pass.

Notes (all non-blocking)

1. The index.ts actionable-rethrow path has no test coverage. The df5774c/6479cee DX improvement — Flow pragma detected + @babel/core missing + genuine Flow syntax → OXC throws → wrapped [vinext] Failed to compile … requires Babel error (index.ts:1127-1136) — is not exercised by any test. tests/babel-flowtype.test.ts covers transformWithFlowBabel returning null (babel-missing) and the happy path, but never the index.ts catch that turns the OXC parse failure into the actionable message. A small unit test driving the vinext:jsx-in-js transform (or asserting the wrap+cause behavior) would lock in the most user-facing piece of the babel-missing branch. The complementary case — plain JS with a leading @flow comment but no Flow syntax compiling cleanly through the OXC fallback — is also untested at the plugin level.

2. hasFlowPragma runs on every .js/.mjs transform. It short-circuits at the first non-comment token, so cost is bounded to the leading comment block — fine. Just flagging that it's now on the hot path for all user JS files, not only Flow ones.

3. withoutNodePathFallback mutates process.env.NODE_PATH + Module._initPaths() globally. Safe here because fileParallelism: false and the tests within the file run sequentially with finally restoration, but it is process-global state — if these tests ever gain .concurrent or the suite's isolation model changes, this becomes a race. A comment to that effect (or scoping via a non-global resolver) would future-proof it.

Summary

No blocking issues. The blocking regression and parity gaps from earlier rounds are fixed, the real Babel transform + sourcemap chaining is verified per-PR via a real-deps fixture, and the babel-missing path produces an actionable error. The one thing I'd consider before merge is note #1 — adding a test for the index.ts actionable-rethrow path so the babel-missing DX behavior is covered, not just the transformWithFlowBabel null branch. The other two notes are minor.

[ask-bonk]

Review posted to PR #1870.

Summary

Reviewed the full current state through df5774c. Verified locally: vp install (frozen) → fixture deps materialized, vp test run tests/babel-flowtype.test.ts → 27 passed, vp check on all 4 changed source/test files → clean.

All prior /bigbonk findings are genuinely resolved, and the implementation is in good shape:

• hasFlowPragma is a sound leading-comment tokenizer (no false positives on template literals/strings/mid-file comments; handles //, /* */, /** */, BOM, hashbang; rejects @flowtype).
• Two-stage Babel→OXC transform with proper sourcemap chaining, exercised per-PR via a real-deps workspace fixture.
• Babel-missing path falls through to OXC and rethrows an actionable install error with cause; cache only memoizes hits; root/cwd pinned for monorepo .babelrc resolution.
• The beforeAll fail-loud guard worked as intended on this clean checkout (3 tests failed actionably before install, all 27 passed after).

No blocking issues. Three non-blocking notes:

1. The index.ts actionable-rethrow path (Flow pragma + missing @babel/core + genuine Flow syntax → wrapped error) has no test coverage — only the transformWithFlowBabel null branch is tested. Worth a small test before merge.
2. hasFlowPragma now runs on every .js/.mjs transform (bounded/short-circuiting, but on the hot path).
3. withoutNodePathFallback mutates global process.env.NODE_PATH/Module._initPaths() — safe today under fileParallelism: false + sequential tests, but fragile if isolation changes.

Working tree left clean.

github run

[james-elicx]

Round-2 notes addressed in 1569302:

1. Actionable-rethrow path now has unit coverage. The vinext:jsx-in-js transform body (Flow pragma detection → Babel attempt → OXC fallback → actionable rethrow) was extracted into transformJsxInJs in packages/vinext/src/utils/flow-babel.ts; the plugin in index.ts now delegates to it after its extension/node_modules filtering. Three new tests cover the previously untested paths: Flow syntax + missing @babel/core → wrapped "install @babel/core and @babel/preset-flow" error with the OXC parse error preserved as cause; pragma-comment-only plain JS + missing Babel → compiles via the OXC fall-through; and the Babel-installed happy path through the full pipeline.

2. hasFlowPragma on the hot path — keeping as-is: it is a bounded short-circuiting scan of the leading comment trivia only (bails at the first real token), runs once per .js/.mjs transform immediately before a full OXC parse of the same source, so its cost is noise relative to the work that follows. node_modules files are additionally pre-filtered by the cheap directive check before reaching it.

3. withoutNodePathFallback global mutation — kept, but hoisted to module scope with its safety contract documented in the JSDoc: the mutation is restored in finally, tests in this file run sequentially (no it.concurrent), and vitest isolates other test files in separate worker processes. There is no per-require isolation hook in Node for NODE_PATH, so this is the minimal deterministic option.

Verification: vp test run tests/babel-flowtype.test.ts 30/30, vp test run tests/jsx-in-js.test.ts tests/jsx-in-js-node-modules.test.ts 3/3, vp check clean.

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #1870 (Babel flowtype page compilation)

Reviewed the full current state through 1569302. This is the strongest the PR has been; every finding from the prior /bigbonk rounds is genuinely resolved, including the round-2 notes addressed in 1569302.

Verified locally

• vp install (frozen) → fixture node_modules materialized; @babel/core symlinks resolve from tests/fixtures/babel-flowtype/
• vp test run tests/babel-flowtype.test.ts → 30 passed
• vp test run tests/jsx-in-js.test.ts tests/jsx-in-js-node-modules.test.ts → 3 passed
• vp check on flow-babel.ts, flow-pragma.ts, index.ts, babel-flowtype.test.ts → clean (format + lint + types)
• Confirmed the transformWithOxc import was removed from index.ts and is now only referenced in utils/flow-babel.ts — no dangling import.

Confirmed resolved

• Actionable-rethrow path now has unit coverage (prior round-2 note Virtual module imports break esbuild dependency optimization when vinext is installed from npm #1): ✅ The transform body was extracted into transformJsxInJs (utils/flow-babel.ts:127) and the plugin in index.ts:1100 delegates to it after extension/node_modules filtering. The three new transformJsxInJs tests cover the previously untested paths: Flow syntax + missing @babel/core → wrapped "install @babel/core and @babel/preset-flow" error with the OXC parse error preserved as cause; pragma-comment-only plain JS + missing Babel → compiles via the OXC fall-through; and the Babel-installed happy path through the full pipeline.
• Real Babel transform + sourcemap chaining run per-PR: ✅ The fixture is a real workspace package (tests/fixtures/babel-flowtype/, wired into pnpm-workspace.yaml via tests/fixtures/* and recorded in pnpm-lock.yaml). The sourcemap test asserts map.sources includes mycomponent.js and sourcesContent includes the original type Props, proving the Babel map is chained into OXC as inMap rather than discarded.
• hasFlowPragma is a sound leading-comment tokenizer: template literals, strings, and mid-file @flow all correctly return false; //, /* */, /** */, BOM, hashbang are detected; @flowtype is rejected via \b. Verified by the 24 detection cases.
• _babelCoreCache only memoizes hits (misses re-attempt, so a mid-session install takes effect) — locked in by the "picks up @babel/core installed after a prior miss" test. root/cwd pinned to projectRoot for monorepo .babelrc resolution.
• withoutNodePathFallback global NODE_PATH/Module._initPaths() mutation is safe as documented: vite.config.ts sets fileParallelism: false and the file uses no .concurrent, with restoration in finally.
• cleanId vs id mismatch fixed — the fallback OXC call and transformWithFlowBabel both receive cleanId.

Notes (all non-blocking)

1. The beforeAll fail-loud guard is working as intended — but worth knowing CI depends on it. On this fresh checkout, before vp install the three real-transform tests failed with the actionable "run vp install" error (not opaque null assertions), and after a frozen install all 30 passed. CI's setup action runs vp install by default, so this is covered; just flagging that these tests are install-state-sensitive by design.
2. No full-page e2e on this PR. The fixture is unit-test-only (no vite.config, not in the deploy/smoke matrix); end-to-end page rendering remains covered by the Next.js deploy-suite test/e2e/babel/index.test.ts (schedule/dispatch). Given the real two-stage transform + sourcemap composition is now exercised per-PR, this is an acceptable boundary — the transform is the part vinext owns.
3. node_modules Flow files: a .js file in node_modules carrying a @flow pragma but no React directive is still skipped by the pre-filter at index.ts:1085-1092 (returns before the Flow path). This matches the pre-existing node_modules policy (only directive-bearing files are transformed) and is the right call for build perf; just noting the Flow path is effectively first-party/user-code only.

Summary

No blocking issues. The implementation is clean and well-tested: detection, the real Babel→OXC two-stage transform, sourcemap chaining, the babel-missing actionable error, and the cache/monorepo semantics are all exercised per-PR via a real-deps fixture. Checks pass; the three notes above are minor. Working tree left clean.

[ask-bonk]

Posted the review to PR #1870.

Summary

Reviewed the full current state through 1569302. No blocking issues — every finding from the prior /bigbonk rounds is genuinely resolved, including the round-2 notes addressed in 1569302.

Verified locally:

• vp install (frozen) → fixture deps materialized
• tests/babel-flowtype.test.ts → 30 passed
• tests/jsx-in-js.test.ts + tests/jsx-in-js-node-modules.test.ts → 3 passed
• vp check on all 4 changed source/test files → clean

Confirmed resolved:

• The previously-untested index.ts actionable-rethrow path now has coverage — the transform body was extracted into transformJsxInJs and three tests drive it (Flow+missing-Babel → wrapped error with cause; pragma-only plain JS → OXC fall-through; happy path).
• Real Babel→OXC transform + sourcemap chaining run per-PR via the real-deps workspace fixture (verified the composed map traces back to original Flow source).
• hasFlowPragma tokenizer is sound; _babelCoreCache only memoizes hits; monorepo .babelrc resolution pinned; cleanId/id consistency fixed; transformWithOxc import correctly removed from index.ts.
• withoutNodePathFallback global mutation is safe (fileParallelism: false, no .concurrent, finally restore).

Notes (non-blocking):

1. The beforeAll fail-loud guard fired correctly on this clean checkout before install — tests are install-state-sensitive by design; CI covers it.
2. No full-page e2e on the PR (fixture is unit-only); end-to-end remains in the nightly Next.js deploy-suite. Acceptable since the transform vinext owns is now per-PR tested.
3. node_modules Flow files without a React directive are still skipped by the existing pre-filter — matches prior policy, correct for build perf.

Working tree left clean.

github run