From 6db7409d287763b625a15ba31418416c4dc25035 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 25 Jun 2026 14:30:23 -0400 Subject: [PATCH 1/8] Remove small runtime CLI dependencies (#15239) --- integration/cli-test.ts | 54 +++++++- .../.changes/patch.use-node-parse-args.md | 2 +- .../__tests__/create-react-router-test.ts | 64 +++++---- packages/create-react-router/index.ts | 39 ++++-- packages/create-react-router/package.json | 2 - packages/react-router-dev/bin.cjs | 15 +- packages/react-router-dev/cli/run.ts | 130 +++++++++++------- packages/react-router-dev/package.json | 1 - .../.changes/patch.remove-get-port.md | 1 + packages/react-router-serve/cli.ts | 51 ++++++- packages/react-router-serve/package.json | 1 - pnpm-lock.yaml | 17 --- 12 files changed, 261 insertions(+), 116 deletions(-) create mode 100644 packages/react-router-serve/.changes/patch.remove-get-port.md diff --git a/integration/cli-test.ts b/integration/cli-test.ts index 70e73554a4..471e1b4184 100644 --- a/integration/cli-test.ts +++ b/integration/cli-test.ts @@ -1,6 +1,15 @@ import { spawnSync } from "node:child_process"; -import { existsSync, rmSync } from "node:fs"; +import { + copyFileSync, + existsSync, + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; import * as path from "node:path"; +import { fileURLToPath } from "node:url"; import { expect, test } from "@playwright/test"; import dedent from "dedent"; @@ -8,12 +17,44 @@ import semver from "semver"; import { createProject } from "./helpers/vite"; +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const rootDirectory = path.resolve(__dirname, ".."); const nodeBin = process.argv[0]; const reactRouterBin = "node_modules/@react-router/dev/dist/cli/index.js"; +const reactRouterPackageBin = path.join( + rootDirectory, + "packages/react-router-dev/bin.cjs", +); const run = (command: string[], options: Parameters[2]) => spawnSync(nodeBin, [reactRouterBin, ...command], options); +const getBinNodeEnv = (command: string[]) => { + let cwd = mkdtempSync(path.join(tmpdir(), "react-router-bin-")); + let env = { ...process.env }; + delete env.NODE_ENV; + + try { + mkdirSync(path.join(cwd, "dist/cli"), { recursive: true }); + copyFileSync(reactRouterPackageBin, path.join(cwd, "bin.cjs")); + writeFileSync( + path.join(cwd, "dist/cli/index.js"), + "console.log(process.env.NODE_ENV);", + ); + + let { stdout, stderr, status } = spawnSync( + nodeBin, + ["bin.cjs", ...command], + { cwd, env }, + ); + expect(stderr.toString()).toBe(""); + expect(status).toBe(0); + return stdout.toString().trim(); + } finally { + rmSync(cwd, { recursive: true, force: true }); + } +}; + const helpText = dedent` react-router @@ -109,6 +150,17 @@ test.describe("cli", () => { expect(status).toBe(0); }); + test("bin sets NODE_ENV based on the positional command", async () => { + expect(getBinNodeEnv(["dev", "--host", "127.0.0.1"])).toBe("development"); + expect(getBinNodeEnv(["--host", "127.0.0.1", "dev"])).toBe("development"); + expect(getBinNodeEnv(["build", "--mode", "development"])).toBe( + "production", + ); + expect(getBinNodeEnv(["--mode", "development", "build"])).toBe( + "production", + ); + }); + test("routes", async () => { const cwd = await createProject(); let { stdout, stderr, status } = run(["routes"], { cwd }); diff --git a/packages/create-react-router/.changes/patch.use-node-parse-args.md b/packages/create-react-router/.changes/patch.use-node-parse-args.md index bcaa6a4563..86980802f4 100644 --- a/packages/create-react-router/.changes/patch.use-node-parse-args.md +++ b/packages/create-react-router/.changes/patch.use-node-parse-args.md @@ -1 +1 @@ -Use Node's built-in `parseArgs` utility for CLI argument parsing and remove the `arg` dependency. +Use Node's built-in utilities for CLI argument parsing, ANSI-stripping, and child process execution to remove the `arg`, `strip-ansi`, and `execa` dependencies. diff --git a/packages/create-react-router/__tests__/create-react-router-test.ts b/packages/create-react-router/__tests__/create-react-router-test.ts index cedf9ecbab..525d16b786 100644 --- a/packages/create-react-router/__tests__/create-react-router-test.ts +++ b/packages/create-react-router/__tests__/create-react-router-test.ts @@ -1,5 +1,6 @@ import type { ChildProcessWithoutNullStreams } from "node:child_process"; import { execFileSync, spawn } from "node:child_process"; +import { EventEmitter } from "node:events"; import { existsSync, mkdirSync, @@ -13,8 +14,8 @@ import { createRequire } from "node:module"; import { tmpdir } from "node:os"; import path from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; +import { stripVTControlCharacters as stripAnsi } from "node:util"; import semver from "semver"; -import stripAnsi from "strip-ansi"; import { jestTimeout } from "./setupAfterEnv"; import { server } from "./msw"; @@ -22,14 +23,16 @@ import { server } from "./msw"; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); const nodeRequire = createRequire(import.meta.url); -const execaModuleId = nodeRequire.resolve("execa"); -const mockedExeca = jest.fn(); +const actualChildProcess = nodeRequire( + "node:child_process", +) as typeof import("node:child_process"); +const mockedSpawn = jest.fn(actualChildProcess.spawn); const REPO_ROOT = path.resolve(__dirname, "../../.."); const BUILT_CLI = path.resolve(__dirname, "../dist/cli.js"); -(jest as any).unstable_mockModule(execaModuleId, () => ({ - default: mockedExeca, - execa: mockedExeca, +(jest as any).unstable_mockModule("node:child_process", () => ({ + ...actualChildProcess, + spawn: mockedSpawn, })); let createReactRouter: typeof import("../index").createReactRouter; @@ -67,6 +70,7 @@ describe("create-react-router CLI", () => { beforeEach(() => { jest.clearAllMocks(); + mockedSpawn.mockImplementation(actualChildProcess.spawn); }); afterEach(async () => { @@ -76,6 +80,14 @@ describe("create-react-router CLI", () => { tempDirs = new Set(); }); + function mockSpawnSuccess() { + mockedSpawn.mockImplementation(() => { + let child = new EventEmitter(); + process.nextTick(() => child.emit("exit", 0, null)); + return child as ReturnType; + }); + } + function getProjectDir(name: string) { let tmpDir = path.join(TEMP_DIR, name); tempDirs.add(tmpDir); @@ -580,8 +592,7 @@ describe("create-react-router CLI", () => { let projectDir = getProjectDir("npm-install-default"); - let execa = mockedExeca; - execa.mockImplementation(async () => {}); + mockSpawnSuccess(); // Suppress terminal output let stdoutMock = jest @@ -599,7 +610,7 @@ describe("create-react-router CLI", () => { stdoutMock.mockReset(); - expect(execa).toHaveBeenCalledWith( + expect(mockedSpawn).toHaveBeenCalledWith( "npm", expect.arrayContaining(["install"]), expect.anything(), @@ -615,8 +626,7 @@ describe("create-react-router CLI", () => { let projectDir = getProjectDir("npm-install-on-unknown-package-manager"); - let execa = mockedExeca; - execa.mockImplementation(async () => {}); + mockSpawnSuccess(); // Suppress terminal output let stdoutMock = jest @@ -634,7 +644,7 @@ describe("create-react-router CLI", () => { stdoutMock.mockReset(); - expect(execa).toHaveBeenCalledWith( + expect(mockedSpawn).toHaveBeenCalledWith( "npm", expect.arrayContaining(["install"]), expect.anything(), @@ -650,8 +660,7 @@ describe("create-react-router CLI", () => { let projectDir = getProjectDir("npm-install-from-user-agent"); - let execa = mockedExeca; - execa.mockImplementation(async () => {}); + mockSpawnSuccess(); // Suppress terminal output let stdoutMock = jest @@ -669,7 +678,7 @@ describe("create-react-router CLI", () => { stdoutMock.mockReset(); - expect(execa).toHaveBeenCalledWith( + expect(mockedSpawn).toHaveBeenCalledWith( "npm", expect.arrayContaining(["install"]), expect.anything(), @@ -684,8 +693,7 @@ describe("create-react-router CLI", () => { let projectDir = getProjectDir("yarn-create-from-user-agent"); - let execa = mockedExeca; - execa.mockImplementation(async () => {}); + mockSpawnSuccess(); // Suppress terminal output let stdoutMock = jest @@ -703,7 +711,7 @@ describe("create-react-router CLI", () => { stdoutMock.mockReset(); - expect(execa).toHaveBeenCalledWith( + expect(mockedSpawn).toHaveBeenCalledWith( "yarn", expect.arrayContaining(["install"]), expect.anything(), @@ -718,8 +726,7 @@ describe("create-react-router CLI", () => { let projectDir = getProjectDir("pnpm-create-from-user-agent"); - let execa = mockedExeca; - execa.mockImplementation(async () => {}); + mockSpawnSuccess(); // Suppress terminal output let stdoutMock = jest @@ -737,7 +744,7 @@ describe("create-react-router CLI", () => { stdoutMock.mockReset(); - expect(execa).toHaveBeenCalledWith( + expect(mockedSpawn).toHaveBeenCalledWith( "pnpm", expect.arrayContaining(["install"]), expect.anything(), @@ -752,8 +759,7 @@ describe("create-react-router CLI", () => { let projectDir = getProjectDir("bun-create-from-user-agent"); - let execa = mockedExeca; - execa.mockImplementation(async () => {}); + mockSpawnSuccess(); // Suppress terminal output let stdoutMock = jest @@ -771,7 +777,7 @@ describe("create-react-router CLI", () => { stdoutMock.mockReset(); - expect(execa).toHaveBeenCalledWith( + expect(mockedSpawn).toHaveBeenCalledWith( "bun", expect.arrayContaining(["install"]), expect.anything(), @@ -786,8 +792,7 @@ describe("create-react-router CLI", () => { let projectDir = getProjectDir("deno-create-from-user-agent"); - let execa = mockedExeca; - execa.mockImplementation(async () => {}); + mockSpawnSuccess(); // Suppress terminal output let stdoutMock = jest @@ -805,7 +810,7 @@ describe("create-react-router CLI", () => { stdoutMock.mockReset(); - expect(execa).toHaveBeenCalledWith( + expect(mockedSpawn).toHaveBeenCalledWith( "deno", expect.arrayContaining(["install"]), expect.anything(), @@ -820,8 +825,7 @@ describe("create-react-router CLI", () => { let projectDir = getProjectDir("pnpm-create-override"); - let execa = mockedExeca; - execa.mockImplementation(async () => {}); + mockSpawnSuccess(); // Suppress terminal output let stdoutMock = jest @@ -841,7 +845,7 @@ describe("create-react-router CLI", () => { stdoutMock.mockReset(); - expect(execa).toHaveBeenCalledWith( + expect(mockedSpawn).toHaveBeenCalledWith( "pnpm", expect.arrayContaining(["install"]), expect.anything(), diff --git a/packages/create-react-router/index.ts b/packages/create-react-router/index.ts index 573ed66990..5e334caef7 100644 --- a/packages/create-react-router/index.ts +++ b/packages/create-react-router/index.ts @@ -1,12 +1,11 @@ import process from "node:process"; +import { spawn, type StdioOptions } from "node:child_process"; import { existsSync } from "node:fs"; import { cp, readFile, realpath, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; -import { parseArgs } from "node:util"; -import stripAnsi from "strip-ansi"; -import { execa } from "execa"; +import { parseArgs, stripVTControlCharacters } from "node:util"; import * as semver from "semver"; import sortPackageJSON from "sort-package-json"; @@ -534,9 +533,9 @@ async function gitInitStep(ctx: Context) { let options = { cwd: ctx.cwd, stdio: "ignore" } as const; let commitMsg = "Initial commit from create-react-router"; try { - await execa("git", ["init"], options); - await execa("git", ["add", "."], options); - await execa("git", ["commit", "-m", commitMsg], options); + await runCommand("git", ["init"], options); + await runCommand("git", ["add", "."], options); + await runCommand("git", ["commit", "-m", commitMsg], options); } catch (err) { error("Oh no!", "Failed to initialize git."); throw err; @@ -560,7 +559,7 @@ async function doneStep(ctx: Context) { `\n${prefix}Enter your project directory using`, color.cyan(`cd .${path.sep}${projectDir}`), ]; - let len = enter[0].length + stripAnsi(enter[1]).length; + let len = enter[0].length + stripVTControlCharacters(enter[1]).length; log(enter.join(len > max ? "\n" + prefix : " ")); } log( @@ -592,7 +591,7 @@ async function installDependencies({ showInstallOutput: boolean; }) { try { - await execa(pkgManager, ["install"], { + await runCommand(pkgManager, ["install"], { cwd, stdio: showInstallOutput ? "inherit" : "ignore", }); @@ -602,6 +601,30 @@ async function installDependencies({ } } +function runCommand( + command: string, + args: string[], + options: { cwd: string; stdio: StdioOptions }, +) { + return new Promise((resolve, reject) => { + let child = spawn(command, args, options); + child.on("error", reject); + child.on("exit", (code, signal) => { + if (code === 0) { + resolve(); + } else { + reject( + new Error( + signal + ? `${command} exited with signal ${signal}` + : `${command} exited with code ${code}`, + ), + ); + } + }); + }); +} + async function updatePackageJSON(ctx: Context) { let packageJSONPath = path.join(ctx.cwd, "package.json"); if (!existsSync(packageJSONPath)) { diff --git a/packages/create-react-router/package.json b/packages/create-react-router/package.json index bec8dd197f..5b5b10e09f 100644 --- a/packages/create-react-router/package.json +++ b/packages/create-react-router/package.json @@ -39,14 +39,12 @@ } }, "dependencies": { - "execa": "9.6.1", "gunzip-maybe": "^1.4.2", "log-update": "^8.0.0", "picocolors": "^1.1.1", "semver": "^7.8.1", "sisteransi": "^1.0.5", "sort-package-json": "^3.6.1", - "strip-ansi": "^7.2.0", "tar-fs": "^3.1.2" }, "devDependencies": { diff --git a/packages/react-router-dev/bin.cjs b/packages/react-router-dev/bin.cjs index 5a0012cc7b..2dfc24857b 100755 --- a/packages/react-router-dev/bin.cjs +++ b/packages/react-router-dev/bin.cjs @@ -1,13 +1,20 @@ #!/usr/bin/env node -void (async () => { - let { default: arg } = await import("arg"); +let { parseArgs } = require("node:util"); + +const commands = new Set(["build", "dev", "reveal", "routes", "typegen"]); +void (async () => { // Minimal replication of our actual parsing in `run.ts`. If not already set, // default `NODE_ENV` so React loads the proper version in its CJS entry script. // We have to do this before importing `run.ts` since that is what imports // `react` (indirectly via `react-router`) - let args = arg({}, { argv: process.argv.slice(2), permissive: true }); - if (args._.length === 0 || args._[0] === "dev") { + let { positionals } = parseArgs({ + args: process.argv.slice(2), + allowPositionals: true, + strict: false, + }); + let command = positionals.find((positional) => commands.has(positional)); + if (!command || command === "dev") { process.env.NODE_ENV = process.env.NODE_ENV ?? "development"; } else { process.env.NODE_ENV = process.env.NODE_ENV ?? "production"; diff --git a/packages/react-router-dev/cli/run.ts b/packages/react-router-dev/cli/run.ts index fe96f51a46..4d527d2f6a 100644 --- a/packages/react-router-dev/cli/run.ts +++ b/packages/react-router-dev/cli/run.ts @@ -1,4 +1,4 @@ -import arg from "arg"; +import { parseArgs } from "node:util"; import semver from "semver"; import colors from "picocolors"; @@ -78,6 +78,26 @@ ${colors.blueBright("react-router")} $ react-router typegen --watch `; +type ParsedValue = string | boolean | Array | undefined; + +function getBooleanArg(value: ParsedValue) { + return typeof value === "boolean" ? value : undefined; +} + +function getBooleanStringArg(value: ParsedValue) { + return typeof value === "boolean" || typeof value === "string" + ? value + : undefined; +} + +function getNumberArg(value: ParsedValue) { + return typeof value === "string" ? Number(value) : undefined; +} + +function getStringArg(value: ParsedValue) { + return typeof value === "string" ? value : undefined; +} + /** * Programmatic interface for running the react-router CLI with the given command line * arguments. @@ -106,54 +126,66 @@ export async function run( return !nextArg || nextArg.startsWith("-"); }; - let args = arg( - { - "--force": Boolean, - "--help": Boolean, - "-h": "--help", - "--json": Boolean, - "--token": String, - "--typescript": Boolean, - "--no-typescript": Boolean, - "--version": Boolean, - "-v": "--version", - "--port": Number, - "-p": "--port", - "--config": String, - "-c": "--config", - "--assetsInlineLimit": Number, - "--clearScreen": Boolean, - "--cors": Boolean, - "--emptyOutDir": Boolean, - "--host": isBooleanFlag("--host") ? Boolean : String, - "--logLevel": String, - "-l": "--logLevel", - "--minify": String, - "--mode": String, - "-m": "--mode", - "--open": isBooleanFlag("--open") ? Boolean : String, - "--strictPort": Boolean, - "--profile": Boolean, - "--sourcemapClient": isBooleanFlag("--sourcemapClient") - ? Boolean - : String, - "--sourcemapServer": isBooleanFlag("--sourcemapServer") - ? Boolean - : String, - "--watch": Boolean, - }, - { - argv, + let { values, positionals } = parseArgs({ + args: argv, + allowPositionals: true, + options: { + assetsInlineLimit: { type: "string" }, + clearScreen: { type: "boolean" }, + config: { type: "string", short: "c" }, + cors: { type: "boolean" }, + emptyOutDir: { type: "boolean" }, + force: { type: "boolean" }, + help: { type: "boolean", short: "h" }, + host: { type: isBooleanFlag("--host") ? "boolean" : "string" }, + json: { type: "boolean" }, + logLevel: { type: "string", short: "l" }, + minify: { type: "string" }, + mode: { type: "string", short: "m" }, + "no-typescript": { type: "boolean" }, + open: { type: isBooleanFlag("--open") ? "boolean" : "string" }, + port: { type: "string", short: "p" }, + profile: { type: "boolean" }, + sourcemapClient: { + type: isBooleanFlag("--sourcemapClient") ? "boolean" : "string", + }, + sourcemapServer: { + type: isBooleanFlag("--sourcemapServer") ? "boolean" : "string", + }, + strictPort: { type: "boolean" }, + token: { type: "string" }, + typescript: { type: "boolean" }, + version: { type: "boolean", short: "v" }, + watch: { type: "boolean" }, }, - ); - - let input = args._; - - let flags: any = Object.entries(args).reduce((acc, [key, value]) => { - key = key.replace(/^--/, ""); - acc[key] = value; - return acc; - }, {} as any); + }); + + let input = positionals; + + let flags: any = { + assetsInlineLimit: getNumberArg(values.assetsInlineLimit), + clearScreen: getBooleanArg(values.clearScreen), + config: getStringArg(values.config), + cors: getBooleanArg(values.cors), + emptyOutDir: getBooleanArg(values.emptyOutDir), + force: getBooleanArg(values.force), + help: getBooleanArg(values.help), + host: getBooleanStringArg(values.host), + json: getBooleanArg(values.json), + logLevel: getStringArg(values.logLevel), + minify: getStringArg(values.minify), + mode: getStringArg(values.mode), + open: getBooleanStringArg(values.open), + port: getNumberArg(values.port), + profile: getBooleanArg(values.profile), + sourcemapClient: getBooleanStringArg(values.sourcemapClient), + sourcemapServer: getBooleanStringArg(values.sourcemapServer), + strictPort: getBooleanArg(values.strictPort), + token: getStringArg(values.token), + typescript: getBooleanArg(values.typescript), + version: getBooleanArg(values.version), + watch: getBooleanArg(values.watch), + }; if (flags.help) { console.log(helpText); @@ -165,7 +197,7 @@ export async function run( } flags.interactive = flags.interactive ?? isMain; - if (args["--no-typescript"]) { + if (values["no-typescript"]) { flags.typescript = false; } diff --git a/packages/react-router-dev/package.json b/packages/react-router-dev/package.json index 5fe7f8861b..5f245fa000 100644 --- a/packages/react-router-dev/package.json +++ b/packages/react-router-dev/package.json @@ -75,7 +75,6 @@ "@babel/types": "^7.29.7", "@react-router/node": "workspace:*", "@remix-run/node-fetch-server": "^0.13.3", - "arg": "^5.0.1", "babel-dead-code-elimination": "^1.0.12", "chokidar": "^5.0.0", "dedent": "^1.7.2", diff --git a/packages/react-router-serve/.changes/patch.remove-get-port.md b/packages/react-router-serve/.changes/patch.remove-get-port.md new file mode 100644 index 0000000000..0824fa9572 --- /dev/null +++ b/packages/react-router-serve/.changes/patch.remove-get-port.md @@ -0,0 +1 @@ +Use Node's built-in networking APIs to find an available port and remove the `get-port` dependency. diff --git a/packages/react-router-serve/cli.ts b/packages/react-router-serve/cli.ts index a361c81051..b2bc504889 100644 --- a/packages/react-router-serve/cli.ts +++ b/packages/react-router-serve/cli.ts @@ -1,5 +1,6 @@ #!/usr/bin/env node import fs from "node:fs"; +import net from "node:net"; import os from "node:os"; import path from "node:path"; import url from "node:url"; @@ -11,7 +12,6 @@ import express from "express"; import type { RequestHandler as ExpressRequestHandler } from "express"; import morgan from "morgan"; import sourceMapSupport from "source-map-support"; -import getPort from "get-port"; process.env.NODE_ENV = process.env.NODE_ENV ?? "production"; @@ -69,6 +69,51 @@ function parseNumber(raw?: string) { return maybe; } +async function getAvailablePort( + preferredPort: number, + host?: string, +): Promise { + let preferredAvailablePort = await checkPort(preferredPort, host); + let availablePort = preferredAvailablePort ?? (await checkPort(0, host)); + + if (availablePort === undefined) { + throw new Error("No available port found"); + } + + return availablePort; +} + +function checkPort(port: number, host?: string): Promise { + return new Promise((resolve, reject) => { + let server = net.createServer(); + let listenOptions = host ? { port, host } : { port }; + + server.unref(); + + server.once("error", (error: NodeJS.ErrnoException) => { + if (error.code === "EADDRINUSE" || error.code === "EACCES") { + resolve(undefined); + } else { + reject(error); + } + }); + + server.listen(listenOptions, () => { + let address = server.address(); + let availablePort = + typeof address === "object" && address ? address.port : port; + + server.close((error) => { + if (error) { + reject(error); + } else { + resolve(availablePort); + } + }); + }); + }); +} + function getExpressPath(publicPath: string) { // Vite allows `base` to be an absolute URL, but Express route paths must be // pathnames. Strip any origin before mounting static asset middleware. @@ -84,7 +129,9 @@ function getExpressPath(publicPath: string) { } async function run() { - let port = parseNumber(process.env.PORT) ?? (await getPort({ port: 3000 })); + let port = + parseNumber(process.env.PORT) ?? + (await getAvailablePort(3000, process.env.HOST)); let buildPathArg = process.argv[2]; diff --git a/packages/react-router-serve/package.json b/packages/react-router-serve/package.json index d34923eefd..c572c3a277 100644 --- a/packages/react-router-serve/package.json +++ b/packages/react-router-serve/package.json @@ -43,7 +43,6 @@ "@remix-run/node-fetch-server": "^0.13.3", "compression": "^1.8.1", "express": "^5.2.1", - "get-port": "7.2.0", "morgan": "^1.10.1", "source-map-support": "^0.5.21" }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d253fc3995..c8122d4263 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -595,9 +595,6 @@ importers: packages/create-react-router: dependencies: - execa: - specifier: 9.6.1 - version: 9.6.1 gunzip-maybe: specifier: ^1.4.2 version: 1.4.2 @@ -616,9 +613,6 @@ importers: sort-package-json: specifier: ^3.6.1 version: 3.6.1 - strip-ansi: - specifier: ^7.2.0 - version: 7.2.0 tar-fs: specifier: ^3.1.2 version: 3.1.2 @@ -790,9 +784,6 @@ importers: '@remix-run/node-fetch-server': specifier: ^0.13.3 version: 0.13.3 - arg: - specifier: ^5.0.1 - version: 5.0.2 babel-dead-code-elimination: specifier: ^1.0.12 version: 1.0.12 @@ -1028,9 +1019,6 @@ importers: express: specifier: ^5.2.1 version: 5.2.1 - get-port: - specifier: 7.2.0 - version: 7.2.0 morgan: specifier: ^1.10.1 version: 1.10.1 @@ -5017,9 +5005,6 @@ packages: resolution: {integrity: sha512-ixiS0nLNNG5jNQzgZJNoUpBKdo9yTYZMGJ+QgT2jmjR7G7+QHRCc4v6LQ3NgE7EBJq+o0ams3waJwkrlBom8Ig==} engines: {node: '>=14'} - arg@5.0.2: - resolution: {integrity: sha512-PYjyFOLKQ9y57JvQ6QLo8dAgNqswh8M1RMJYdQduT6xbWSgK36P/Z/v+p888pM69jMMfS8Xd8F6I1kQ/I9HUGg==} - argparse@1.0.10: resolution: {integrity: sha512-o5Roy6tNG4SL/FOkCAN6RzjiakZS25RLYFrcMttJqbdd8BWrnA+fGz57iN5Pb06pvBGvl5gQ0B48dJlslXvoTg==} @@ -12542,8 +12527,6 @@ snapshots: are-docs-informative@0.0.2: {} - arg@5.0.2: {} - argparse@1.0.10: dependencies: sprintf-js: 1.0.3 From 8d0a3595ffcae9ddf5d75a7dde07d440b66f2215 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 25 Jun 2026 14:44:53 -0400 Subject: [PATCH 2/8] Improve change file PR comment (#15241) * Improve change file PR comment * Remove change file comment truncation * Inline rows * Remove sample change files * Inline change file regex * Update * Update --- scripts/pr.ts | 44 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/scripts/pr.ts b/scripts/pr.ts index 0f7aa7d683..33819f424b 100644 --- a/scripts/pr.ts +++ b/scripts/pr.ts @@ -55,10 +55,16 @@ type Check = (ctx: CheckContext) => Promise; const CHANGE_FILE_MARKER = ""; +type ChangeFileSummary = { + type: string; + firstLine: string; +}; + const CHANGE_FILE_FOUND_COMMENT = `${CHANGE_FILE_MARKER} ### ✅ Change File Found -A [change file](https://github.com/remix-run/react-router/blob/main/docs/community/contributing.md#change-files) file exists in this PR. Thanks!`; +One or more [change files](https://github.com/remix-run/react-router/blob/main/docs/community/contributing.md#change-files) found. +`; const CHANGE_FILE_MISSING_COMMENT = `${CHANGE_FILE_MARKER} ### ⚠️ No Change File Found @@ -138,14 +144,42 @@ async function changeFileCheck(ctx: CheckContext): Promise { } let files = await getPrFiles(ctx.prNumber); - let regex = /^packages\/[^/]+\/\.changes\/[^/]+\.md$/; - let found = files.some((f) => regex.test(f.filename)); - console.log(`changeFileCheck: found=${found}`); + let regex = + /^packages\/[^/]+\/\.changes\/(major|minor|patch|unstable)\.[^/]+\.md$/; + let summaries: ChangeFileSummary[] = files + .filter((f) => regex.test(f.filename)) + .map((f) => { + let type = f.filename.match(regex)?.[1] ?? "unknown"; + let firstLine = + fs.readFileSync(f.filename, "utf8").split(/\r?\n/, 1)[0].trim() ?? + "_No first line found._"; + + return { + type, + firstLine, + }; + }); + + console.log(`changeFileCheck: found ${summaries.length} change files`); + + let body = CHANGE_FILE_MISSING_COMMENT; + + if (summaries.length > 0) { + body = [ + CHANGE_FILE_FOUND_COMMENT, + "| Type | Change |", + "| --- | --- |", + ...summaries + .map((s) => `| \`${s.type}\` | ${s.firstLine.replaceAll("|", "\\|")} |`) + .join("\n"), + ].join("\n"); + } + return [ { type: "upsert-sticky-comment", marker: CHANGE_FILE_MARKER, - body: found ? CHANGE_FILE_FOUND_COMMENT : CHANGE_FILE_MISSING_COMMENT, + body, }, ]; } From c45edad5ee23e091eb2546a40a81f1b3cca71839 Mon Sep 17 00:00:00 2001 From: Vincent Gao Date: Thu, 25 Jun 2026 21:05:45 +0200 Subject: [PATCH 3/8] Fix Bun default import handling for Babel helpers (#15214) --- contributors.yml | 1 + .../patch.bun-babel-default-imports.md | 1 + packages/react-router-dev/vite/babel-test.ts | 15 +++++++++++++++ packages/react-router-dev/vite/babel.ts | 19 +++++++++++++------ 4 files changed, 30 insertions(+), 6 deletions(-) create mode 100644 packages/react-router-dev/.changes/patch.bun-babel-default-imports.md create mode 100644 packages/react-router-dev/vite/babel-test.ts diff --git a/contributors.yml b/contributors.yml index bdfeee9556..95307e8509 100644 --- a/contributors.yml +++ b/contributors.yml @@ -147,6 +147,7 @@ - fucancode - fyzhu - fz6m +- gaoflow - gaspard - gatzjames - gavriguy diff --git a/packages/react-router-dev/.changes/patch.bun-babel-default-imports.md b/packages/react-router-dev/.changes/patch.bun-babel-default-imports.md new file mode 100644 index 0000000000..46f2ee79f3 --- /dev/null +++ b/packages/react-router-dev/.changes/patch.bun-babel-default-imports.md @@ -0,0 +1 @@ +Fixed `react-router typegen` crashes under the Bun runtime when Babel default imports are already unwrapped. diff --git a/packages/react-router-dev/vite/babel-test.ts b/packages/react-router-dev/vite/babel-test.ts new file mode 100644 index 0000000000..575704a0bb --- /dev/null +++ b/packages/react-router-dev/vite/babel-test.ts @@ -0,0 +1,15 @@ +import { unwrapDefault } from "./babel"; + +describe("unwrapDefault", () => { + test("returns the default export from wrapped CommonJS imports", () => { + function wrappedExport() {} + + expect(unwrapDefault({ default: wrappedExport })).toBe(wrappedExport); + }); + + test("returns the import value when the runtime has already unwrapped it", () => { + function unwrappedExport() {} + + expect(unwrapDefault(unwrappedExport)).toBe(unwrappedExport); + }); +}); diff --git a/packages/react-router-dev/vite/babel.ts b/packages/react-router-dev/vite/babel.ts index ee4cc25c43..502a95cca2 100644 --- a/packages/react-router-dev/vite/babel.ts +++ b/packages/react-router-dev/vite/babel.ts @@ -5,12 +5,19 @@ import * as t from "@babel/types"; import _traverse from "@babel/traverse"; import _generate from "@babel/generator"; -// These `require`s were needed to support building within vite-ecosystem-ci, -// otherwise we get errors that `traverse` and `generate` are not functions. -const traverse = (_traverse as any) - .default as typeof import("@babel/traverse").default; -const generate = (_generate as any) - .default as typeof import("@babel/generator").default; +type DefaultImport = T | { default: T }; + +export function unwrapDefault(value: DefaultImport): T { + return (value as { default?: T }).default ?? (value as T); +} + +// Babel's CommonJS packages are exposed differently across runtimes. +const traverse = unwrapDefault( + _traverse as DefaultImport, +); +const generate = unwrapDefault( + _generate as DefaultImport, +); export { traverse, generate, parse, t }; export type { Babel, NodePath, ParseResult }; From c35eee9834dd10850068c6a8d98d2cdd0942860c Mon Sep 17 00:00:00 2001 From: Remco Haszing Date: Thu, 25 Jun 2026 21:25:55 +0200 Subject: [PATCH 4/8] Add Vite plugin registry metadata (#14946) --- contributors.yml | 1 + packages/react-router-dev/package.json | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/contributors.yml b/contributors.yml index 95307e8509..b53d5b0b3c 100644 --- a/contributors.yml +++ b/contributors.yml @@ -370,6 +370,7 @@ - raphaelbronsveld - redabacha - refusado +- remcohaszing - remorses - renyu-io - restareaByWeezy diff --git a/packages/react-router-dev/package.json b/packages/react-router-dev/package.json index 5f245fa000..85e251e1e9 100644 --- a/packages/react-router-dev/package.json +++ b/packages/react-router-dev/package.json @@ -3,6 +3,13 @@ "type": "module", "version": "8.0.1", "description": "Dev tools and CLI for React Router", + "keywords": [ + "react", + "router", + "react-router", + "vite", + "vite-plugin" + ], "homepage": "https://reactrouter.com", "bugs": { "url": "https://github.com/remix-run/react-router/issues" @@ -41,6 +48,17 @@ "bin": { "react-router": "bin.cjs" }, + "compatiblePackages": { + "schemaVersion": 1, + "rolldown": { + "type": "incompatible", + "reason": "Uses Vite-specific APIs" + }, + "rollup": { + "type": "incompatible", + "reason": "Uses Vite-specific APIs" + } + }, "scripts": { "build": "wireit", "typecheck": "tsc" From 7336ae58e4578ec3c2f02e775d7526a1a30160b8 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 25 Jun 2026 15:38:39 -0400 Subject: [PATCH 5/8] Add instrumentation result metadata (#15235) --- docs/how-to/instrumentation.md | 157 ++++- integration/browser-entry-test.ts | 12 +- .../.changes/minor.instrumentation-meta.md | 5 + .../__tests__/router/instrumentation-test.ts | 189 +++++- .../__tests__/server-runtime/handler-test.ts | 224 +++++++ packages/react-router/index.ts | 2 + .../lib/router/instrumentation.ts | 596 +++++++++++++----- packages/react-router/lib/router/router.ts | 74 ++- packages/react-router/lib/router/utils.ts | 39 +- .../react-router/lib/server-runtime/server.ts | 25 +- 10 files changed, 1087 insertions(+), 236 deletions(-) create mode 100644 packages/react-router/.changes/minor.instrumentation-meta.md diff --git a/docs/how-to/instrumentation.md b/docs/how-to/instrumentation.md index ed57f5bd76..ea1105682c 100644 --- a/docs/how-to/instrumentation.md +++ b/docs/how-to/instrumentation.md @@ -44,8 +44,11 @@ export const instrumentations = [ async request(handleRequest, { request }) { let url = `${request.method} ${request.url}`; console.log(`Request start: ${url}`); - await handleRequest(); - console.log(`Request end: ${url}`); + let result = await handleRequest(); + let pattern = result.meta?.pattern ?? "unknown"; + console.log( + `Request end: ${url} (${result.statusCode} ${pattern})`, + ); }, }); }, @@ -92,20 +95,24 @@ const instrumentations = [ router.instrument({ // Instrument navigations async navigate(callNavigate, { currentUrl, to }) { - let nav = `${currentUrl} → ${to}`; + let nav = `${currentUrl} -> ${to}`; console.log(`Navigation start: ${nav}`); - await callNavigate(); - console.log(`Navigation end: ${nav}`); + let result = await callNavigate(); + console.log( + `Navigation end: ${nav} (${result.meta?.pattern})`, + ); }, // Instrument fetcher calls async fetch( callFetch, { href, currentUrl, fetcherKey }, ) { - let fetch = `${fetcherKey} → ${href}`; + let fetch = `${fetcherKey} -> ${href}`; console.log(`Fetcher start: ${fetch}`); - await callFetch(); - console.log(`Fetcher end: ${fetch}`); + let result = await callFetch(); + console.log( + `Fetcher end: ${fetch} (${result.meta?.pattern})`, + ); }, }); }, @@ -160,20 +167,24 @@ const instrumentations = [ router.instrument({ // Instrument navigations async navigate(callNavigate, { currentUrl, to }) { - let nav = `${currentUrl} → ${to}`; + let nav = `${currentUrl} -> ${to}`; console.log(`Navigation start: ${nav}`); - await callNavigate(); - console.log(`Navigation end: ${nav}`); + let result = await callNavigate(); + console.log( + `Navigation end: ${nav} (${result.meta?.pattern})`, + ); }, // Instrument fetcher calls async fetch( callFetch, { href, currentUrl, fetcherKey }, ) { - let fetch = `${fetcherKey} → ${href}`; + let fetch = `${fetcherKey} -> ${href}`; console.log(`Fetcher start: ${fetch}`); - await callFetch(); - console.log(`Fetcher end: ${fetch}`); + let result = await callFetch(); + console.log( + `Fetcher end: ${fetch} (${result.meta?.pattern})`, + ); }, }); }, @@ -227,7 +238,9 @@ export const instrumentations = [ handler.instrument({ async request(handleRequest, { request, context }) { // Runs around ALL requests to your app - await handleRequest(); + let result = await handleRequest(); + let statusCode = result.statusCode; + let routePattern = result.meta?.pattern; }, }); }, @@ -248,14 +261,16 @@ export const instrumentations = [ router.instrument({ async navigate(callNavigate, { to, currentUrl }) { // Runs around navigation operations - await callNavigate(); + let result = await callNavigate(); + let routePattern = result.meta?.pattern; }, async fetch( callFetch, { href, currentUrl, fetcherKey }, ) { // Runs around fetcher operations - await callFetch(); + let result = await callFetch(); + let routePattern = result.meta?.pattern; }, }); }, @@ -327,7 +342,7 @@ This ensures that instrumentation is safe to add to production applications and To ensure that instrumentation code doesn't impact the runtime application, errors are caught internally and prevented from propagating outward. This design choice shows up in 2 aspects. -First, if a "handler" function (loader, action, request handler, navigation, etc.) throws an error, that error will not bubble out of the `callHandler` function invoked from your instrumentation. Instead, the `callHandler` function returns a discriminated union result of type `{ type: "success", error: undefined } | { type: "error", error: unknown }`. This ensures your entire instrumentation function runs without needing any try/catch/finally logic to handle application errors. +First, if a "handler" function (loader, action, request handler, navigation, etc.) throws an error, that error will not bubble out of the `callHandler` function invoked from your instrumentation. Instead, the `callHandler` function returns a discriminated union result of type `{ status: "success", error: undefined } | { status: "error", error: Error }`. This ensures your entire instrumentation function runs without needing any try/catch/finally logic to handle application errors. ```tsx export const instrumentations = [ @@ -374,6 +389,64 @@ export const instrumentations = [ ]; ``` +### Result Metadata + +Some instrumented calls return additional information that is only available after React Router starts processing the request, navigation, or fetcher call. + +- Route-level instrumentations (`loader`/`action`/`middleware`) don't include `meta` because metadata is available on the `info` parameter +- Client navigation/fetcher and Server request handler instrumentations return a meta field + - `meta` contains the same values passed to loaders and actions + - `url`: The normalized `URL` for the matched route request + - `pattern`: The matched route pattern, such as `/projects/:id` + - `params`: The matched route params + - `meta` may be `undefined` when React Router does not have route metadata for the instrumented call, such as server manifest requests or numeric POP navigations like `navigate(-1)` + - For client navigations that redirect, `meta` describes the original navigation target instead of the final redirected location. +- Server request handler instrumentations also return the `statusCode` of the response + +```tsx +// entry.server.tsx +export const instrumentations = [ + { + handler(handler) { + handler.instrument({ + async request(handleRequest) { + let result = await handleRequest(); + + let statusCode = result.statusCode; + let routeUrl = result.meta?.url; + let routePattern = result.meta?.pattern; + let routeParams = result.meta?.params; + }, + }); + }, + }, +]; + +// entry.client.tsx +const instrumentations = [ + { + router(router) { + router.instrument({ + async navigate(callNavigate) { + let result = await callNavigate(); + + let routeUrl = result.meta?.url; + let routePattern = result.meta?.pattern; + let routeParams = result.meta?.params; + }, + async fetch(callFetch) { + let result = await callFetch(); + + let routeUrl = result.meta?.url; + let routePattern = result.meta?.pattern; + let routeParams = result.meta?.params; + }, + }); + }, + }, +]; +``` + ### Composition You can compose multiple instrumentations by providing an array: @@ -429,8 +502,16 @@ export const instrumentations = [ const logging: ServerInstrumentation = { handler({ instrument }) { instrument({ - request: (fn, { request }) => - log(`request ${request.url}`, fn), + async request(fn, { request }) { + let label = `request ${request.url}`; + let start = Date.now(); + console.log(`-> ${label}`); + let result = await fn(); + let pattern = result.meta?.pattern ?? ""; + console.log( + `<- ${label} (${Date.now() - start}ms ${result.statusCode} ${pattern})`, + ); + }, }); }, route({ instrument, id }) { @@ -447,9 +528,9 @@ async function log( cb: () => Promise, ) { let start = Date.now(); - console.log(`➡️ ${label}`); + console.log(`-> ${label}`); await cb(); - console.log(`⬅️ ${label} (${Date.now() - start}ms)`); + console.log(`<- ${label} (${Date.now() - start}ms)`); } export const instrumentations = [logging]; @@ -523,10 +604,34 @@ export const instrumentations = [otel]; const windowPerf: ClientInstrumentation = { router({ instrument }) { instrument({ - navigate: (fn, { to, currentUrl }) => - measure(`navigation:${currentUrl}->${to}`, fn), - fetch: (fn, { href }) => - measure(`fetcher:${href}`, fn), + async navigate(fn, { to, currentUrl }) { + let label = `navigation:${currentUrl}->${to}`; + performance.mark(`start:${label}`); + let result = await fn(); + performance.mark(`end:${label}`); + performance.measure( + label, + `start:${label}`, + `end:${label}`, + ); + console.log( + `navigation pattern: ${result.meta?.pattern}`, + ); + }, + async fetch(fn, { href }) { + let label = `fetcher:${href}`; + performance.mark(`start:${label}`); + let result = await fn(); + performance.mark(`end:${label}`); + performance.measure( + label, + `start:${label}`, + `end:${label}`, + ); + console.log( + `fetcher pattern: ${result.meta?.pattern}`, + ); + }, }); }, route({ instrument, id }) { diff --git a/integration/browser-entry-test.ts b/integration/browser-entry-test.ts index ec1731f537..070e789e95 100644 --- a/integration/browser-entry-test.ts +++ b/integration/browser-entry-test.ts @@ -218,8 +218,12 @@ test("allows users to instrument the client side router via HydratedRouter", asy router.instrument({ async navigate(impl, info) { console.log("start navigate", JSON.stringify(Object.entries(info).sort())); - await impl(); - console.log("end navigate", JSON.stringify(Object.entries(info).sort())); + let result = await impl(); + console.log("end navigate", JSON.stringify(Object.entries(info).sort()), JSON.stringify({ + url: result.meta.url, + pattern: result.meta.pattern, + params: result.meta.params, + })); }, async fetch(impl, info) { console.log("start fetch", JSON.stringify(Object.entries(info).sort())); @@ -300,7 +304,9 @@ test("allows users to instrument the client side router via HydratedRouter", asy "start loader routes/page /page", "end loader root /page", "end loader routes/page /page", - 'end navigate [["currentUrl","/"],["to","/page"]]', + expect.stringMatching( + /^end navigate \[\["currentUrl","\/"\],\["to","\/page"\]\] \{"url":"http:\/\/localhost:\d+\/page","pattern":"page","params":\{\}\}$/, + ), ]); logs.splice(0); diff --git a/packages/react-router/.changes/minor.instrumentation-meta.md b/packages/react-router/.changes/minor.instrumentation-meta.md new file mode 100644 index 0000000000..c7ce47290e --- /dev/null +++ b/packages/react-router/.changes/minor.instrumentation-meta.md @@ -0,0 +1,5 @@ +Return route metadata from server request, client navigation, and client fetcher instrumentations + +- Adds result metadata after instrumented calls complete, including the URL, + matched route pattern, and params. +- Adds known HTTP status codes to server request handler instrumentation results. diff --git a/packages/react-router/__tests__/router/instrumentation-test.ts b/packages/react-router/__tests__/router/instrumentation-test.ts index 6838d7945a..c7040523ff 100644 --- a/packages/react-router/__tests__/router/instrumentation-test.ts +++ b/packages/react-router/__tests__/router/instrumentation-test.ts @@ -1552,6 +1552,7 @@ describe("instrumentation", () => { expect(args.request.method).toBe("GET"); expect(args.request.url).toBe("http://localhost/a"); expect(args.request.url).toBe("http://localhost/a"); + expect(args.url.href).toBe("http://localhost/a"); expect(args.request.headers.get).toBeDefined(); expect(args.request.headers.set).not.toBeDefined(); expect(args.params).toEqual({ slug: "a", extra: "extra" }); @@ -1631,8 +1632,8 @@ describe("instrumentation", () => { router.instrument({ async navigate(navigate, info) { spy("start", info); - await navigate(); - spy("end", info); + let result = await navigate(); + spy("end", info, result.meta); }, }); }, @@ -1644,8 +1645,72 @@ describe("instrumentation", () => { await router.navigate("/page"); expect(spy.mock.calls).toEqual([ ["start", { currentUrl: "/", to: "/page" }], - ["end", { currentUrl: "/", to: "/page" }], + [ + "end", + { currentUrl: "/", to: "/page" }, + { + url: expect.any(URL), + pattern: "/page", + params: {}, + }, + ], + ]); + expect(spy.mock.calls[1][2].url.href).toBe("http://localhost/page"); + expect(router.state).toMatchObject({ + navigation: { state: "idle" }, + location: { pathname: "/page" }, + loaderData: { page: "PAGE" }, + }); + }); + + it("returns the original navigation target metadata for redirected navigations", async () => { + let spy = jest.fn(); + let router = createMemoryRouter( + [ + { + index: true, + }, + { + id: "redirect", + path: "/redirect", + loader: () => redirect("/page"), + }, + { + id: "page", + path: "/page", + loader: () => "PAGE", + }, + ], + { + instrumentations: [ + { + router(router) { + router.instrument({ + async navigate(navigate, info) { + let result = await navigate(); + spy(info, result.meta); + }, + }); + }, + }, + ], + }, + ); + + await router.navigate("/redirect"); + expect(spy.mock.calls).toEqual([ + [ + { currentUrl: "/", to: "/redirect" }, + { + url: expect.any(URL), + pattern: "/redirect", + params: {}, + }, + ], ]); + expect(spy.mock.calls[0][1].url.href).toBe( + "http://localhost/redirect", + ); expect(router.state).toMatchObject({ navigation: { state: "idle" }, location: { pathname: "/page" }, @@ -1653,6 +1718,103 @@ describe("instrumentation", () => { }); }); + it("keeps navigation metadata scoped to overlapping instrumentation calls", async () => { + let spy = jest.fn(); + let firstContinue = createDeferred(); + let router = createMemoryRouter( + [ + { + index: true, + }, + { + id: "first", + path: "/first", + loader: () => "FIRST", + }, + { + id: "second", + path: "/second", + loader: () => "SECOND", + }, + ], + { + instrumentations: [ + { + router(router) { + router.instrument({ + async navigate(navigate, info) { + if (info.to === "/first") { + await firstContinue.promise; + } + let result = await navigate(); + spy(info.to, result.meta?.pattern); + }, + }); + }, + }, + ], + }, + ); + + let firstNavigation = router.navigate("/first"); + await tick(); + await router.navigate("/second"); + + firstContinue.resolve(); + await firstNavigation; + + expect(spy.mock.calls).toEqual([ + ["/second", "/second"], + ["/first", "/first"], + ]); + }); + + it("returns undefined navigation metadata for numeric POP navigations", async () => { + let spy = jest.fn(); + let router = createMemoryRouter( + [ + { + index: true, + }, + { + id: "first", + path: "/first", + loader: () => "FIRST", + }, + { + id: "second", + path: "/second", + loader: () => "SECOND", + }, + ], + { + initialEntries: ["/", "/first", "/second"], + initialIndex: 2, + instrumentations: [ + { + router(router) { + router.instrument({ + async navigate(navigate, info) { + let result = await navigate(); + spy(info.to, result.meta); + }, + }); + }, + }, + ], + }, + ); + + await router.navigate(-1); + + expect(spy.mock.calls).toEqual([[-1, undefined]]); + expect(router.state).toMatchObject({ + navigation: { state: "idle" }, + location: { pathname: "/first" }, + loaderData: { first: "FIRST" }, + }); + }); + it("allows instrumentation of fetchers", async () => { let spy = jest.fn(); let router = createMemoryRouter( @@ -1673,8 +1835,8 @@ describe("instrumentation", () => { router.instrument({ async fetch(fetch, info) { spy("start", info); - await fetch(); - spy("end", info); + let result = await fetch(); + spy("end", info, result.meta); }, }); }, @@ -1690,8 +1852,17 @@ describe("instrumentation", () => { await router.fetch("key", "0", "/page"); expect(spy.mock.calls).toEqual([ ["start", { href: "/page", currentUrl: "/", fetcherKey: "key" }], - ["end", { href: "/page", currentUrl: "/", fetcherKey: "key" }], + [ + "end", + { href: "/page", currentUrl: "/", fetcherKey: "key" }, + { + url: expect.any(URL), + pattern: "/page", + params: {}, + }, + ], ]); + expect(spy.mock.calls[1][2].url.href).toBe("http://localhost/page"); expect(router.state).toMatchObject({ navigation: { state: "idle" }, location: { pathname: "/" }, @@ -2081,6 +2252,7 @@ describe("instrumentation", () => { }, params: {}, pattern: "/", + url: expect.any(URL), context: { get: expect.any(Function), }, @@ -2100,6 +2272,7 @@ describe("instrumentation", () => { }, params: {}, pattern: "/", + url: expect.any(URL), context: { get: expect.any(Function), }, @@ -2157,6 +2330,7 @@ describe("instrumentation", () => { }, params: {}, pattern: "/", + url: expect.any(URL), context: { get: expect.any(Function) }, }, ], @@ -2173,6 +2347,7 @@ describe("instrumentation", () => { }, params: {}, pattern: "/", + url: expect.any(URL), context: { get: expect.any(Function) }, }, ], @@ -2230,6 +2405,7 @@ describe("instrumentation", () => { }, params: {}, pattern: "/", + url: expect.any(URL), context: { get: expect.any(Function) }, }, ], @@ -2246,6 +2422,7 @@ describe("instrumentation", () => { }, params: {}, pattern: "/", + url: expect.any(URL), context: { get: expect.any(Function) }, }, ], diff --git a/packages/react-router/__tests__/server-runtime/handler-test.ts b/packages/react-router/__tests__/server-runtime/handler-test.ts index 118a1542c7..13f6c6470e 100644 --- a/packages/react-router/__tests__/server-runtime/handler-test.ts +++ b/packages/react-router/__tests__/server-runtime/handler-test.ts @@ -1,7 +1,231 @@ +import type { InstrumentationResultMeta } from "../../lib/router/instrumentation"; +import { data } from "../../lib/router/utils"; import { createRequestHandler } from "../../lib/server-runtime/server"; import { mockServerBuild } from "./utils"; +type RequestInstrumentationResult = { + status: string; + error: string | undefined; + statusCode: number; + meta: InstrumentationResultMeta | undefined; +}; + describe("createRequestHandler", () => { + it("returns route metadata from request handler instrumentations", async () => { + let meta: unknown; + let statusCode: number; + let build = mockServerBuild( + { + root: { + default: true, + }, + "routes/user": { + parentId: "root", + path: "users/:id", + default: true, + loader: () => null, + }, + }, + { + instrumentations: [ + { + handler({ instrument }) { + instrument({ + async request(callHandler) { + let result = await callHandler(); + meta = result.meta; + statusCode = result.statusCode; + }, + }); + }, + }, + ], + }, + ); + let handler = createRequestHandler(build); + + await handler( + new Request("http://example.com/users/123", { + signal: new AbortController().signal, + }), + ); + + expect(meta).toEqual({ + url: expect.any(URL), + pattern: "users/:id", + params: { id: "123" }, + }); + expect(meta?.url.href).toBe("http://example.com/users/123"); + expect(statusCode).toBe(200); + }); + + it("does not return route metadata from request handler instrumentations for manifest requests", async () => { + let meta: unknown = "unset"; + let statusCode: number | undefined; + let build = mockServerBuild( + { + root: { + default: {}, + }, + "routes/a": { + path: "a", + }, + }, + { + instrumentations: [ + { + handler({ instrument }) { + instrument({ + async request(callHandler) { + let result = await callHandler(); + meta = result.meta; + statusCode = result.statusCode; + }, + }); + }, + }, + ], + }, + ); + let handler = createRequestHandler(build); + + let response = await handler( + new Request( + `http://example.com/__manifest?paths=%2Fa&version=${build.assets.version}`, + { + signal: new AbortController().signal, + }, + ), + ); + + expect(response.status).toBe(200); + expect(meta).toBeUndefined(); + expect(statusCode).toBe(200); + }); + + it("returns an error boundary response from request handler instrumentations for thrown loader errors", async () => { + let requestResult: RequestInstrumentationResult | undefined; + let build = mockServerBuild( + { + root: { + path: "/", + default: true, + ErrorBoundary: true, + loader() { + throw new Error("Kaboom!"); + }, + }, + }, + { + handleError() {}, + handleDocumentRequest(_request, status) { + return new Response("Route error boundary rendered", { status }); + }, + instrumentations: [ + { + handler({ instrument }) { + instrument({ + async request(callHandler) { + let result = await callHandler(); + requestResult = { + status: result.status, + error: result.error?.message, + statusCode: result.statusCode, + meta: result.meta, + }; + }, + }); + }, + }, + ], + }, + ); + let handler = createRequestHandler(build); + + let response = await handler( + new Request("http://example.com/", { + signal: new AbortController().signal, + }), + ); + + expect(response.status).toBe(500); + expect(await response.text()).toBe("Route error boundary rendered"); + expect(requestResult).toEqual({ + status: "success", + error: undefined, + statusCode: 500, + meta: { + url: expect.any(URL), + pattern: "/", + params: {}, + }, + }); + expect(requestResult?.meta?.url.href).toBe("http://example.com/"); + }); + + it("returns an error boundary response from request handler instrumentations for thrown data", async () => { + let requestResult: RequestInstrumentationResult | undefined; + let build = mockServerBuild( + { + root: { + path: "/", + default: true, + ErrorBoundary: true, + loader() { + throw data( + { message: "Nope!" }, + { status: 418, statusText: "I'm a teapot" }, + ); + }, + }, + }, + { + handleError() {}, + handleDocumentRequest(_request, status) { + return new Response("Route error boundary rendered", { status }); + }, + instrumentations: [ + { + handler({ instrument }) { + instrument({ + async request(callHandler) { + let result = await callHandler(); + requestResult = { + status: result.status, + error: result.error?.message, + statusCode: result.statusCode, + meta: result.meta, + }; + }, + }); + }, + }, + ], + }, + ); + let handler = createRequestHandler(build); + + let response = await handler( + new Request("http://example.com/", { + signal: new AbortController().signal, + }), + ); + + expect(response.status).toBe(418); + expect(await response.text()).toBe("Route error boundary rendered"); + expect(requestResult).toEqual({ + status: "success", + error: undefined, + statusCode: 418, + meta: { + url: expect.any(URL), + pattern: "/", + params: {}, + }, + }); + expect(requestResult?.meta?.url.href).toBe("http://example.com/"); + }); + it("retains request headers when stripping body off for loaders", async () => { let build = mockServerBuild({ root: { diff --git a/packages/react-router/index.ts b/packages/react-router/index.ts index 34a03bbc64..c74de7da75 100644 --- a/packages/react-router/index.ts +++ b/packages/react-router/index.ts @@ -77,6 +77,8 @@ export type { InstrumentRouterFunction, InstrumentRouteFunction, InstrumentationHandlerResult, + InstrumentationClientRouterResult, + InstrumentationServerHandlerResult, } from "./lib/router/instrumentation"; export { createStaticHandler, diff --git a/packages/react-router/lib/router/instrumentation.ts b/packages/react-router/lib/router/instrumentation.ts index f26e50e1cd..f3022c0e33 100644 --- a/packages/react-router/lib/router/instrumentation.ts +++ b/packages/react-router/lib/router/instrumentation.ts @@ -1,6 +1,7 @@ import type { RequestHandler } from "../server-runtime/server"; import { createPath, invariant } from "./history"; import type { Router } from "./router"; +import { createContext, RouterContextProvider } from "./utils"; import type { ActionFunctionArgs, DataRouteObject, @@ -12,7 +13,6 @@ import type { MaybePromise, MiddlewareFunction, RouterContext, - RouterContextProvider, } from "./utils"; // Public APIs @@ -34,13 +34,48 @@ export type InstrumentRouterFunction = (router: InstrumentableRouter) => void; export type InstrumentRouteFunction = (route: InstrumentableRoute) => void; +/** + * Route metadata available after React Router has matched an instrumented + * request, navigation, or fetcher call. + */ +export type InstrumentationResultMeta = { + url: LoaderFunctionArgs["url"]; + pattern: string; + params: LoaderFunctionArgs["params"]; +}; + +/** + * Result returned by route-level instrumented handler calls, such as + * instrumented loaders, actions, middleware, and lazy route functions. + */ export type InstrumentationHandlerResult = | { status: "success"; error: undefined } | { status: "error"; error: Error }; +/** + * Result returned by client-side router instrumented navigation and fetcher + * calls. + */ +export type InstrumentationClientRouterResult = InstrumentationHandlerResult & { + meta: InstrumentationResultMeta | undefined; +}; + +/** + * Result returned by server request handler instrumentation. + */ +export type InstrumentationServerHandlerResult = + InstrumentationHandlerResult & { + statusCode: number; + meta: InstrumentationResultMeta | undefined; + }; + +export type InstrumentationMetaReceiver = ( + meta: InstrumentationResultMeta | undefined, +) => void; + // Shared -type InstrumentFunction = ( - handler: () => Promise, +type InstrumentFunction = ( + handler: () => Promise, info: T, ) => Promise; @@ -79,12 +114,12 @@ type RouteInstrumentations = { type RouteLazyInstrumentationInfo = undefined; -type RouteHandlerInstrumentationInfo = Readonly<{ - request: ReadonlyRequest; - params: LoaderFunctionArgs["params"]; - pattern: string; - context: ReadonlyContext; -}>; +type RouteHandlerInstrumentationInfo = Readonly< + Omit & { + request: ReadonlyRequest; + context: ReadonlyContext; + } +>; // Router Instrumentation type InstrumentableRouter = { @@ -92,8 +127,14 @@ type InstrumentableRouter = { }; type RouterInstrumentations = { - navigate?: InstrumentFunction; - fetch?: InstrumentFunction; + navigate?: InstrumentFunction< + RouterNavigationInstrumentationInfo, + InstrumentationClientRouterResult + >; + fetch?: InstrumentFunction< + RouterFetchInstrumentationInfo, + InstrumentationClientRouterResult + >; }; type RouterNavigationInstrumentationInfo = Readonly<{ @@ -121,7 +162,10 @@ type InstrumentableRequestHandler = { }; type RequestHandlerInstrumentations = { - request?: InstrumentFunction; + request?: InstrumentFunction< + RequestHandlerInstrumentationInfo, + InstrumentationServerHandlerResult + >; }; type RequestHandlerInstrumentationInfo = Readonly<{ @@ -131,18 +175,35 @@ type RequestHandlerInstrumentationInfo = Readonly<{ const UninstrumentedSymbol = Symbol("Uninstrumented"); +type InstrumentableFunction = (...args: never[]) => MaybePromise; +type InstrumentedFunction = T & { + [UninstrumentedSymbol]?: T; +}; + +export const instrumentationResultMetaContext = + createContext(); + +// Client router instrumentations need route metadata captured inside the router +// after matching, but exposing this on public router state/options would make it +// part of the API. Keep a private, one-shot receiver keyed by router instance so +// navigate/fetch instrumentation can receive the metadata without leaking it. +let instrumentationClientResultMetaReceivers = new WeakMap< + Router, + InstrumentationMetaReceiver +>(); + export function getRouteInstrumentationUpdates( fns: InstrumentRouteFunction[], route: Readonly, ) { let aggregated: { - lazy: InstrumentFunction[]; - "lazy.loader": InstrumentFunction[]; - "lazy.action": InstrumentFunction[]; - "lazy.middleware": InstrumentFunction[]; - middleware: InstrumentFunction[]; - loader: InstrumentFunction[]; - action: InstrumentFunction[]; + lazy: NonNullable[]; + "lazy.loader": NonNullable[]; + "lazy.action": NonNullable[]; + "lazy.middleware": NonNullable[]; + middleware: NonNullable[]; + loader: NonNullable[]; + action: NonNullable[]; } = { lazy: [], "lazy.loader": [], @@ -159,11 +220,26 @@ export function getRouteInstrumentationUpdates( index: route.index, path: route.path, instrument(i) { - let keys = Object.keys(aggregated) as Array; - for (let key of keys) { - if (i[key]) { - aggregated[key].push(i[key] as any); - } + if (i.lazy != null) { + aggregated.lazy.push(i.lazy); + } + if (i["lazy.loader"] != null) { + aggregated["lazy.loader"].push(i["lazy.loader"]); + } + if (i["lazy.action"] != null) { + aggregated["lazy.action"].push(i["lazy.action"]); + } + if (i["lazy.middleware"] != null) { + aggregated["lazy.middleware"].push(i["lazy.middleware"]); + } + if (i.middleware != null) { + aggregated.middleware.push(i.middleware); + } + if (i.loader != null) { + aggregated.loader.push(i.loader); + } + if (i.action != null) { + aggregated.action.push(i.action); } }, }), @@ -178,48 +254,122 @@ export function getRouteInstrumentationUpdates( // Instrument lazy functions if (typeof route.lazy === "function" && aggregated.lazy.length > 0) { - let instrumented = wrapImpl(aggregated.lazy, route.lazy, () => undefined); - if (instrumented) { - updates.lazy = instrumented as DataRouteObject["lazy"]; - } + let lazy = route.lazy; + updates.lazy = async (...args) => { + let result = await recurseRight( + aggregated.lazy, + undefined, + () => lazy(...args), + getInstrumentationInnerResult, + ); + return throwOrReturnResult(result); + }; } // Instrument the lazy object format if (typeof route.lazy === "object") { let lazyObject: LazyRouteObject = route.lazy; - (["middleware", "loader", "action"] as const).forEach((key) => { - let lazyFn = lazyObject[key]; - let instrumentations = aggregated[`lazy.${key}`]; - if (typeof lazyFn === "function" && instrumentations.length > 0) { - let instrumented = wrapImpl(instrumentations, lazyFn, () => undefined); - if (instrumented) { - updates.lazy = Object.assign(updates.lazy || {}, { - [key]: instrumented, - }); - } - } - }); + + if ( + typeof lazyObject.middleware === "function" && + aggregated["lazy.middleware"].length > 0 + ) { + let middleware = lazyObject.middleware; + updates.lazy = Object.assign(updates.lazy || {}, { + middleware: async ( + ...args: Parameters< + NonNullable["middleware"]> + > + ) => { + let result = await recurseRight( + aggregated["lazy.middleware"], + undefined, + () => middleware(...args), + getInstrumentationInnerResult, + ); + return throwOrReturnResult(result); + }, + }); + } + + if ( + typeof lazyObject.loader === "function" && + aggregated["lazy.loader"].length > 0 + ) { + let loader = lazyObject.loader; + updates.lazy = Object.assign(updates.lazy || {}, { + loader: async ( + ...args: Parameters< + NonNullable["loader"]> + > + ) => { + let result = await recurseRight( + aggregated["lazy.loader"], + undefined, + () => loader(...args), + getInstrumentationInnerResult, + ); + return throwOrReturnResult(result); + }, + }); + } + + if ( + typeof lazyObject.action === "function" && + aggregated["lazy.action"].length > 0 + ) { + let action = lazyObject.action; + updates.lazy = Object.assign(updates.lazy || {}, { + action: async ( + ...args: Parameters< + NonNullable["action"]> + > + ) => { + let result = await recurseRight( + aggregated["lazy.action"], + undefined, + () => action(...args), + getInstrumentationInnerResult, + ); + return throwOrReturnResult(result); + }, + }); + } } // Instrument loader/action functions - (["loader", "action"] as const).forEach((key) => { - let handler = route[key]; - if (typeof handler === "function" && aggregated[key].length > 0) { - // @ts-expect-error - let original = handler[UninstrumentedSymbol] ?? handler; - let instrumented = wrapImpl(aggregated[key], original, (...args) => - getHandlerInfo(args[0] as LoaderFunctionArgs | ActionFunctionArgs), + if (typeof route.loader === "function" && aggregated.loader.length > 0) { + let original = getUninstrumentedHandler(route.loader); + let instrumented = async (...args: Parameters) => { + let result = await recurseRight( + aggregated.loader, + getHandlerInfo(args[0]), + () => original(...args), + getInstrumentationInnerResult, ); - if (instrumented) { - if (key === "loader" && original.hydrate === true) { - (instrumented as LoaderFunction).hydrate = true; - } - // @ts-expect-error - instrumented[UninstrumentedSymbol] = original; - updates[key] = instrumented; - } + return throwOrReturnResult(result); + }; + if (original.hydrate === true) { + (instrumented as LoaderFunction).hydrate = true; } - }); + setUninstrumentedHandler(instrumented, original); + updates.loader = instrumented; + } + + if (typeof route.action === "function" && aggregated.action.length > 0) { + let original = getUninstrumentedHandler(route.action); + let instrumented = async (...args: Parameters) => { + let result = await recurseRight( + aggregated.action, + getHandlerInfo(args[0]), + () => original(...args), + getInstrumentationInnerResult, + ); + return throwOrReturnResult(result); + }; + setUninstrumentedHandler(instrumented, original); + updates.action = instrumented; + } // Instrument middleware functions if ( @@ -228,17 +378,18 @@ export function getRouteInstrumentationUpdates( aggregated.middleware.length > 0 ) { updates.middleware = route.middleware.map((middleware) => { - // @ts-expect-error - let original = middleware[UninstrumentedSymbol] ?? middleware; - let instrumented = wrapImpl(aggregated.middleware, original, (...args) => - getHandlerInfo(args[0] as Parameters[0]), - ); - if (instrumented) { - // @ts-expect-error - instrumented[UninstrumentedSymbol] = original; - return instrumented; - } - return middleware; + let original = getUninstrumentedHandler(middleware); + let instrumented = async (...args: Parameters) => { + let result = await recurseRight( + aggregated.middleware, + getHandlerInfo(args[0]), + () => original(...args), + getInstrumentationInnerResult, + ); + return throwOrReturnResult(result); + }; + setUninstrumentedHandler(instrumented, original); + return instrumented; }); } @@ -250,8 +401,8 @@ export function instrumentClientSideRouter( fns: InstrumentRouterFunction[], ): Router { let aggregated: { - navigate: InstrumentFunction[]; - fetch: InstrumentFunction[]; + navigate: NonNullable[]; + fetch: NonNullable[]; } = { navigate: [], fetch: [], @@ -260,58 +411,96 @@ export function instrumentClientSideRouter( fns.forEach((fn) => fn({ instrument(i) { - let keys = Object.keys(i) as Array; - for (let key of keys) { - if (i[key]) { - aggregated[key].push(i[key] as any); - } + if (i.navigate != null) { + aggregated.navigate.push(i.navigate); + } + if (i.fetch != null) { + aggregated.fetch.push(i.fetch); } }, }), ); if (aggregated.navigate.length > 0) { - // @ts-expect-error - let navigate = router.navigate[UninstrumentedSymbol] ?? router.navigate; - let instrumentedNavigate = wrapImpl( - aggregated.navigate, - navigate, - (...args) => { - let [to, opts] = args as Parameters; - return { - to: - typeof to === "number" || typeof to === "string" - ? to - : to - ? createPath(to) - : ".", - ...getRouterInfo(router, opts ?? {}), - } satisfies RouterNavigationInstrumentationInfo; - }, - ) as Router["navigate"]; - if (instrumentedNavigate) { - // @ts-expect-error - instrumentedNavigate[UninstrumentedSymbol] = navigate; - router.navigate = instrumentedNavigate; - } + let navigate = getUninstrumentedHandler(router.navigate); + let instrumentedNavigate = async ( + ...args: Parameters + ): Promise>> => { + let [to, opts] = args; + let meta: InstrumentationResultMeta | undefined; + let info: RouterNavigationInstrumentationInfo = { + to: + typeof to === "number" || typeof to === "string" + ? to + : to + ? createPath(to) + : ".", + ...getRouterInfo(router, opts ?? {}), + }; + let result = await recurseRight( + aggregated.navigate, + info, + async () => { + if (typeof to === "number") { + return await navigate(...args); + } + let cleanup = setInstrumentationClientResultMetaReceiver( + router, + (value) => { + meta = value; + }, + ); + try { + return await navigate(...args); + } finally { + cleanup(); + } + }, + (result): InstrumentationClientRouterResult => ({ + ...getInstrumentationInnerResult(result), + meta, + }), + ); + return throwOrReturnResult(result); + }; + setUninstrumentedHandler(instrumentedNavigate, navigate); + router.navigate = instrumentedNavigate as Router["navigate"]; } if (aggregated.fetch.length > 0) { - // @ts-expect-error - let fetch = router.fetch[UninstrumentedSymbol] ?? router.fetch; - let instrumentedFetch = wrapImpl(aggregated.fetch, fetch, (...args) => { - let [key, , href, opts] = args as Parameters; - return { - href: href ?? ".", - fetcherKey: key, - ...getRouterInfo(router, opts ?? {}), - } satisfies RouterFetchInstrumentationInfo; - }) as Router["fetch"]; - if (instrumentedFetch) { - // @ts-expect-error - instrumentedFetch[UninstrumentedSymbol] = fetch; - router.fetch = instrumentedFetch; - } + let fetch = getUninstrumentedHandler(router.fetch); + let instrumentedFetch = async (...args: Parameters) => { + let [key, _, href, opts] = args; + let meta: InstrumentationResultMeta | undefined; + let result = await recurseRight( + aggregated.fetch, + { + href: href ?? ".", + fetcherKey: key, + ...getRouterInfo(router, opts ?? {}), + } satisfies RouterFetchInstrumentationInfo, + async () => { + let cleanup = setInstrumentationClientResultMetaReceiver( + router, + (value) => { + meta = value; + }, + ); + try { + return await fetch(...args); + } finally { + cleanup(); + } + }, + (result): InstrumentationClientRouterResult => ({ + ...getInstrumentationInnerResult(result), + meta, + }), + ); + return throwOrReturnResult(result); + }; + setUninstrumentedHandler(instrumentedFetch, fetch); + router.fetch = instrumentedFetch; } return router; @@ -322,7 +511,10 @@ export function instrumentHandler( fns: InstrumentRequestHandlerFunction[], ): RequestHandler { let aggregated: { - request: InstrumentFunction[]; + request: InstrumentFunction< + RequestHandlerInstrumentationInfo, + InstrumentationServerHandlerResult + >[]; } = { request: [], }; @@ -330,11 +522,8 @@ export function instrumentHandler( fns.forEach((fn) => fn({ instrument(i) { - let keys = Object.keys(i) as Array; - for (let key of keys) { - if (i[key]) { - aggregated[key].push(i[key] as any); - } + if (i.request != null) { + aggregated.request.push(i.request); } }, }), @@ -343,73 +532,131 @@ export function instrumentHandler( let instrumentedHandler = handler; if (aggregated.request.length > 0) { - instrumentedHandler = wrapImpl(aggregated.request, handler, (...args) => { - let [request, context] = args as Parameters; - return { - request: getReadonlyRequest(request), - context: context != null ? getReadonlyContext(context) : context, - } satisfies RequestHandlerInstrumentationInfo; - }) as RequestHandler; + instrumentedHandler = async (...args) => { + let [request, context] = args; + let instrumentationContext = context ?? new RouterContextProvider(); + let result = await recurseRight( + aggregated.request, + { + request: getReadonlyRequest(request), + context: getReadonlyContext(instrumentationContext), + } satisfies RequestHandlerInstrumentationInfo, + () => handler(request, instrumentationContext), + (result, info) => { + let meta: InstrumentationResultMeta | undefined; + try { + meta = info.context?.get(instrumentationResultMetaContext); + } catch { + // Not all instrumentation contexts have request/route metadata. + } + invariant( + result.value instanceof Response, + "Expected a Response from the request handler", + ); + return { + ...getInstrumentationInnerResult(result), + statusCode: result.value.status, + meta, + }; + }, + ); + return throwOrReturnResult(result); + }; } return instrumentedHandler; } -function wrapImpl( - impls: InstrumentFunction[], - handler: (...args: any[]) => MaybePromise, - getInfo: (...args: unknown[]) => T, +function getUninstrumentedHandler( + handler: T, +): T { + return (handler as InstrumentedFunction)[UninstrumentedSymbol] ?? handler; +} + +function setUninstrumentedHandler( + handler: (...args: TArgs) => MaybePromise, + uninstrumentedHandler: (...args: TArgs) => MaybePromise, ) { - if (impls.length === 0) { - return null; - } - return async (...args: unknown[]) => { - let result = await recurseRight( - impls, - getInfo(...args), - () => handler(...args), - impls.length - 1, - ); - if (result.type === "error") { - throw result.value; + (handler as InstrumentedFunction<(...args: TArgs) => MaybePromise>)[ + UninstrumentedSymbol + ] = uninstrumentedHandler; +} + +export function setInstrumentationClientResultMetaReceiver( + router: Router, + receiver: InstrumentationMetaReceiver, +): () => void { + instrumentationClientResultMetaReceivers.set(router, receiver); + return () => { + if (instrumentationClientResultMetaReceivers.get(router) === receiver) { + instrumentationClientResultMetaReceivers.delete(router); } - return result.value; }; } -type RecurseResult = { type: "success" | "error"; value: unknown }; +export function consumeInstrumentationClientResultMetaReceiver( + router: Router, +): InstrumentationMetaReceiver | undefined { + let receiver = instrumentationClientResultMetaReceivers.get(router); + instrumentationClientResultMetaReceivers.delete(router); + return receiver; +} -async function recurseRight( - impls: InstrumentFunction[], - info: T, - handler: () => MaybePromise, - index: number, -): Promise { +type RecurseResult = + | { type: "success"; value: TResult } + | { type: "error"; value: unknown }; + +function throwOrReturnResult(result: RecurseResult): TResult { + if (result.type === "error") { + throw result.value; + } + return result.value; +} + +async function recurseRight< + TResult, + TInfo extends InstrumentationInfo, + TInnerResult extends InstrumentationHandlerResult, +>( + impls: InstrumentFunction[], + info: TInfo, + handler: () => MaybePromise, + getInnerResult: (result: RecurseResult, info: TInfo) => TInnerResult, + state: RecurseState = { + result: null, + innerResult: null, + }, + index = impls.length - 1, +): Promise> { let impl = impls[index]; - let result: RecurseResult | undefined; if (!impl) { try { let value = await handler(); - result = { type: "success", value }; + state.result = { type: "success", value }; } catch (e) { - result = { type: "error", value: e }; + state.result = { type: "error", value: e }; } + state.innerResult = getInnerResult(state.result, info); } else { // If they forget to call the handler, or if they throw before calling the // handler, we need to ensure the handlers still gets called - let handlerPromise: ReturnType | undefined = undefined; - let callHandler = async (): Promise => { + let handlerPromise: Promise> | undefined = undefined; + let callHandler = async (): Promise => { if (handlerPromise) { console.error("You cannot call instrumented handlers more than once"); } else { - handlerPromise = recurseRight(impls, info, handler, index - 1); + handlerPromise = recurseRight( + impls, + info, + handler, + getInnerResult, + state, + index - 1, + ); } - result = await handlerPromise; - invariant(result, "Expected a result"); - if (result.type === "error" && result.value instanceof Error) { - return { status: "error", error: result.value }; - } - return { status: "success", error: undefined }; + await handlerPromise; + invariant(state.innerResult, "Expected an inner result"); + return state.innerResult; }; try { @@ -426,14 +673,30 @@ async function recurseRight( await handlerPromise; } - if (result) { - return result; + if (state.result) { + return state.result; } - return { + state.result = { type: "error", value: new Error("No result assigned in instrumentation chain."), }; + state.innerResult = getInnerResult(state.result, info); + return state.result; +} + +type RecurseState = { + result: RecurseResult | null; + innerResult: TInnerResult | null; +}; + +function getInstrumentationInnerResult( + result: RecurseResult, +): InstrumentationHandlerResult { + if (result.type === "error" && result.value instanceof Error) { + return { status: "error", error: result.value }; + } + return { status: "success", error: undefined }; } function getHandlerInfo( @@ -442,11 +705,11 @@ function getHandlerInfo( | ActionFunctionArgs | Parameters[0], ): RouteHandlerInstrumentationInfo { - let { request, context, params, pattern } = args; + let { request, context, params } = args; return { + ...args, request: getReadonlyRequest(request), params: { ...params }, - pattern, context: getReadonlyContext(context), }; } @@ -465,6 +728,7 @@ function getRouterInfo( ...("body" in opts ? { body: opts.body } : {}), }; } + // Return a shallow readonly "clone" of the Request with the info they may // want to read from during instrumentation function getReadonlyRequest(request: Request): { diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index dace75a307..7e530e4c3e 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -10,11 +10,14 @@ import { } from "./history"; import type { ClientInstrumentation, + InstrumentationMetaReceiver, + InstrumentationResultMeta, InstrumentRouteFunction, InstrumentRouterFunction, ServerInstrumentation, } from "./instrumentation"; import { + consumeInstrumentationClientResultMetaReceiver, getRouteInstrumentationUpdates, instrumentClientSideRouter, } from "./instrumentation"; @@ -56,6 +59,7 @@ import { ResultType, convertRouteMatchToUiMatch, convertRoutesToDataRoutes, + createDataFunctionUrl, getPathContributingMatches, getResolveToMatches, isAbsoluteUrl, @@ -1673,6 +1677,11 @@ export function createRouter(init: RouterInit): Router { return promise; } + // Consume this immediately before any async work kicks off so it doesn't stick + // around for subsequent interrupting navigations + let instrumentationNavigateMetaReceiver = + consumeInstrumentationClientResultMetaReceiver(router); + let normalizedPath = normalizeTo( state.location, state.matches, @@ -1791,6 +1800,7 @@ export function createRouter(init: RouterInit): Router { enableViewTransition: opts && opts.viewTransition, flushSync, callSiteDefaultShouldRevalidate: opts && opts.defaultShouldRevalidate, + instrumentationNavigateMetaReceiver, }); } @@ -1870,6 +1880,7 @@ export function createRouter(init: RouterInit): Router { enableViewTransition?: boolean; flushSync?: boolean; callSiteDefaultShouldRevalidate?: boolean; + instrumentationNavigateMetaReceiver?: InstrumentationMetaReceiver; }, ): Promise { // Abort any in-progress navigations and start a new one. Unset any ongoing @@ -1927,6 +1938,15 @@ export function createRouter(init: RouterInit): Router { matches = fogOfWar.matches; } + if (opts?.instrumentationNavigateMetaReceiver) { + let meta = getInstrumentationNavigateMeta( + init.history, + location, + matches, + ); + opts.instrumentationNavigateMetaReceiver(meta); + } + // Short circuit with a 404 on the root error boundary if we match nothing if (!matches) { let { error, notFoundMatches, route } = handleNavigational404( @@ -2592,6 +2612,11 @@ export function createRouter(init: RouterInit): Router { let flushSync = (opts && opts.flushSync) === true; + // Consume this immediately before any async work kicks off so it doesn't stick + // around for subsequent interrupting calls + let instrumentationResultMetaReceiver = + consumeInstrumentationClientResultMetaReceiver(router); + let routesToUse = dataRoutes.activeRoutes; let normalizedPath = normalizeTo( state.location, @@ -2614,6 +2639,15 @@ export function createRouter(init: RouterInit): Router { matches = fogOfWar.matches; } + if (instrumentationResultMetaReceiver) { + let meta = getInstrumentationNavigateMeta( + init.history, + normalizedPath, + matches, + ); + instrumentationResultMetaReceiver(meta); + } + if (!matches) { setFetcherError( key, @@ -6946,34 +6980,6 @@ function createClientSideRequest( return new Request(url, init); } -// Create the normalized URL instance to pass to loaders/actions/middleware. -// We strip the `?index` param because that is a React Router implementation detail. -function createDataFunctionUrl(request: Request, path: To): URL { - let url = new URL(request.url); - - let parsed = typeof path === "string" ? parsePath(path) : path; - url.pathname = parsed.pathname || "/"; - - if (parsed.search) { - let searchParams = new URLSearchParams(parsed.search); - - // Strip naked index param, preserve any other index params with values - let indexValues = searchParams.getAll("index"); - searchParams.delete("index"); - for (let value of indexValues.filter(Boolean)) { - searchParams.append("index", value); - } - let search = searchParams.toString(); - url.search = search ? `?${search}` : ""; - } else { - url.search = ""; - } - - url.hash = parsed.hash || ""; - - return url; -} - function convertFormDataToSearchParams(formData: FormData): URLSearchParams { let searchParams = new URLSearchParams(); @@ -7463,6 +7469,18 @@ function getTargetMatch(matches: DataRouteMatch[], location: Path | string) { return pathMatches[pathMatches.length - 1]; } +function getInstrumentationNavigateMeta( + history: History, + location: To, + matches: DataRouteMatch[] | null, +): InstrumentationResultMeta { + return { + url: createDataFunctionUrl(history.createURL(location), location), + pattern: matches ? getRoutePattern(matches) : "", + params: matches?.[0]?.params ? { ...matches[0].params } : {}, + }; +} + function getSubmissionFromNavigation( navigation: Navigation, ): Submission | undefined { diff --git a/packages/react-router/lib/router/utils.ts b/packages/react-router/lib/router/utils.ts index 5498cf95b9..7c7c5fc81c 100644 --- a/packages/react-router/lib/router/utils.ts +++ b/packages/react-router/lib/router/utils.ts @@ -2276,11 +2276,48 @@ by the star-slash in the `getRoutePattern` regex and messes up the parsed commen for `isRouteErrorResponse` above. This comment seems to reset the parser. */ -export function getRoutePattern(matches: RouteMatch[]) { +// Accept the narrow shape we read so this can be used with server-runtime +// matches, which do not include the full RouteMatch fields like pathnameBase. +export function getRoutePattern(matches: { route: { path?: string } }[]) { let parts = matches.map((m) => m.route.path).filter(Boolean) as string[]; return joinPaths(parts) || "/"; } +// Create the normalized URL instance to pass to loaders/actions/middleware. +// We strip the `?index` param because that is a React Router implementation detail. +export function createDataFunctionUrl( + request: Request | URL | string, + path: To, +): URL { + let url = new URL( + typeof request === "string" || request instanceof URL + ? request + : request.url, + ); + + let parsed = typeof path === "string" ? parsePath(path) : path; + url.pathname = parsed.pathname || "/"; + + if (parsed.search) { + let searchParams = new URLSearchParams(parsed.search); + + // Strip naked index param, preserve any other index params with values + let indexValues = searchParams.getAll("index"); + searchParams.delete("index"); + for (let value of indexValues.filter(Boolean)) { + searchParams.append("index", value); + } + let search = searchParams.toString(); + url.search = search ? `?${search}` : ""; + } else { + url.search = ""; + } + + url.hash = parsed.hash || ""; + + return url; +} + export const isBrowser = typeof window !== "undefined" && typeof window.document !== "undefined" && diff --git a/packages/react-router/lib/server-runtime/server.ts b/packages/react-router/lib/server-runtime/server.ts index 1b24447502..91edf2c315 100644 --- a/packages/react-router/lib/server-runtime/server.ts +++ b/packages/react-router/lib/server-runtime/server.ts @@ -42,7 +42,11 @@ import { getDocumentHeaders } from "./headers"; import type { EntryRoute } from "../dom/ssr/routes"; import { URL_LIMIT, getManifestPath } from "../dom/ssr/fog-of-war"; import type { InstrumentRequestHandlerFunction } from "../router/instrumentation"; -import { instrumentHandler } from "../router/instrumentation"; +import { + instrumentationResultMetaContext, + instrumentHandler, +} from "../router/instrumentation"; +import { createDataFunctionUrl, getRoutePattern } from "../router/utils"; import { throwIfPotentialCSRFAttack } from "../actions"; import { getNormalizedPath } from "./urls"; @@ -76,6 +80,9 @@ function derive(build: ServerBuild, mode?: string) { ); } }); + let requestHandlerInstrumentations = build.entry.module.instrumentations + ?.map((i) => i.handler) + .filter(Boolean) as InstrumentRequestHandlerFunction[]; let requestHandler: RequestHandler = async (request, initialContext) => { let params: RouteMatch["params"] = {}; @@ -105,7 +112,8 @@ function derive(build: ServerBuild, mode?: string) { loadContext = initialContext || new RouterContextProvider(); let requestUrl = new URL(request.url); - let normalizedPathname = getNormalizedPath(request).pathname; + let normalizedPath = getNormalizedPath(request); + let normalizedPathname = normalizedPath.pathname; let isSpaMode = getBuildTimeHeader(request, "X-React-Router-SPA-Mode") === "yes"; @@ -207,6 +215,13 @@ function derive(build: ServerBuild, mode?: string) { if (matches && matches.length > 0) { Object.assign(params, matches[0].params); } + if (requestHandlerInstrumentations?.length) { + loadContext.set(instrumentationResultMetaContext, { + url: createDataFunctionUrl(request, normalizedPath), + pattern: matches ? getRoutePattern(matches) : "", + params: matches?.[0]?.params ? { ...matches[0].params } : {}, + }); + } let response: Response; if (requestUrl.pathname.endsWith(".data")) { @@ -295,12 +310,10 @@ function derive(build: ServerBuild, mode?: string) { return response; }; - if (build.entry.module.instrumentations) { + if (requestHandlerInstrumentations?.length) { requestHandler = instrumentHandler( requestHandler, - build.entry.module.instrumentations - .map((i) => i.handler) - .filter(Boolean) as InstrumentRequestHandlerFunction[], + requestHandlerInstrumentations, ); } From bb5dc76ef932e6ed2674e637717f5361e2b2ff10 Mon Sep 17 00:00:00 2001 From: Remix Run Bot Date: Thu, 25 Jun 2026 19:39:26 +0000 Subject: [PATCH 6/8] chore: format --- .../react-router/__tests__/router/instrumentation-test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/react-router/__tests__/router/instrumentation-test.ts b/packages/react-router/__tests__/router/instrumentation-test.ts index c7040523ff..7589f7c43a 100644 --- a/packages/react-router/__tests__/router/instrumentation-test.ts +++ b/packages/react-router/__tests__/router/instrumentation-test.ts @@ -1708,9 +1708,7 @@ describe("instrumentation", () => { }, ], ]); - expect(spy.mock.calls[0][1].url.href).toBe( - "http://localhost/redirect", - ); + expect(spy.mock.calls[0][1].url.href).toBe("http://localhost/redirect"); expect(router.state).toMatchObject({ navigation: { state: "idle" }, location: { pathname: "/page" }, From ce6dfec19dc66396ed1a29d48730ca9bf59d02bc Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 25 Jun 2026 16:37:05 -0400 Subject: [PATCH 7/8] Fix change file pr comment script --- scripts/pr.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/pr.ts b/scripts/pr.ts index 33819f424b..7575e5465c 100644 --- a/scripts/pr.ts +++ b/scripts/pr.ts @@ -169,9 +169,9 @@ async function changeFileCheck(ctx: CheckContext): Promise { CHANGE_FILE_FOUND_COMMENT, "| Type | Change |", "| --- | --- |", - ...summaries - .map((s) => `| \`${s.type}\` | ${s.firstLine.replaceAll("|", "\\|")} |`) - .join("\n"), + ...summaries.map( + (s) => `| \`${s.type}\` | ${s.firstLine.replaceAll("|", "\\|")} |`, + ), ].join("\n"); } From db526f08df9e7678e1265269f782b3dec62454f1 Mon Sep 17 00:00:00 2001 From: Avi Vahl Date: Fri, 26 Jun 2026 00:09:15 +0300 Subject: [PATCH 8/8] fix: warning with vite@8.1.0 (#15230) * fix: warning with vite@8.1.0 the new vite version started warning about this deprecated option being used. envDir was added in vite@6.3.0, so react-router is safe to use it. refs: https://github.com/vitejs/vite/blob/v8.1.0/packages/vite/CHANGELOG.md https://github.com/vitejs/vite/pull/22555 * docs: add a changelog entry * Apply suggestion from @brophdawg11 --------- Co-authored-by: Matt Brophy --- contributors.yml | 1 + .../.changes/patch.fix-envfile-warning-appeared-vite810.md | 1 + packages/react-router-dev/vite/plugin.ts | 2 +- packages/react-router-dev/vite/vite-runner.ts | 2 +- 4 files changed, 4 insertions(+), 2 deletions(-) create mode 100644 packages/react-router-dev/.changes/patch.fix-envfile-warning-appeared-vite810.md diff --git a/contributors.yml b/contributors.yml index b53d5b0b3c..7cd8265fed 100644 --- a/contributors.yml +++ b/contributors.yml @@ -47,6 +47,7 @@ - Artur- - ashusnapx - avipatel97 +- AviVahl - awreese - aymanemadidi - ayushmanchhabra diff --git a/packages/react-router-dev/.changes/patch.fix-envfile-warning-appeared-vite810.md b/packages/react-router-dev/.changes/patch.fix-envfile-warning-appeared-vite810.md new file mode 100644 index 0000000000..da1044700a --- /dev/null +++ b/packages/react-router-dev/.changes/patch.fix-envfile-warning-appeared-vite810.md @@ -0,0 +1 @@ +Replace the deprecated `envFile:false` Vite config with `envDir:false` to eliminate a deprecation warning when using vite@8.1.0+ diff --git a/packages/react-router-dev/vite/plugin.ts b/packages/react-router-dev/vite/plugin.ts index 4bf3439e95..a9853cd198 100644 --- a/packages/react-router-dev/vite/plugin.ts +++ b/packages/react-router-dev/vite/plugin.ts @@ -1455,7 +1455,7 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = () => { hmr: false, }, configFile: false, - envFile: false, + envDir: false, plugins: [ childCompilerPlugins // Exclude this plugin from the child compiler to prevent an diff --git a/packages/react-router-dev/vite/vite-runner.ts b/packages/react-router-dev/vite/vite-runner.ts index ba30c10808..fc0f54b766 100644 --- a/packages/react-router-dev/vite/vite-runner.ts +++ b/packages/react-router-dev/vite/vite-runner.ts @@ -46,7 +46,7 @@ export async function createContext({ postcss: {}, }, configFile: false, - envFile: false, + envDir: false, plugins: [], environments: { __config_loader: {