[PR #1015] [MERGED] refactor: extract instrumentation lazy init to server/instrumentation-runtime.ts #1019

New issue

Closed

opened 2026-05-06 13:11:35 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:11:35 +02:00

Owner

📋 Pull Request Information

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

Base: main ← Head: pr/extract-instrumentation-lazy-init

📝 Commits (1)

40384c9 refactor: extract instrumentation lazy init to server/instrumentation-runtime.ts

📊 Changes

4 files changed (+185 additions, -67 deletions)

View changed files

📝 packages/vinext/src/entries/app-rsc-entry.ts (+17 -34)
➕ packages/vinext/src/server/instrumentation-runtime.ts (+76 -0)
📝 tests/__snapshots__/entry-templates.test.ts.snap (+8 -33)
📝 tests/instrumentation.test.ts (+84 -0)

📄 Description

Testing Kimi2.6

Summary

Extracts the 25-line __ensureInstrumentation block from the generated App Router RSC entry into a normal, typed, unit-testable module: server/instrumentation-runtime.ts.

Principle: Codegen should describe the app shape; normal modules should implement behavior.

What changed

Before

entries/app-rsc-entry.ts emitted ~25 lines of inline generated code for every App Router project that uses instrumentation.ts:

Module-level mutable state (__instrumentationInitialized, __instrumentationInitPromise)
An idempotent async function __ensureInstrumentation()
register() and onRequestError wiring
VINEXT_PRERENDER short-circuit

This is the canonical "imperative shell with mutable bookkeeping" pattern. Embedding it in a template string makes it untestable, hard to review, and easy to drift between dev/prod paths.

After

New module: packages/vinext/src/server/instrumentation-runtime.ts exports ensureInstrumentationRegistered(instrumentationModule).
Generated entry: imports the helper and calls await ensureInstrumentationRegistered(_instrumentation) — two lines instead of twenty-five.
Tests: Added focused unit tests for idempotency, concurrent racing, prerender skipping, and graceful handling of modules without register/onRequestError.

Why this matters

Testability: The lazy-init semantics (shared promise, dedup, env-gate) are now covered by Vitest unit tests instead of only by integration tests.
Entry thinness: The generated RSC entry stays focused on route-specific imports, manifests, and thin closures — exactly what codegen should own.
Dev/prod parity: Since both dev and prod use the same ensureInstrumentationRegistered import, fixes or additions to instrumentation behavior automatically apply everywhere.

Next.js references

Next.js supports instrumentation.ts via a register() hook called once at server startup, plus an optional onRequestError() handler. The semantics we preserve:

Lazy init on first request (not top-level await) to avoid blocking V8 isolate startup — see how Next.js defers instrumentation registration in the dev server:
- packages/next/src/server/dev/next-dev-server.ts
Global wiring for onRequestError so the handler is reachable across module graphs — Next.js stores the hook on a shared instrumentation instance:
- packages/next/src/server/lib/instrumentation/internal-lifecycle.ts
Official docs:
- https://nextjs.org/docs/app/building-your-application/optimizing/instrumentation

Test plan

# New unit tests for the extracted helper
pnpm test tests/instrumentation.test.ts

# Snapshot update for generated entry templates
pnpm test tests/entry-templates.test.ts

# App Router integration (covers instrumentation in dev + prod)
pnpm test tests/app-router.test.ts

All pass locally.

_{🔄 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/1015 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 5/2/2026 **Status:** ✅ Merged **Merged:** 5/2/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `pr/extract-instrumentation-lazy-init` --- ### 📝 Commits (1) - [`40384c9`](https://github.com/cloudflare/vinext/commit/40384c9e1b16cb29972f7e6eca8e456c821d10c7) refactor: extract instrumentation lazy init to server/instrumentation-runtime.ts ### 📊 Changes **4 files changed** (+185 additions, -67 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/entries/app-rsc-entry.ts` (+17 -34) ➕ `packages/vinext/src/server/instrumentation-runtime.ts` (+76 -0) 📝 `tests/__snapshots__/entry-templates.test.ts.snap` (+8 -33) 📝 `tests/instrumentation.test.ts` (+84 -0) </details> ### 📄 Description Testing Kimi2.6 ## Summary Extracts the 25-line `__ensureInstrumentation` block from the generated App Router RSC entry into a normal, typed, unit-testable module: `server/instrumentation-runtime.ts`. **Principle:** Codegen should describe the app shape; normal modules should implement behavior. ## What changed ### Before `entries/app-rsc-entry.ts` emitted ~25 lines of inline generated code for every App Router project that uses `instrumentation.ts`: - Module-level mutable state (`__instrumentationInitialized`, `__instrumentationInitPromise`) - An idempotent `async function __ensureInstrumentation()` - `register()` and `onRequestError` wiring - `VINEXT_PRERENDER` short-circuit This is the canonical "imperative shell with mutable bookkeeping" pattern. Embedding it in a template string makes it untestable, hard to review, and easy to drift between dev/prod paths. ### After 1. **New module:** `packages/vinext/src/server/instrumentation-runtime.ts` exports `ensureInstrumentationRegistered(instrumentationModule)`. 2. **Generated entry:** imports the helper and calls `await ensureInstrumentationRegistered(_instrumentation)` — two lines instead of twenty-five. 3. **Tests:** Added focused unit tests for idempotency, concurrent racing, prerender skipping, and graceful handling of modules without `register`/`onRequestError`. ## Why this matters - **Testability:** The lazy-init semantics (shared promise, dedup, env-gate) are now covered by Vitest unit tests instead of only by integration tests. - **Entry thinness:** The generated RSC entry stays focused on route-specific imports, manifests, and thin closures — exactly what codegen should own. - **Dev/prod parity:** Since both dev and prod use the same `ensureInstrumentationRegistered` import, fixes or additions to instrumentation behavior automatically apply everywhere. ## Next.js references Next.js supports `instrumentation.ts` via a `register()` hook called once at server startup, plus an optional `onRequestError()` handler. The semantics we preserve: - **Lazy init on first request** (not top-level await) to avoid blocking V8 isolate startup — see how Next.js defers instrumentation registration in the dev server: - [`packages/next/src/server/dev/next-dev-server.ts`](https://github.com/vercel/next.js/blob/canary/packages/next/src/server/dev/next-dev-server.ts) - **Global wiring for `onRequestError`** so the handler is reachable across module graphs — Next.js stores the hook on a shared instrumentation instance: - [`packages/next/src/server/lib/instrumentation/internal-lifecycle.ts`](https://github.com/vercel/next.js/blob/canary/packages/next/src/server/lib/instrumentation/internal-lifecycle.ts) - **Official docs:** - https://nextjs.org/docs/app/building-your-application/optimizing/instrumentation ## Test plan ```bash # New unit tests for the extracted helper pnpm test tests/instrumentation.test.ts # Snapshot update for generated entry templates pnpm test tests/entry-templates.test.ts # App Router integration (covers instrumentation in dev + prod) pnpm test tests/app-router.test.ts ``` All pass locally. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>