feat(instrumentation): support nextConfig.instrumentationClientInject

[Divkix]

Summary

Adds support for the nextConfig.instrumentationClientInject config option introduced in Next.js. This lets users inject client bootstrap modules (for side effects and an optional onRouterTransitionStart hook) ahead of the user's instrumentation-client file, without modifying source files.

Closes #1326.

Changes

• New virtual module generator — generateInstrumentationClientInjectModule() generates ESM with side-effect imports in array order, then the user's instrumentation-client file last (or vinext/client/empty-module when absent). A single composed onRouterTransitionStart fans out to every module's hook.
• Config types — added instrumentationClientInject to NextConfig and ResolvedNextConfig with default [].
• Vite plugin wiring — new vinext:instrumentation-client-inject plugin with resolveId + load hooks. The virtual module string is precomputed once in config() to avoid rebuilding on every HMR cycle.
• Tests — 7 unit tests covering empty injects, single/multi specifiers, hook ordering, and empty-module fallback.

Verification

• Pure function verified via inline Node assertions
• tsc --noEmit clean (exit 0)
• Follows existing virtual module conventions (\0-prefixed IDs, same as virtual:vinext-* modules)
• Empty instrumentationClientInject is a transparent passthrough (existing resolve.alias continues to handle the user file or empty-module)

Behavioral parity with Next.js

• Injected modules run for side effects before hydration in array order
• User instrumentation-client file runs last
• Composed onRouterTransitionStart fires in config order across all modules
• Bare npm names and relative paths are resolved by Vite's resolver naturally

[pkg-pr-new]

Open in StackBlitz

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

commit: 5ce7226

[james-elicx]

/bigbonk review

[ask-bonk]

Review: feat(instrumentation): support nextConfig.instrumentationClientInject

CI is fully green. The overall approach — precomputing the virtual module once in config() and serving it via a thin resolveId/load pair — is clean and follows the repo's existing virtual-module conventions. I reviewed the Next.js source (next-instrumentation-client-loader.ts, create-compiler-aliases.ts) to verify behavioral parity.

There are a few issues worth addressing before merge, ranging from a behavioral gap to some minor improvements.

Summary of issues

1. Behavioral gap: clientInjectModule not cleared when config reloads — clientInjectModule is set when instrumentationClientInject.length > 0 but never cleared back to null when the config changes to an empty array (e.g., HMR of next.config). Minor in practice since config changes typically restart the dev server, but worth noting.

2. Missing input validation on inject specifiers — Empty strings and non-string values in the array could produce broken imports. Next.js validates via z.array(z.string()) in its config schema.

3. Hook composition diverges slightly from Next.js — Next.js uses mod && mod.onRouterTransitionStart && mod.onRouterTransitionStart(...) (short-circuit AND chain), while vinext uses typeof ... === "function". Functionally equivalent for well-behaved modules, but the typeof check is arguably more defensive.

4. Generated code exports an untyped function — The composed onRouterTransitionStart(url, type) has no type annotations. Not a runtime concern since this is generated code consumed by Vite's pipeline, but worth noting for consistency with the ClientInstrumentationHooks type.

Overall this is a solid first implementation. The issues above are minor enough that they could be addressed in a follow-up.

[ask-bonk]

When injects is empty, this returns "export {};" regardless of whether the user has an instrumentation-client file. The PR description says this is intentional ("the alias already handles the user file or empty-module"), which is correct — the resolve.alias at index.ts:1261 maps private-next-instrumentation-client to the user file or empty-module and takes effect when this plugin returns null from resolveId.

