[PR #273] [MERGED] fix(config): pass correct Next.js phase to function-form configs + instrumentation test coverage #431

New issue

Closed

opened 2026-05-06 12:39:47 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 12:39:47 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/273
Author: @NathanDrake2406
Created: 3/5/2026
Status: ✅ Merged
Merged: 3/6/2026
Merged by: @james-elicx

Base: main ← Head: fix/config-phase-instrumentation-tests

📝 Commits (8)

9aa9182 fix(config): pass correct Next.js phase to function-form configs
18755cd test: add instrumentation module test coverage
1da418e fix(test): use shared helper for App Router static export test
f6f23da Merge remote-tracking branch 'upstream/main' into fix/config-phase-instrumentation-tests
d9632f1 fix: address PR review feedback
502ea91 refactor: use phase constants in tests, simplify pure function test setup
e365024 refactor: use shared startFixtureServer helper in static export tests
a7af79c merge: resolve conflict with upstream/main

📊 Changes

5 files changed (+243 additions, -25 deletions)

View changed files

📝 packages/vinext/src/config/next-config.ts (+6 -5)
📝 packages/vinext/src/index.ts (+3 -1)
➕ tests/instrumentation.test.ts (+176 -0)
➕ tests/next-config.test.ts (+53 -0)
📝 tests/static-export.test.ts (+5 -19)

📄 Description

Summary

fix(config): unwrapConfig() hardcoded "phase-development-server" when calling function-form Next.js configs, even during production builds. Apps that branch on phase in next.config.ts (e.g. (phase) => phase === "phase-production-build" ? prodConfig : devConfig) received the wrong config during vinext build. Now maps Vite's env.command to the correct Next.js phase ("build" → "phase-production-build", "serve" → "phase-development-server").
test(instrumentation): Added 11 tests for server/instrumentation.ts which previously had zero coverage. Covers findInstrumentationFile (file discovery priority, src/ fallback, null when absent), runInstrumentation (register lifecycle, onRequestError storage, error isolation), and reportRequestError (handler forwarding, no-op without handler, handler error isolation).
fix(test): Fixed 6 pre-existing failures in static-export.test.ts App Router section. The test used a local startFixtureServer that loaded the fixture's vite.config.ts via configFile, which caused RSC plugin timing issues on Node 25. Switched to the shared startFixtureServerHelper from helpers.ts (uses configFile: false with explicit plugins), matching the pattern used by app-router.test.ts.

Test plan

npx vitest run tests/next-config.test.ts — 3 tests verifying phase propagation (production build phase, dev default, object-form ignores phase)
npx vitest run tests/instrumentation.test.ts — 11 tests covering file discovery, register/onRequestError lifecycle, error isolation
npx vitest run tests/static-export.test.ts — 14 tests now all pass (was 6 failures)
npx vitest run — full suite passes (2182 passed, 0 failed)
pnpm run typecheck — clean

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/273 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 3/5/2026 **Status:** ✅ Merged **Merged:** 3/6/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `fix/config-phase-instrumentation-tests` --- ### 📝 Commits (8) - [`9aa9182`](https://github.com/cloudflare/vinext/commit/9aa9182ffae7a6de4de2410cffc032972e5660b3) fix(config): pass correct Next.js phase to function-form configs - [`18755cd`](https://github.com/cloudflare/vinext/commit/18755cd3c03a25074a771ad78c1681dab3e86679) test: add instrumentation module test coverage - [`1da418e`](https://github.com/cloudflare/vinext/commit/1da418e4fedcf9ba08ac45cb67430ad8f7148677) fix(test): use shared helper for App Router static export test - [`f6f23da`](https://github.com/cloudflare/vinext/commit/f6f23daaa0ebf91a4cb89401db6821da5039a92d) Merge remote-tracking branch 'upstream/main' into fix/config-phase-instrumentation-tests - [`d9632f1`](https://github.com/cloudflare/vinext/commit/d9632f104060caee36cffc0e187113b7d4db3191) fix: address PR review feedback - [`502ea91`](https://github.com/cloudflare/vinext/commit/502ea912e5604fcdb6f6d8767fee5c59475ba33b) refactor: use phase constants in tests, simplify pure function test setup - [`e365024`](https://github.com/cloudflare/vinext/commit/e3650240a45534cde24243b998a62ad5ec7583a8) refactor: use shared startFixtureServer helper in static export tests - [`a7af79c`](https://github.com/cloudflare/vinext/commit/a7af79cd7330a40e89403167070a24c1541d201a) merge: resolve conflict with upstream/main ### 📊 Changes **5 files changed** (+243 additions, -25 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/config/next-config.ts` (+6 -5) 📝 `packages/vinext/src/index.ts` (+3 -1) ➕ `tests/instrumentation.test.ts` (+176 -0) ➕ `tests/next-config.test.ts` (+53 -0) 📝 `tests/static-export.test.ts` (+5 -19) </details> ### 📄 Description ## Summary - **fix(config):** `unwrapConfig()` hardcoded `"phase-development-server"` when calling function-form Next.js configs, even during production builds. Apps that branch on `phase` in `next.config.ts` (e.g. `(phase) => phase === "phase-production-build" ? prodConfig : devConfig`) received the wrong config during `vinext build`. Now maps Vite's `env.command` to the correct Next.js phase (`"build"` → `"phase-production-build"`, `"serve"` → `"phase-development-server"`). - **test(instrumentation):** Added 11 tests for `server/instrumentation.ts` which previously had zero coverage. Covers `findInstrumentationFile` (file discovery priority, src/ fallback, null when absent), `runInstrumentation` (register lifecycle, onRequestError storage, error isolation), and `reportRequestError` (handler forwarding, no-op without handler, handler error isolation). - **fix(test):** Fixed 6 pre-existing failures in `static-export.test.ts` App Router section. The test used a local `startFixtureServer` that loaded the fixture's `vite.config.ts` via `configFile`, which caused RSC plugin timing issues on Node 25. Switched to the shared `startFixtureServerHelper` from `helpers.ts` (uses `configFile: false` with explicit plugins), matching the pattern used by `app-router.test.ts`. ## Test plan - [x] `npx vitest run tests/next-config.test.ts` — 3 tests verifying phase propagation (production build phase, dev default, object-form ignores phase) - [x] `npx vitest run tests/instrumentation.test.ts` — 11 tests covering file discovery, register/onRequestError lifecycle, error isolation - [x] `npx vitest run tests/static-export.test.ts` — 14 tests now all pass (was 6 failures) - [x] `npx vitest run` — full suite passes (2182 passed, 0 failed) - [x] `pnpm run typecheck` — clean --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>