Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions packages/vinext/src/client/instrumentation-client-inject.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/**
* Generate a virtual ESM module that implements the Next.js
* `instrumentationClientInject` contract for client bootstrap.
*
* Resolution follows two paths depending on whether injects are configured:
*
* **Empty injects (`injects.length === 0`):** Returns `export {}` and the
* plugin does not serve a virtual module. The `resolve.alias` for
* `private-next-instrumentation-client` resolves directly to the user's
* `instrumentation-client` file (or `vinext/client/empty-module` when absent),
* so the user's `onRouterTransitionStart` is used as-is with no composition.
*
* **Non-empty injects:** The plugin serves this generated module via
* `resolveId`/`load`. It side-effect-imports each inject in config order, then
* the user's file last, and exports a single composed `onRouterTransitionStart`
* that fans out to every module's hook.
*
* @param injects - Module specifiers from `nextConfig.instrumentationClientInject`
* @param userPath - Absolute path to the user's `instrumentation-client` file,
* or `null` when the file doesn't exist
*/
export function generateInstrumentationClientInjectModule(
injects: readonly string[],
userPath: string | null,
): string {
const EMPTY_MODULE = "vinext/client/empty-module";

// No injects: Next.js keeps the current transparent passthrough.
// The alias already handles the user file or empty-module, so emit
// nothing that could shadow what the alias resolves.
if (injects.length === 0) {
return "export {};";
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).


const lines: string[] = [];

for (let i = 0; i < injects.length; i++) {
lines.push(`import * as __vinj_${i} from ${JSON.stringify(injects[i])};`);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

const lastIndex = injects.length;
lines.push(`import * as __vinj_${lastIndex} from ${JSON.stringify(userPath ?? EMPTY_MODULE)};`);

const hookCalls: string[] = [];
for (let i = 0; i <= lastIndex; i++) {
hookCalls.push(
` if (typeof __vinj_${i}.onRouterTransitionStart === "function") {`,
` __vinj_${i}.onRouterTransitionStart(url, type);`,
` }`,
);
}

lines.push("");
lines.push("export function onRouterTransitionStart(url: string, type: string) {");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

lines.push(...hookCalls);
lines.push(`}`);
lines.push("");

return lines.join("\n");
}
13 changes: 13 additions & 0 deletions packages/vinext/src/config/next-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,12 @@ export type NextConfig = {
output?: "export" | "standalone";
/** File extensions treated as routable pages/routes (Next.js pageExtensions) */
pageExtensions?: string[];
/**
* Module specifiers that are required for side effects on the client before
* hydration, in array order, ahead of the user's `instrumentation-client.{ts,js}`.
* Each entry may be a bare npm package name or a path relative to the project root.
*/
instrumentationClientInject?: string[];
/** Extra origins allowed to access the dev server. */
allowedDevOrigins?: string[];
/** Maximum age in seconds for stale ISR entries before blocking regeneration. */
Expand Down Expand Up @@ -290,6 +296,7 @@ export type ResolvedNextConfig = {
trailingSlash: boolean;
output: "" | "export" | "standalone";
pageExtensions: string[];
instrumentationClientInject: string[];
cacheComponents: boolean;
redirects: NextRedirect[];
rewrites: {
Expand Down Expand Up @@ -951,6 +958,7 @@ export async function resolveNextConfig(
buildId,
deploymentId,
sassOptions: null,
instrumentationClientInject: [],
};
detectNextIntlConfig(root, resolved);
return resolved;
Expand Down Expand Up @@ -1138,6 +1146,11 @@ export async function resolveNextConfig(
trailingSlash: config.trailingSlash ?? false,
output: output === "export" || output === "standalone" ? output : "",
pageExtensions,
instrumentationClientInject: Array.isArray(config.instrumentationClientInject)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

? (config.instrumentationClientInject as unknown[]).filter(
(x): x is string => typeof x === "string",
)
: [],
cacheComponents: config.cacheComponents ?? false,
redirects,
rewrites,
Expand Down
34 changes: 34 additions & 0 deletions packages/vinext/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ import { clientReferenceDedupPlugin } from "./plugins/client-reference-dedup.js"
import { dataUrlCssPlugin } from "./plugins/css-data-url.js";
import { createRscClientReferenceLoadersPlugin } from "./plugins/rsc-client-reference-loaders.js";
import { createInstrumentationClientTransformPlugin } from "./plugins/instrumentation-client.js";
import { generateInstrumentationClientInjectModule } from "./client/instrumentation-client-inject.js";
import { createMiddlewareServerOnlyPlugin } from "./plugins/middleware-server-only.js";
import { createOptimizeImportsPlugin } from "./plugins/optimize-imports.js";
import { createOgInlineFetchAssetsPlugin, ogAssetsPlugin } from "./plugins/og-assets.js";
Expand Down Expand Up @@ -438,6 +439,9 @@ const VIRTUAL_APP_BROWSER_ENTRY = "virtual:vinext-app-browser-entry";
const RESOLVED_APP_BROWSER_ENTRY = "\0" + VIRTUAL_APP_BROWSER_ENTRY;
const VIRTUAL_ROOT_PARAMS = "virtual:vinext-root-params";
const RESOLVED_ROOT_PARAMS = "\0" + VIRTUAL_ROOT_PARAMS;
/** Virtual module for composed instrumentation-client bootstrap. */
const VIRTUAL_INSTRUMENTATION_CLIENT = "private-next-instrumentation-client";
const RESOLVED_INSTRUMENTATION_CLIENT = "\0" + VIRTUAL_INSTRUMENTATION_CLIENT;
/** Image file extensions handled by the vinext:image-imports plugin.
* Shared between the Rolldown hook filter and the transform handler regex. */
const IMAGE_EXTS = "png|jpe?g|gif|webp|avif|svg|ico|bmp|tiff?";
Expand Down Expand Up @@ -654,6 +658,7 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
let middlewarePath: string | null = null;
let instrumentationPath: string | null = null;
let instrumentationClientPath: string | null = null;
let clientInjectModule: string | null = null;
let hasCloudflarePlugin = false;
let warnedInlineNextConfigOverride = false;
let hasNitroPlugin = false;
Expand Down Expand Up @@ -1072,6 +1077,13 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
instrumentationPath = findInstrumentationFile(root, fileMatcher);
instrumentationClientPath = findInstrumentationClientFile(root, fileMatcher);
middlewarePath = findMiddlewareFile(root, fileMatcher);
clientInjectModule =
nextConfig.instrumentationClientInject.length > 0
? generateInstrumentationClientInjectModule(
nextConfig.instrumentationClientInject,
instrumentationClientPath,
)
: null;
if (env?.command === "build") {
await writeRouteTypes();
}
Expand Down Expand Up @@ -2321,6 +2333,28 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
// Stub node:async_hooks in client builds — see src/plugins/async-hooks-stub.ts
asyncHooksStubPlugin,
createInstrumentationClientTransformPlugin(() => instrumentationClientPath),
// Generate a virtual `private-next-instrumentation-client` module when
// `nextConfig.instrumentationClientInject` is non-empty. Side-effect imports
// run in array order, ending with the user's `instrumentation-client` file
// (or empty-module), and a single composed `onRouterTransitionStart` fans
// out to every module's hook.
{
name: "vinext:instrumentation-client-inject",
enforce: "pre",

resolveId(id) {
if (id !== VIRTUAL_INSTRUMENTATION_CLIENT) return null;
// The module was generated in config() if there are injects to compose.
// When empty, resolve.alias handles passthrough to the user file or empty-module.
return clientInjectModule !== null ? RESOLVED_INSTRUMENTATION_CLIENT : null;
},

load(id) {
if (id !== RESOLVED_INSTRUMENTATION_CLIENT) return null;
// Deterministic output precomputed once in config().
return clientInjectModule;
},
},
// Dedup client references from RSC proxy modules — see src/plugins/client-reference-dedup.ts
...(options.experimental?.clientReferenceDedup ? [clientReferenceDedupPlugin()] : []),
// Proxy plugin for @mdx-js/rollup. The real MDX plugin is created lazily
Expand Down
58 changes: 58 additions & 0 deletions tests/instrumentation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
findInstrumentationClientFile,
findInstrumentationFile,
} from "../packages/vinext/src/server/instrumentation.js";
import { generateInstrumentationClientInjectModule } from "../packages/vinext/src/client/instrumentation-client-inject.js";
import { createValidFileMatcher } from "../packages/vinext/src/routing/file-matcher.js";

// The runInstrumentation/reportRequestError describe blocks re-import via
Expand Down Expand Up @@ -342,3 +343,60 @@ describe("reportRequestError", () => {
expect(onRequestError).toHaveBeenCalledOnce();
});
});

describe("generateInstrumentationClientInjectModule", () => {
it("returns passthrough when injects is empty", () => {
const code = generateInstrumentationClientInjectModule([], null);
expect(code).toBe("export {};");
});

it("generates a single import for one inject entry", () => {
const code = generateInstrumentationClientInjectModule(["./inject-a.js"], null);
expect(code).toContain('import * as __vinj_0 from "./inject-a.js"');
expect(code).toContain("export function onRouterTransitionStart(url: string, type: string)");
expect(code).toContain('typeof __vinj_0.onRouterTransitionStart === "function"');
expect(code).toContain("\n __vinj_0.onRouterTransitionStart(url, type);\n");
});

it("generates imports in config order with user file last", () => {
const code = generateInstrumentationClientInjectModule(
["./inject-a.js", "some-npm-pkg"],
"/project/instrumentation-client.ts",
);
expect(code).toContain('import * as __vinj_0 from "./inject-a.js"');
expect(code).toContain('import * as __vinj_1 from "some-npm-pkg"');
expect(code).toContain('import * as __vinj_2 from "/project/instrumentation-client.ts"');
});

it("falls back to empty-module when user file is absent", () => {
const code = generateInstrumentationClientInjectModule(["./inject-a.js"], null);
expect(code).toContain('import * as __vinj_1 from "vinext/client/empty-module"');
});

it("composes hook calls for every module in array order", () => {
const code = generateInstrumentationClientInjectModule(
["./inject-a.js", "./inject-b.js"],
"/project/instrumentation-client.ts",
);
// Each module should have its own hook-check-and-call
expect(code).toContain('typeof __vinj_0.onRouterTransitionStart === "function"');
expect(code).toContain("__vinj_0.onRouterTransitionStart(url, type)");
expect(code).toContain('typeof __vinj_1.onRouterTransitionStart === "function"');
expect(code).toContain("__vinj_1.onRouterTransitionStart(url, type)");
expect(code).toContain('typeof __vinj_2.onRouterTransitionStart === "function"');
expect(code).toContain("__vinj_2.onRouterTransitionStart(url, type)");
});

it("exports empty object when injects is empty and user file is present", () => {
const code = generateInstrumentationClientInjectModule(
[],
"/project/instrumentation-client.ts",
);
expect(code).toBe("export {};");
});

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

1 change: 1 addition & 0 deletions tests/next-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,7 @@ describe("detectNextIntlConfig", () => {
buildId: "test-build-id",
deploymentId: undefined,
sassOptions: null,
instrumentationClientInject: [],
...overrides,
};
}
Expand Down
Loading