However, this means there's an asymmetry in the hook composition path: when injects are present, the composed onRouterTransitionStart fans out to all modules including the user file. When injects are empty, the user's onRouterTransitionStart flows directly through the alias. Both paths work, but it's worth documenting this explicitly in the function's JSDoc (the current doc hints at it but doesn't state the two resolution paths clearly).

[ask-bonk]

The module string is only computed when instrumentationClientInject.length > 0, but clientInjectModule is never reset to null on subsequent config re-evaluations. In Vite's multi-environment build pipeline, config() can fire multiple times. If nextConfig is already set (the if (!nextConfig) guard at line 1042), this code path is still reached because instrumentationPath/instrumentationClientPath/middlewarePath are intentionally re-evaluated outside that guard.

This is fine today because nextConfig is only resolved once (the guard), so instrumentationClientInject won't change across calls. But if the guard is ever removed (e.g., to support config HMR), clientInjectModule would need an explicit reset:

Suggested change

	instrumentationClientPath,
	clientInjectModule = nextConfig.instrumentationClientInject.length > 0
	? generateInstrumentationClientInjectModule(
	nextConfig.instrumentationClientInject,
	instrumentationClientPath,
	)
	: null;

This makes the assignment unconditional and self-clearing, which is safer against future refactors.

[ask-bonk]

Minor: this validates that instrumentationClientInject is an array, but doesn't validate that each element is a string. Next.js uses Zod (z.array(z.string())) in config-schema.ts to reject non-string elements. Consider filtering:

Suggested change

	instrumentationClientInject: Array.isArray(config.instrumentationClientInject)
	instrumentationClientInject: Array.isArray(config.instrumentationClientInject)
	? (config.instrumentationClientInject as unknown[]).filter(
	(x): x is string => typeof x === 'string'
	)
	: [],

This is consistent with how optimizePackageImports is handled a few lines above (line 1048).

[ask-bonk]

Nit: the generated function signature has no type annotations. This is fine since it's a virtual module consumed by Vite (not user-facing code), and Vite will treat it as JS regardless. But for consistency with ClientInstrumentationHooks, you could annotate the parameters in the generated code:

export function onRouterTransitionStart(url: string, type: string) {

Not blocking — just a polish suggestion.

[ask-bonk]

Good test coverage for the pure code-generation function. One edge case worth adding: what happens when an inject specifier contains characters that could break the generated import (e.g., a specifier with quotes or backslashes)? JSON.stringify handles this correctly, but a test would document the contract:

it("escapes special characters in specifier paths", () => {
  const code = generateInstrumentationClientInjectModule(
    ['./path/with"quote.js'],
    null,
  );
  expect(code).toContain('from "./path/with\\"quote.js"');
});

Also, there's no integration-level test that verifies the Vite plugin wiring end-to-end (i.e., that the virtual module is actually served when instrumentationClientInject is configured). The unit tests cover generateInstrumentationClientInjectModule in isolation, which is great, but a fixture-based test would catch issues like the resolve.alias vs resolveId ordering concern. This could be a follow-up.

[ask-bonk]

Review submitted successfully. Here's a summary of the feedback I posted:

PR #1416 adds support for nextConfig.instrumentationClientInject, a Next.js config option that lets users inject client bootstrap modules ahead of the user's instrumentation-client file. The implementation is clean — it precomputes a virtual ESM module at config time and serves it via a resolveId/load plugin pair.

All CI checks pass. I left 5 inline comments covering:

1. clientInjectModule self-clearing (index.ts:1068) — the conditional assignment should be made unconditional to be safe against future refactors that might remove the nextConfig once-only guard.
2. Input validation (next-config.ts:1141) — individual array elements aren't validated as strings; suggested filtering to match how optimizePackageImports is handled.
3. Documentation of dual resolution paths (instrumentation-client-inject.ts:26) — the empty-injects path vs non-empty path have different resolution mechanics worth documenting.
4. Untyped generated function (instrumentation-client-inject.ts:47) — minor polish suggestion for type annotations.
5. Missing integration test and edge case (instrumentation.test.ts:397) — suggested a special-character escaping test and noted the absence of an end-to-end Vite plugin wiring test.

github run

[james-elicx]

/bigbonk review

[ask-bonk]

Re-review: feat(instrumentation): support nextConfig.instrumentationClientInject

The previous review's feedback has been addressed — input validation filters non-strings, the JSDoc documents the dual resolution paths, and the special-character escaping test was added. The clientInjectModule assignment was already self-clearing via the ternary (I think the original comment misread the code slightly; the assignment is unconditional since it's outside the if (!nextConfig) guard).

I did a deeper pass comparing against the Next.js reference implementation (next-instrumentation-client-loader.ts) and the Next.js E2E tests (test/e2e/instrumentation-client-hook/). The implementation is correct and the plugin wiring is sound — enforce: "pre" ensures the resolveId hook runs before vite:resolve applies the resolve.alias fallback, which is the right ordering.

Two remaining items worth discussing, neither blocking:

1. ESM import ordering vs CJS require ordering — Next.js generates require() calls (CJS), which execute synchronously in source order. This PR generates static import * as ... statements (ESM). The ES module spec evaluates same-level static imports in source order, and Vite/Rolldown preserve this, so in practice the side-effect ordering is correct. But it's a semantic difference worth being aware of if a future edge case surfaces around circular dependencies or async module evaluation.

2. Missing integration test — The unit tests cover the code-generation function thoroughly. There's no integration test that verifies the virtual module is actually served through Vite's plugin pipeline when instrumentationClientInject is configured. The previous review noted this and suggested it as a follow-up, which seems reasonable.

LGTM with the two inline comments below.

[ask-bonk]

Next.js resolves inject specifiers against the project root before emitting require() calls (via this.resolve(rootContext, spec) in the webpack loader). Here, bare specifiers and relative paths are left for Vite's resolver to handle at import resolution time, which should work correctly since the virtual module's importer context is the project root. Just flagging the difference — if a user reports that relative paths in instrumentationClientInject resolve against the wrong directory, this is the place to look.

[ask-bonk]

Nit from the previous review that's still applicable — the generated function has TypeScript type annotations (url: string, type: string) but this is loaded as a virtual module via the load hook, which Vite treats as JavaScript by default (no id.endsWith('.ts') to trigger the TS transform). This works today because Vite 8 / Rolldown processes all modules through OXC regardless, but if you wanted to be explicit you could either:

1. Drop the TS annotations from the generated code (make it plain JS), or
2. Return { code, map: null } with an explicit lang: 'ts' or append .ts to the resolved ID

Not blocking — the current behavior is correct under Vite 8.

[ask-bonk]

Review submitted. Here's a summary of the feedback:

PR #1416 adds support for nextConfig.instrumentationClientInject, reimplementing the Next.js feature that lets users inject client bootstrap modules ahead of instrumentation-client. The previous review's feedback has been addressed in the latest commit.

I verified against the Next.js reference implementation (next-instrumentation-client-loader.ts) and E2E tests. The implementation is correct:

• The enforce: "pre" plugin correctly intercepts private-next-instrumentation-client before vite:resolve applies the resolve.alias fallback
• The clientInjectModule assignment is properly self-clearing (it's outside the if (!nextConfig) guard)
• Input validation filters non-string array elements
• The dual resolution paths are well-documented in the JSDoc

I left 2 non-blocking inline comments:

1. Specifier resolution context — Next.js pre-resolves inject specifiers against the project root via webpack's this.resolve(). This PR leaves them for Vite's resolver, which should work but is a difference to watch if users report relative path issues.

2. TypeScript annotations in generated JS — The generated virtual module contains TS type annotations (url: string, type: string) but is served via load() without an explicit .ts extension. Works under Vite 8/OXC but could be made explicit.

Overall: LGTM. The integration test gap (no fixture-based test for the full plugin pipeline) is the most notable missing piece, but fine as a follow-up.

github run