diff --git a/decisions/0015-observability.md b/decisions/0015-observability.md new file mode 100644 index 0000000000..e4aa369ac0 --- /dev/null +++ b/decisions/0015-observability.md @@ -0,0 +1,119 @@ +# Title + +Date: 2025-09-22 + +Status: proposed + +## Context + +We want it to be easy to add observability to production React Router applications. This involves the ability to add logging, error reporting, and performance tracing to your application on both the server and the client. + +We always had a good story for user-facing error handling via `ErrorBoundary`, but until recently we only had a server-side error-reporting solution via the `entry.server` `handleError` export. In `7.8.2`, we shipped an `onError` client-side equivalent so it should now be possible to report on errors on the server and client pretty easily. + +We have not historically had great recommendations for the other 2 facets of observability - logging and performance tracing. Middleware, shipped in `7.3.0` and stabilized in `7.9.0` gave us a way to "wrap" request handlers at any level of the tree, which provides a good solution for logging and _some_ high-level performance tracing. But it's too coarse-grained and does not allow folks to drill down into their applications. + +This has also been raised in the (currently) 2nd-most upvoted Proposal in the past year: https://github.com/remix-run/react-router/discussions/13749. + +One way to add fine-grained logging/tracing today is to manually include it in all of your loaders and actions, but this is tedious and error-prone. + +Another way is to "instrument" the server build, which has long been our suggestion - initially to the folks at Sentry - and over time to RR users here and there in discord and github issues. but, we've never formally documented this as a recommended pattern, and it currently only works on the server and requires that you use a custom server. + +## Decision + +Adopt instrumentation as a first class API and the recommended way to implement observability in your application. + +There are 2 levels in which we want to instrument: + +- router level - ability to track the start and end of a router operation + - requests on the server handler + - initialization, navigations, and fetchers on the client router +- route level + - loaders, actions, middlewares + +On the server, if you are using a custom server, this is already possible by wrapping the react router handler and walking the `build.routes` tree and wrapping the route handlers. + +To provide the same functionality when using `@react-router/serve` we need to open up a new API. Currently, I am proposing 2 new exports from `entry.server`. These will be run on the server build in `createRequestHandler` and that way can work without a custom server. This will also allow custom-server users today to move some more code from their custom server into React Router by leveraging these new exports. + +```tsx +// entry.server.tsx + +// Wrap incoming request handlers. Currently applies to _all_ requests handled +// by the RR handler, including: +// - manifest reqeusts +// - document requests +// - `.data` requests +// - resource route requests +export function instrumentHandler(handler: RequestHandler): RequestHandler { + return (...args) => { + let [request] = args; + let path = new URL(request.url).pathname; + let start = Date.now(); + console.log(`Request start: ${request.method} ${path}`); + + try { + return await handler(...args); + } finally { + let duration = Date.now() - start; + console.log(`Request end: ${request.method} ${path} (${duration}ms)`); + } + }; +} + +// Instrument an individual route, allowing you to wrap middleware/loader/action/etc. +// This also gives you a place to do global "shouldRevalidate" which is a nice side +// effect as folks have asked for that for a long time +export function instrumentRoute(route: RouteModule): RequestHandler { + let { loader } = route; + let newRoute = { ...route }; + if (loader) { + newRoute.loader = (args) => { + let { request } = args; + let path = new URL(request.url).pathname; + let start = Date.now(); + console.log(`Loader start: ${request.method} ${path}`); + + try { + return await loader(...args); + } finally { + let duration = Date.now() - start; + console.log(`Loader end: ${request.method} ${path} (${duration}ms)`); + } + }; + } + return newRoute; +} +``` + +Open questions: + +- On the server we could technically do this at build time, but I don't expect this to have a large startup cost and doing it at build-time just feels a bit more magical and would differ from any examples we want to show in data mode. +- Another option for custom server folks would be to make these parameters to `createRequestHandler`, but then we'd still need a way for `react-router-server` users to use them and thus we'd still need to support them in `entry.server`, so might as well make it consistent for both. + +Client-side, it's a similar story. You could do this today at the route level in Data mode before calling `createBrowserRouter`, and you could wrap `router.navigate`/`router.fetch` after that. but there's no way to instrument the router `initialize` method without "ejecting" to using the lower level `createRouter`. And there is no way to do this in framework mode. + +I think we can open up APIs similar to those in `entry.server` but do them on `createBrowserRouter` and `HydratedRouter`: + +```tsx +function instrumentRouter(router: DataRouter): DataRouter { /* ... */ } + +function instrumentRoute(route: RouteObject): RouteObject { /* ... */ } + +// Data mode +let router = createBrowserRouter(routes, { + instrumentRouter, + instrumentRoute, +}) + +// Framework mode + +``` + +In both of these cases, we'll handle the instrumentation at the router creation level. And by passing `instrumentRoute` into the router, we can properly instrument future routes discovered via `route.lazy` or `patchRouteOnNavigation` + +## Alternatives Considered + +Originally we wanted to add an [Events API](https://github.com/remix-run/react-router/discussions/9565), but this proved to [have issues](https://github.com/remix-run/react-router/discussions/13749#discussioncomment-14135422) with the ability to "wrap" logic for easier OTEL instrumentation. These were not [insurmountable](https://github.com/remix-run/react-router/discussions/13749#discussioncomment-14421335), but the solutions didn't feel great. + +Client side, we also considered whether this could be done via `patchRoutes`, but that's currently intended mostly to add new routes and doesn't work for `route.lazy` routes. In some RSC-use cases it can update parts of an existing route, but it sonly allows updates for the server-rendered RSC "elements," and doesn't walk the entire child tree to update children routes so it's not an ideal solution for updating loaders in the entire tree. diff --git a/packages/react-router/__tests__/router/fetchers-test.ts b/packages/react-router/__tests__/router/fetchers-test.ts index 0fca5f7a39..4d516b16ae 100644 --- a/packages/react-router/__tests__/router/fetchers-test.ts +++ b/packages/react-router/__tests__/router/fetchers-test.ts @@ -373,6 +373,7 @@ describe("fetchers", () => { request: new Request("http://localhost/foo", { signal: A.loaders.root.stub.mock.calls[0][0].request.signal, }), + pattern: expect.any(String), context: {}, }); }); @@ -3373,6 +3374,7 @@ describe("fetchers", () => { expect(F.actions.root.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); @@ -3402,6 +3404,7 @@ describe("fetchers", () => { expect(F.actions.root.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); @@ -3429,6 +3432,7 @@ describe("fetchers", () => { expect(F.actions.root.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); @@ -3456,6 +3460,7 @@ describe("fetchers", () => { expect(F.actions.root.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); @@ -3484,6 +3489,7 @@ describe("fetchers", () => { expect(F.actions.root.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); @@ -3514,6 +3520,7 @@ describe("fetchers", () => { expect(F.actions.root.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); @@ -3543,6 +3550,7 @@ describe("fetchers", () => { expect(F.actions.root.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); diff --git a/packages/react-router/__tests__/router/instrumentation-test.ts b/packages/react-router/__tests__/router/instrumentation-test.ts new file mode 100644 index 0000000000..65480e89c0 --- /dev/null +++ b/packages/react-router/__tests__/router/instrumentation-test.ts @@ -0,0 +1,308 @@ +import { LoaderFunction } from "../../lib/router/utils"; +import { cleanup, setup } from "./utils/data-router-setup"; +import { createDeferred, createFormData, tick } from "./utils/utils"; + +// Detect any failures inside the router navigate code +afterEach(() => { + cleanup(); +}); + +describe("instrumentation", () => { + it("allows instrumentation of lazy", async () => { + let spy = jest.fn(); + let lazyDfd = createDeferred<{ loader: LoaderFunction }>(); + let t = setup({ + routes: [ + { + index: true, + }, + { + id: "page", + path: "/page", + lazy: () => lazyDfd.promise, + }, + ], + unstable_instrumentRoute: (route) => { + route.instrument({ + async lazy(lazy) { + spy("start"); + await lazy(); + spy("end"); + }, + }); + }, + }); + + await t.navigate("/page"); + expect(spy.mock.calls).toEqual([["start"]]); + + await lazyDfd.resolve({ loader: () => "PAGE" }); + expect(spy.mock.calls).toEqual([["start"], ["end"]]); + await tick(); + expect(spy.mock.calls).toEqual([["start"], ["end"]]); + expect(t.router.state).toMatchObject({ + navigation: { state: "idle" }, + location: { pathname: "/page" }, + loaderData: { page: "PAGE" }, + }); + }); + + it("allows instrumentation of loaders", async () => { + let spy = jest.fn(); + let t = setup({ + routes: [ + { + index: true, + }, + { + id: "page", + path: "/page", + loader: true, + }, + ], + unstable_instrumentRoute: (route) => { + route.instrument({ + async loader(loader) { + spy("start"); + await loader(); + spy("end"); + }, + }); + }, + }); + + let A = await t.navigate("/page"); + expect(spy).toHaveBeenNthCalledWith(1, "start"); + await A.loaders.page.resolve("PAGE"); + expect(spy).toHaveBeenNthCalledWith(2, "end"); + expect(t.router.state).toMatchObject({ + navigation: { state: "idle" }, + location: { pathname: "/page" }, + loaderData: { page: "PAGE" }, + }); + }); + + it("allows instrumentation of actions", async () => { + let spy = jest.fn(); + let t = setup({ + routes: [ + { + index: true, + }, + { + id: "page", + path: "/page", + action: true, + }, + ], + unstable_instrumentRoute: (route) => { + route.instrument({ + async action(action) { + spy("start"); + await action(); + spy("end"); + }, + }); + }, + }); + + let A = await t.navigate("/page", { + formMethod: "POST", + formData: createFormData({}), + }); + expect(spy).toHaveBeenNthCalledWith(1, "start"); + await A.actions.page.resolve("PAGE"); + expect(spy).toHaveBeenNthCalledWith(2, "end"); + expect(t.router.state).toMatchObject({ + navigation: { state: "idle" }, + location: { pathname: "/page" }, + actionData: { page: "PAGE" }, + }); + }); + + it("allows instrumentation of loaders when lazy is used", async () => { + let spy = jest.fn(); + let lazyDfd = createDeferred<{ loader: LoaderFunction }>(); + let loaderDfd = createDeferred(); + let t = setup({ + routes: [ + { + index: true, + }, + { + id: "page", + path: "/page", + lazy: () => lazyDfd.promise, + }, + ], + unstable_instrumentRoute: (route) => { + route.instrument({ + async loader(loader) { + spy("start"); + await loader(); + spy("end"); + }, + }); + }, + }); + + await t.navigate("/page"); + expect(spy).not.toHaveBeenCalled(); + + await lazyDfd.resolve({ loader: () => loaderDfd.promise }); + expect(spy.mock.calls).toEqual([["start"]]); + + await loaderDfd.resolve("PAGE"); + expect(spy.mock.calls).toEqual([["start"], ["end"]]); + + await tick(); + expect(t.router.state).toMatchObject({ + navigation: { state: "idle" }, + location: { pathname: "/page" }, + loaderData: { page: "PAGE" }, + }); + }); + + it("does not double-instrument when a static `loader` is used alongside `lazy`", async () => { + let spy = jest.fn(); + let lazyDfd = createDeferred<{ loader: LoaderFunction }>(); + let warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}); + let t = setup({ + routes: [ + { + index: true, + }, + { + id: "page", + path: "/page", + loader: true, + lazy: () => lazyDfd.promise, + }, + ], + unstable_instrumentRoute: (route) => { + route.instrument({ + async loader(loader) { + spy("start"); + await loader(); + spy("end"); + }, + }); + }, + }); + + let A = await t.navigate("/page"); + expect(spy.mock.calls).toEqual([["start"]]); + await lazyDfd.resolve({ action: () => "ACTION", loader: () => "WRONG" }); + await A.loaders.page.resolve("PAGE"); + await tick(); + expect(spy.mock.calls).toEqual([["start"], ["end"]]); + expect(t.router.state).toMatchObject({ + navigation: { state: "idle" }, + location: { pathname: "/page" }, + loaderData: { page: "PAGE" }, + }); + spy.mockClear(); + + let B = await t.navigate("/page", { + formMethod: "POST", + formData: createFormData({}), + }); + expect(spy).not.toHaveBeenCalled(); + await B.loaders.page.resolve("PAGE"); + await tick(); + expect(spy.mock.calls).toEqual([["start"], ["end"]]); + expect(t.router.state).toMatchObject({ + navigation: { state: "idle" }, + location: { pathname: "/page" }, + loaderData: { page: "PAGE" }, + }); + spy.mockClear(); + + warnSpy.mockRestore(); + }); + + it("provides read-only information to instrumentation wrappers", async () => { + let spy = jest.fn(); + let t = setup({ + routes: [ + { + index: true, + }, + { + id: "slug", + path: "/:slug", + loader: true, + }, + ], + unstable_instrumentRoute: (route) => { + route.instrument({ + async loader(loader, info) { + spy(info); + Object.assign(info.params, { extra: "extra" }); + await loader(); + }, + }); + }, + }); + + let A = await t.navigate("/a"); + await A.loaders.slug.resolve("A"); + let args = spy.mock.calls[0][0]; + expect(args.request.method).toBe("GET"); + expect(args.request.url).toBe("http://localhost/a"); + expect(args.request.url).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" }); + expect(args.pattern).toBe("/:slug"); + expect(args.context.get).toBeDefined(); + expect(args.context.set).not.toBeDefined(); + expect(t.router.state.matches[0].params).toEqual({ slug: "a" }); + }); + + it("allows composition of multiple instrumentations", async () => { + let spy = jest.fn(); + let t = setup({ + routes: [ + { + index: true, + }, + { + id: "page", + path: "/page", + loader: true, + }, + ], + unstable_instrumentRoute: (route) => { + route.instrument({ + async loader(loader) { + spy("start inner"); + await loader(); + spy("end inner"); + }, + }); + route.instrument({ + async loader(loader) { + spy("start outer"); + await loader(); + spy("end outer"); + }, + }); + }, + }); + + let A = await t.navigate("/page"); + await A.loaders.page.resolve("PAGE"); + expect(spy.mock.calls).toEqual([ + ["start outer"], + ["start inner"], + ["end inner"], + ["end outer"], + ]); + expect(t.router.state).toMatchObject({ + navigation: { state: "idle" }, + location: { pathname: "/page" }, + loaderData: { page: "PAGE" }, + }); + }); +}); diff --git a/packages/react-router/__tests__/router/router-test.ts b/packages/react-router/__tests__/router/router-test.ts index 704d7eff3f..3580b2d176 100644 --- a/packages/react-router/__tests__/router/router-test.ts +++ b/packages/react-router/__tests__/router/router-test.ts @@ -1503,6 +1503,7 @@ describe("a router", () => { request: new Request("http://localhost/tasks", { signal: nav.loaders.tasks.stub.mock.calls[0][0].request.signal, }), + pattern: "/tasks", context: {}, }); @@ -1512,6 +1513,7 @@ describe("a router", () => { request: new Request("http://localhost/tasks/1", { signal: nav2.loaders.tasksId.stub.mock.calls[0][0].request.signal, }), + pattern: "/tasks/:id", context: {}, }); @@ -1521,6 +1523,7 @@ describe("a router", () => { request: new Request("http://localhost/tasks?foo=bar", { signal: nav3.loaders.tasks.stub.mock.calls[0][0].request.signal, }), + pattern: "/tasks", context: {}, }); @@ -1532,6 +1535,7 @@ describe("a router", () => { request: new Request("http://localhost/tasks?foo=bar", { signal: nav4.loaders.tasks.stub.mock.calls[0][0].request.signal, }), + pattern: "/tasks", context: {}, }); @@ -1930,6 +1934,7 @@ describe("a router", () => { expect(nav.actions.tasks.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: "/tasks", context: {}, }); @@ -1974,6 +1979,7 @@ describe("a router", () => { expect(nav.actions.tasks.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); // Assert request internals, cannot do a deep comparison above since some @@ -2007,6 +2013,7 @@ describe("a router", () => { expect(nav.actions.tasks.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); diff --git a/packages/react-router/__tests__/router/submission-test.ts b/packages/react-router/__tests__/router/submission-test.ts index a89bb430e1..75e296227e 100644 --- a/packages/react-router/__tests__/router/submission-test.ts +++ b/packages/react-router/__tests__/router/submission-test.ts @@ -948,6 +948,7 @@ describe("submissions", () => { expect(nav.actions.root.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); @@ -982,6 +983,7 @@ describe("submissions", () => { expect(nav.actions.root.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); @@ -1014,6 +1016,7 @@ describe("submissions", () => { expect(nav.actions.root.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); @@ -1118,6 +1121,7 @@ describe("submissions", () => { expect(nav.actions.root.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); @@ -1156,6 +1160,7 @@ describe("submissions", () => { expect(nav.actions.root.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); @@ -1191,6 +1196,7 @@ describe("submissions", () => { expect(nav.actions.root.stub).toHaveBeenCalledWith({ params: {}, request: expect.any(Request), + pattern: expect.any(String), context: {}, }); diff --git a/packages/react-router/__tests__/router/utils/data-router-setup.ts b/packages/react-router/__tests__/router/utils/data-router-setup.ts index 0a2dc3a9cc..dd1d253fcd 100644 --- a/packages/react-router/__tests__/router/utils/data-router-setup.ts +++ b/packages/react-router/__tests__/router/utils/data-router-setup.ts @@ -5,6 +5,7 @@ import type { HydrationState, Router, RouterNavigateOptions, + RouterInit, } from "../../../lib/router/router"; import type { AgnosticDataRouteObject, @@ -134,14 +135,10 @@ export const TASK_ROUTES: TestRouteObject[] = [ }, ]; -type SetupOpts = { +type SetupOpts = Omit & { routes: TestRouteObject[]; - basename?: string; initialEntries?: InitialEntry[]; initialIndex?: number; - hydrationRouteProperties?: string[]; - hydrationData?: HydrationState; - dataStrategy?: DataStrategyFunction; }; // We use a slightly modified version of createDeferred here that includes the @@ -202,12 +199,9 @@ export function getFetcherData(router: Router) { export function setup({ routes, - basename, initialEntries, initialIndex, - hydrationRouteProperties, - hydrationData, - dataStrategy, + ...routerInit }: SetupOpts) { let guid = 0; // Global "active" helpers, keyed by navType:guid:loaderOrAction:routeId. @@ -318,13 +312,10 @@ export function setup({ jest.spyOn(history, "push"); jest.spyOn(history, "replace"); currentRouter = createRouter({ - basename, history, routes: enhanceRoutes(routes), - hydrationRouteProperties, - hydrationData, window: testWindow, - dataStrategy: dataStrategy, + ...routerInit, }); let fetcherData = getFetcherData(currentRouter); diff --git a/packages/react-router/lib/components.tsx b/packages/react-router/lib/components.tsx index c903ca829a..e4c964c4d9 100644 --- a/packages/react-router/lib/components.tsx +++ b/packages/react-router/lib/components.tsx @@ -1,6 +1,7 @@ import * as React from "react"; import type { + History, InitialEntry, Location, MemoryHistory, @@ -168,6 +169,19 @@ export interface MemoryRouterOpts { * Index of `initialEntries` the application should initialize to */ initialIndex?: number; + /** + * Function allowing you to instrument a route object prior to creating the + * client-side router (and on any subsequently added routes via `route.lazy` or + * `patchRoutesOnNavigation`). This is mostly useful for observability such + * as wrapping loaders/actions/middlewares with logging and/or performance tracing. + */ + unstable_instrumentRoute?: (r: DataRouteObject) => DataRouteObject; + /** + * Function allowing you to instrument the client-side router. This is mostly + * useful for observability such as wrapping `router.navigate`/`router.fetch` + * with logging and/or performance tracing. + */ + unstable_instrumentRouter?: (r: DataRouter) => DataRouter; /** * Override the default data strategy of loading in parallel. * Only intended for advanced usage. @@ -196,6 +210,8 @@ export interface MemoryRouterOpts { * @param {MemoryRouterOpts.hydrationData} opts.hydrationData n/a * @param {MemoryRouterOpts.initialEntries} opts.initialEntries n/a * @param {MemoryRouterOpts.initialIndex} opts.initialIndex n/a + * @param {MemoryRouterOpts.unstable_instrumentRoute} opts.unstable_instrumentRoute n/a + * @param {MemoryRouterOpts.unstable_instrumentRouter} opts.unstable_instrumentRouter n/a * @param {MemoryRouterOpts.patchRoutesOnNavigation} opts.patchRoutesOnNavigation n/a * @returns An initialized {@link DataRouter} to pass to {@link RouterProvider | ``} */ @@ -203,21 +219,45 @@ export function createMemoryRouter( routes: RouteObject[], opts?: MemoryRouterOpts, ): DataRouter { - return createRouter({ - basename: opts?.basename, - getContext: opts?.getContext, - future: opts?.future, - history: createMemoryHistory({ + return createAndInitializeDataRouter( + routes, + createMemoryHistory({ initialEntries: opts?.initialEntries, initialIndex: opts?.initialIndex, }), + opts, + ); +} + +export function createAndInitializeDataRouter( + routes: RouteObject[], + history: History, + opts?: Omit< + RouterInit, + "routes" | "history" | "mapRouteProperties" | "hydrationRouteProperties" + > & { + unstable_instrumentRouter?: (r: DataRouter) => DataRouter; + }, +): DataRouter { + let router = createRouter({ + basename: opts?.basename, + dataStrategy: opts?.dataStrategy, + future: opts?.future, + getContext: opts?.getContext, + history, hydrationData: opts?.hydrationData, - routes, hydrationRouteProperties, + unstable_instrumentRoute: opts?.unstable_instrumentRoute, mapRouteProperties, - dataStrategy: opts?.dataStrategy, patchRoutesOnNavigation: opts?.patchRoutesOnNavigation, - }).initialize(); + routes, + }); + + if (opts?.unstable_instrumentRouter) { + router = opts.unstable_instrumentRouter(router); + } + + return router.initialize(); } class Deferred { diff --git a/packages/react-router/lib/dom-export/hydrated-router.tsx b/packages/react-router/lib/dom-export/hydrated-router.tsx index c6429c82f1..222259e040 100644 --- a/packages/react-router/lib/dom-export/hydrated-router.tsx +++ b/packages/react-router/lib/dom-export/hydrated-router.tsx @@ -7,6 +7,7 @@ import type { HydrationState, RouterInit, unstable_ClientOnErrorFunction, + DataRouteObject, } from "react-router"; import { UNSAFE_getHydrationData as getHydrationData, @@ -78,8 +79,12 @@ function initSsrInfo(): void { function createHydratedRouter({ getContext, + unstable_instrumentRoute, + unstable_instrumentRouter, }: { getContext?: RouterInit["getContext"]; + unstable_instrumentRoute?: RouterInit["unstable_instrumentRoute"]; + unstable_instrumentRouter?: (r: DataRouter) => DataRouter; }): DataRouter { initSsrInfo(); @@ -172,6 +177,7 @@ function createHydratedRouter({ getContext, hydrationData, hydrationRouteProperties, + unstable_instrumentRoute, mapRouteProperties, future: { middleware: ssrInfo.context.future.v8_middleware, @@ -192,6 +198,11 @@ function createHydratedRouter({ ssrInfo.context.basename, ), }); + + if (unstable_instrumentRouter) { + router = unstable_instrumentRouter(router); + } + ssrInfo.router = router; // We can call initialize() immediately if the router doesn't have any @@ -223,6 +234,18 @@ export interface HydratedRouterProps { * functions */ getContext?: RouterInit["getContext"]; + /** + * Function allowing you to instrument a route object prior to creating the + * client-side router. This is mostly useful for observability such as wrapping + * loaders/actions/middlewares with logging and/or performance tracing. + */ + unstable_instrumentRoute(route: DataRouteObject): DataRouteObject; + /** + * Function allowing you to instrument the client-side router. This is mostly + * useful for observability such as wrapping `router.navigate`/`router.fetch` + * with logging and/or performance tracing. + */ + unstable_instrumentRouter(router: DataRouter): DataRouter; /** * An error handler function that will be called for any loader/action/render * errors that are encountered in your application. This is useful for @@ -259,6 +282,8 @@ export function HydratedRouter(props: HydratedRouterProps) { if (!router) { router = createHydratedRouter({ getContext: props.getContext, + unstable_instrumentRoute: props.unstable_instrumentRoute, + unstable_instrumentRouter: props.unstable_instrumentRouter, }); } diff --git a/packages/react-router/lib/dom/lib.tsx b/packages/react-router/lib/dom/lib.tsx index ecbe52374e..43471e358a 100644 --- a/packages/react-router/lib/dom/lib.tsx +++ b/packages/react-router/lib/dom/lib.tsx @@ -75,6 +75,7 @@ import type { RouteObject, NavigateOptions, PatchRoutesOnNavigationFunction, + DataRouteObject, } from "../context"; import { DataRouterContext, @@ -235,6 +236,19 @@ export interface DOMRouterOpts { * ``` */ hydrationData?: HydrationState; + /** + * Function allowing you to instrument a route object prior to creating the + * client-side router (and on any subsequently added routes via `route.lazy` or + * `patchRoutesOnNavigation`). This is mostly useful for observability such + * as wrapping loaders/actions/middlewares with logging and/or performance tracing. + */ + unstable_instrumentRoute?: (r: DataRouteObject) => DataRouteObject; + /** + * Function allowing you to instrument the client-side router. This is mostly + * useful for observability such as wrapping `router.navigate`/`router.fetch` + * with logging and/or performance tracing. + */ + unstable_instrumentRouter?: (r: DataRouter) => DataRouter; /** * Override the default data strategy of running loaders in parallel. * See {@link DataStrategyFunction}. @@ -741,6 +755,8 @@ export interface DOMRouterOpts { * @param {DOMRouterOpts.future} opts.future n/a * @param {DOMRouterOpts.getContext} opts.getContext n/a * @param {DOMRouterOpts.hydrationData} opts.hydrationData n/a + * @param {DOMRouterOpts.unstable_instrumentRoute} opts.unstable_instrumentRoute n/a + * @param {DOMRouterOpts.unstable_instrumentRouter} opts.unstable_instrumentRouter n/a * @param {DOMRouterOpts.patchRoutesOnNavigation} opts.patchRoutesOnNavigation n/a * @param {DOMRouterOpts.window} opts.window n/a * @returns An initialized {@link DataRouter| data router} to pass to {@link RouterProvider | ``} @@ -749,19 +765,26 @@ export function createBrowserRouter( routes: RouteObject[], opts?: DOMRouterOpts, ): DataRouter { - return createRouter({ + let router = createRouter({ basename: opts?.basename, getContext: opts?.getContext, future: opts?.future, history: createBrowserHistory({ window: opts?.window }), hydrationData: opts?.hydrationData || parseHydrationData(), + unstable_instrumentRoute: opts?.unstable_instrumentRoute, routes, mapRouteProperties, hydrationRouteProperties, dataStrategy: opts?.dataStrategy, patchRoutesOnNavigation: opts?.patchRoutesOnNavigation, window: opts?.window, - }).initialize(); + }); + + if (opts?.unstable_instrumentRouter) { + router = opts.unstable_instrumentRouter(router); + } + + return router.initialize(); } /** @@ -777,6 +800,8 @@ export function createBrowserRouter( * @param {DOMRouterOpts.future} opts.future n/a * @param {DOMRouterOpts.getContext} opts.getContext n/a * @param {DOMRouterOpts.hydrationData} opts.hydrationData n/a + * @param {DOMRouterOpts.unstable_instrumentRoute} opts.unstable_instrumentRoute n/a + * @param {DOMRouterOpts.unstable_instrumentRouter} opts.unstable_instrumentRouter n/a * @param {DOMRouterOpts.dataStrategy} opts.dataStrategy n/a * @param {DOMRouterOpts.patchRoutesOnNavigation} opts.patchRoutesOnNavigation n/a * @param {DOMRouterOpts.window} opts.window n/a @@ -786,19 +811,26 @@ export function createHashRouter( routes: RouteObject[], opts?: DOMRouterOpts, ): DataRouter { - return createRouter({ + let router = createRouter({ basename: opts?.basename, getContext: opts?.getContext, future: opts?.future, history: createHashHistory({ window: opts?.window }), hydrationData: opts?.hydrationData || parseHydrationData(), + unstable_instrumentRoute: opts?.unstable_instrumentRoute, routes, mapRouteProperties, hydrationRouteProperties, dataStrategy: opts?.dataStrategy, patchRoutesOnNavigation: opts?.patchRoutesOnNavigation, window: opts?.window, - }).initialize(); + }); + + if (opts?.unstable_instrumentRouter) { + router = opts.unstable_instrumentRouter(router); + } + + return router.initialize(); } function parseHydrationData(): HydrationState | undefined { diff --git a/packages/react-router/lib/dom/ssr/routes.tsx b/packages/react-router/lib/dom/ssr/routes.tsx index 7c1f773a6d..4ffaf4fe36 100644 --- a/packages/react-router/lib/dom/ssr/routes.tsx +++ b/packages/react-router/lib/dom/ssr/routes.tsx @@ -340,7 +340,7 @@ export function createClientRoutes( (routeModule.clientLoader?.hydrate === true || !route.hasLoader); dataRoute.loader = async ( - { request, params, context }: LoaderFunctionArgs, + { request, params, context, pattern }: LoaderFunctionArgs, singleFetch?: unknown, ) => { try { @@ -358,6 +358,7 @@ export function createClientRoutes( request, params, context, + pattern, async serverLoader() { preventInvalidServerHandlerCall("loader", route); @@ -393,7 +394,7 @@ export function createClientRoutes( ); dataRoute.action = ( - { request, params, context }: ActionFunctionArgs, + { request, params, context, pattern }: ActionFunctionArgs, singleFetch?: unknown, ) => { return prefetchStylesAndCallHandler(async () => { @@ -412,6 +413,7 @@ export function createClientRoutes( request, params, context, + pattern, async serverAction() { preventInvalidServerHandlerCall("action", route); return fetchServerAction(singleFetch); diff --git a/packages/react-router/lib/router/instrumentation.ts b/packages/react-router/lib/router/instrumentation.ts new file mode 100644 index 0000000000..1032e21bd8 --- /dev/null +++ b/packages/react-router/lib/router/instrumentation.ts @@ -0,0 +1,159 @@ +import type { + ActionFunction, + AgnosticDataRouteObject, + LazyRouteFunction, + LoaderFunction, + LoaderFunctionArgs, + MaybePromise, + RouterContextProvider, +} from "./utils"; + +type InstrumentationInfo = Readonly<{ + request: { + method: string; + url: string; + headers: Pick; + }; + params: LoaderFunctionArgs["params"]; + pattern: string; + // TODO: Fix for non-middleware + context: Pick; +}>; + +type InstrumentHandlerFunction = ( + handler: () => undefined, + info: InstrumentationInfo, +) => MaybePromise; + +type InstrumentLazyFunction = ( + handler: () => undefined, + info: InstrumentationInfo, +) => MaybePromise; + +type Instrumentations = { + lazy?: InstrumentLazyFunction; + loader?: InstrumentHandlerFunction; + action?: InstrumentHandlerFunction; +}; + +type InstrumentableRoute = { + id: string; + index: boolean | undefined; + path: string | undefined; + instrument(instrumentations: Instrumentations): void; +}; + +const UninstrumentedSymbol = Symbol("Uninstrumented"); + +export type unstable_InstrumentRouteFunction = ( + route: InstrumentableRoute, +) => void; + +function getInstrumentedHandler< + H extends + | LazyRouteFunction + | LoaderFunction + | ActionFunction, +>(impls: InstrumentHandlerFunction[], handler: H): H | null { + if (impls.length === 0) { + return null; + } + return async (...args: Parameters) => { + let value; + let composed = impls.reduce( + (acc, fn) => (i) => fn(acc as () => undefined, i), + async () => { + value = await handler(...args); + }, + ) as unknown as (info: InstrumentationInfo) => Promise; + let composedArgs = args[0] ? [getInstrumentationInfo(args[0])] : []; + await composed(...composedArgs); + return value; + }; +} + +function getInstrumentationInfo(args: LoaderFunctionArgs): InstrumentationInfo { + let { request, context, params, pattern } = args; + return { + // pseudo "Request" with the info they may want to read from + request: { + method: request.method, + url: request.url, + // Maybe make this a proxy that only supports `get`? + headers: { + get: (...args) => request.headers.get(...args), + }, + }, + params: { ...params }, + pattern, + context: { + get: (...args: Parameters) => + context.get(...args), + }, + }; +} + +export function getInstrumentationUpdates( + unstable_instrumentRoute: unstable_InstrumentRouteFunction, + route: AgnosticDataRouteObject, +) { + let instrumentations: Instrumentations[] = []; + unstable_instrumentRoute({ + id: route.id, + index: route.index, + path: route.path, + instrument(i) { + instrumentations.push(i); + }, + }); + + let updates: { + loader?: AgnosticDataRouteObject["loader"]; + action?: AgnosticDataRouteObject["action"]; + lazy?: AgnosticDataRouteObject["lazy"]; + } = {}; + + if (instrumentations.length > 0) { + if (typeof route.lazy === "function") { + let instrumented = getInstrumentedHandler( + instrumentations + .map((i) => i.lazy) + .filter(Boolean) as InstrumentHandlerFunction[], + route.lazy, + ); + if (instrumented) { + updates.lazy = instrumented; + } + } + + if (typeof route.loader === "function") { + // @ts-expect-error + let original = route.loader[UninstrumentedSymbol] ?? route.loader; + let instrumented = getInstrumentedHandler( + instrumentations + .map((i) => i.loader) + .filter(Boolean) as InstrumentHandlerFunction[], + original, + ); + if (instrumented) { + // @ts-expect-error Avoid double-instrumentation on lazy calls to + // `mapRouteProperties` + instrumented[UninstrumentedSymbol] = original; + updates.loader = instrumented; + } + } + + if (typeof route.action === "function") { + let instrumented = getInstrumentedHandler( + instrumentations + .map((i) => i.action) + .filter(Boolean) as InstrumentHandlerFunction[], + route.action, + ); + if (instrumented) { + updates.action = instrumented; + } + } + } + return updates; +} diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index 2c3e379345..afd94733b3 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -9,6 +9,8 @@ import { parsePath, warning, } from "./history"; +import type { unstable_InstrumentRouteFunction } from "./instrumentation"; +import { getInstrumentationUpdates } from "./instrumentation"; import type { AgnosticDataRouteMatch, AgnosticDataRouteObject, @@ -40,7 +42,6 @@ import type { ActionFunction, MiddlewareFunction, MiddlewareNextFunction, - ErrorResponse, } from "./utils"; import { ErrorResponseImpl, @@ -58,6 +59,7 @@ import { resolveTo, stripBasename, RouterContextProvider, + getRoutePattern, } from "./utils"; //////////////////////////////////////////////////////////////////////////////// @@ -403,6 +405,7 @@ export interface RouterInit { history: History; basename?: string; getContext?: () => MaybePromise; + unstable_instrumentRoute?: unstable_InstrumentRouteFunction; mapRouteProperties?: MapRoutePropertiesFunction; future?: Partial; hydrationRouteProperties?: string[]; @@ -866,7 +869,22 @@ export function createRouter(init: RouterInit): Router { ); let hydrationRouteProperties = init.hydrationRouteProperties || []; - let mapRouteProperties = init.mapRouteProperties || defaultMapRouteProperties; + let _mapRouteProperties = + init.mapRouteProperties || defaultMapRouteProperties; + let mapRouteProperties = _mapRouteProperties; + + // Leverage the existing mapRouteProperties logic to execute instrumentRoute + // (if it exists) on all routes in the application + if (init.unstable_instrumentRoute) { + let instrument = init.unstable_instrumentRoute; + + mapRouteProperties = (route: AgnosticDataRouteObject) => { + return { + ..._mapRouteProperties(route), + ...getInstrumentationUpdates(instrument, route), + }; + }; + } // Routes keyed by ID let manifest: RouteManifest = {}; @@ -3518,6 +3536,9 @@ export function createRouter(init: RouterInit): Router { export interface CreateStaticHandlerOptions { basename?: string; mapRouteProperties?: MapRoutePropertiesFunction; + unstable_instrumentRoute?: ( + r: AgnosticDataRouteObject, + ) => AgnosticDataRouteObject; future?: {}; } @@ -3532,8 +3553,24 @@ export function createStaticHandler( let manifest: RouteManifest = {}; let basename = (opts ? opts.basename : null) || "/"; - let mapRouteProperties = + let _mapRouteProperties = opts?.mapRouteProperties || defaultMapRouteProperties; + let mapRouteProperties = _mapRouteProperties; + + // Leverage the existing mapRouteProperties logic to execute instrumentRoute + // (if it exists) on all routes in the application + if (opts?.unstable_instrumentRoute) { + let instrument = opts?.unstable_instrumentRoute; + // TODO: Clean up these types + mapRouteProperties = (r: AgnosticDataRouteObject) => { + return instrument({ + ...r, + ..._mapRouteProperties(r), + } as AgnosticDataRouteObject) as AgnosticDataRouteObject & { + hasErrorBoundary: boolean; + }; + }; + } let dataRoutes = convertRoutesToDataRoutes( routes, @@ -3660,6 +3697,7 @@ export function createStaticHandler( let response = await runServerMiddlewarePipeline( { request, + pattern: getRoutePattern(matches.map((m) => m.route.path)), matches, params: matches[0].params, // If we're calling middleware then it must be enabled so we can cast @@ -3891,6 +3929,7 @@ export function createStaticHandler( let response = await runServerMiddlewarePipeline( { request, + pattern: getRoutePattern(matches.map((m) => m.route.path)), matches, params: matches[0].params, // If we're calling middleware then it must be enabled so we can cast @@ -4289,6 +4328,7 @@ export function createStaticHandler( mapRouteProperties, manifest, request, + getRoutePattern(matches.map((m) => m.route.path)), match, [], requestContext, @@ -4300,6 +4340,7 @@ export function createStaticHandler( mapRouteProperties, manifest, request, + getRoutePattern(matches.map((m) => m.route.path)), match, [], requestContext, @@ -4787,6 +4828,7 @@ function getMatchesToLoad( mapRouteProperties, manifest, request, + getRoutePattern(matches.map((m) => m.route.path)), match, lazyRoutePropertiesToSkip, scopedContext, @@ -4816,6 +4858,7 @@ function getMatchesToLoad( mapRouteProperties, manifest, request, + getRoutePattern(matches.map((m) => m.route.path)), match, lazyRoutePropertiesToSkip, scopedContext, @@ -5583,7 +5626,12 @@ async function runMiddlewarePipeline( ) as [string, MiddlewareFunction][]; let result = await callRouteMiddleware( - { request, params, context }, + { + request, + params, + context, + pattern: getRoutePattern(matches.map((m) => m.route.path)), + }, tuples, handler, processResult, @@ -5705,6 +5753,7 @@ function getDataStrategyMatch( mapRouteProperties: MapRoutePropertiesFunction, manifest: RouteManifest, request: Request, + pattern: string, match: DataRouteMatch, lazyRoutePropertiesToSkip: string[], scopedContext: unknown, @@ -5753,6 +5802,7 @@ function getDataStrategyMatch( ) { return callLoaderOrAction({ request, + pattern, match, lazyHandlerPromise: _lazyPromises?.handler, lazyRoutePromise: _lazyPromises?.route, @@ -5799,6 +5849,7 @@ function getTargetedDataStrategyMatches( mapRouteProperties, manifest, request, + getRoutePattern(matches.map((m) => m.route.path)), match, lazyRoutePropertiesToSkip, scopedContext, @@ -5826,6 +5877,7 @@ async function callDataStrategyImpl( // back out below. let dataStrategyArgs = { request, + pattern: getRoutePattern(matches.map((m) => m.route.path)), params: matches[0].params, context: scopedContext, matches, @@ -5883,6 +5935,7 @@ async function callDataStrategyImpl( // Default logic for calling a loader/action is the user has no specified a dataStrategy async function callLoaderOrAction({ request, + pattern, match, lazyHandlerPromise, lazyRoutePromise, @@ -5890,6 +5943,7 @@ async function callLoaderOrAction({ scopedContext, }: { request: Request; + pattern: string; match: AgnosticDataRouteMatch; lazyHandlerPromise: Promise | undefined; lazyRoutePromise: Promise | undefined; @@ -5923,6 +5977,7 @@ async function callLoaderOrAction({ return handler( { request, + pattern, params: match.params, context: scopedContext, }, diff --git a/packages/react-router/lib/router/utils.ts b/packages/react-router/lib/router/utils.ts index 14e5e40c51..4ba9871a0e 100644 --- a/packages/react-router/lib/router/utils.ts +++ b/packages/react-router/lib/router/utils.ts @@ -269,6 +269,11 @@ type DefaultContext = MiddlewareEnabled extends true interface DataFunctionArgs { /** A {@link https://developer.mozilla.org/en-US/docs/Web/API/Request Fetch Request instance} which you can use to read headers (like cookies, and {@link https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams URLSearchParams} from the request. */ request: Request; + /** + * Matched un-interpolated route pattern for the current path (i.e., /blog/:slug). + * Mostly useful as a identifier to aggregate on for logging/tracing/etc. + */ + pattern: string; /** * {@link https://reactrouter.com/start/framework/routing#dynamic-segments Dynamic route params} for the current route. * @example @@ -535,7 +540,7 @@ export type AgnosticPatchRoutesOnNavigationFunction< * properties from framework-agnostic properties */ export interface MapRoutePropertiesFunction { - (route: AgnosticRouteObject): { + (route: AgnosticDataRouteObject): { hasErrorBoundary: boolean; } & Record; } @@ -808,18 +813,18 @@ export function convertRoutesToDataRoutes( if (isIndexRoute(route)) { let indexRoute: AgnosticDataIndexRouteObject = { ...route, - ...mapRouteProperties(route), id, }; + Object.assign(indexRoute, mapRouteProperties(indexRoute)); manifest[id] = indexRoute; return indexRoute; } else { let pathOrLayoutRoute: AgnosticDataNonIndexRouteObject = { ...route, - ...mapRouteProperties(route), id, children: undefined, }; + Object.assign(pathOrLayoutRoute, mapRouteProperties(pathOrLayoutRoute)); manifest[id] = pathOrLayoutRoute; if (route.children) { @@ -1995,3 +2000,7 @@ export function isRouteErrorResponse(error: any): error is ErrorResponse { "data" in error ); } + +export function getRoutePattern(paths: (string | undefined)[]) { + return paths.filter(Boolean).join("/").replace(/\/\/*/g, "/"); +} diff --git a/packages/react-router/lib/server-runtime/build.ts b/packages/react-router/lib/server-runtime/build.ts index 25462b1be9..3ca14141aa 100644 --- a/packages/react-router/lib/server-runtime/build.ts +++ b/packages/react-router/lib/server-runtime/build.ts @@ -1,5 +1,6 @@ import type { ActionFunctionArgs, + AgnosticDataRouteObject, LoaderFunctionArgs, RouterContextProvider, } from "../router/utils"; @@ -12,6 +13,7 @@ import type { import type { ServerRouteManifest } from "./routes"; import type { AppLoadContext } from "./data"; import type { MiddlewareEnabled } from "../types/future"; +import type { RequestHandler } from "./server"; type OptionalCriticalCss = CriticalCss | undefined; @@ -58,12 +60,23 @@ export interface HandleDocumentRequestFunction { export interface HandleDataRequestFunction { ( response: Response, - args: LoaderFunctionArgs | ActionFunctionArgs, + args: { + request: LoaderFunctionArgs["request"] | ActionFunctionArgs["request"]; + context: LoaderFunctionArgs["context"] | ActionFunctionArgs["context"]; + params: LoaderFunctionArgs["params"] | ActionFunctionArgs["params"]; + }, ): Promise | Response; } export interface HandleErrorFunction { - (error: unknown, args: LoaderFunctionArgs | ActionFunctionArgs): void; + ( + error: unknown, + args: { + request: LoaderFunctionArgs["request"] | ActionFunctionArgs["request"]; + context: LoaderFunctionArgs["context"] | ActionFunctionArgs["context"]; + params: LoaderFunctionArgs["params"] | ActionFunctionArgs["params"]; + }, + ): void; } /** @@ -74,5 +87,9 @@ export interface ServerEntryModule { default: HandleDocumentRequestFunction; handleDataRequest?: HandleDataRequestFunction; handleError?: HandleErrorFunction; + unstable_instrumentHandler?: (handler: RequestHandler) => RequestHandler; + unstable_instrumentRoute?: ( + route: AgnosticDataRouteObject, + ) => AgnosticDataRouteObject; streamTimeout?: number; } diff --git a/packages/react-router/lib/server-runtime/data.ts b/packages/react-router/lib/server-runtime/data.ts index a1b67be014..17df22d468 100644 --- a/packages/react-router/lib/server-runtime/data.ts +++ b/packages/react-router/lib/server-runtime/data.ts @@ -26,6 +26,7 @@ export async function callRouteHandler( request: stripRoutesParam(stripIndexParam(args.request)), params: args.params, context: args.context, + pattern: args.pattern, }); // If they returned a redirect via data(), re-throw it as a Response diff --git a/packages/react-router/lib/server-runtime/server.ts b/packages/react-router/lib/server-runtime/server.ts index d3b78ae510..c6192ee375 100644 --- a/packages/react-router/lib/server-runtime/server.ts +++ b/packages/react-router/lib/server-runtime/server.ts @@ -55,6 +55,7 @@ function derive(build: ServerBuild, mode?: string) { let serverMode = isServerMode(mode) ? mode : ServerMode.Production; let staticHandler = createStaticHandler(dataRoutes, { basename: build.basename, + unstable_instrumentRoute: build.entry.module.unstable_instrumentRoute, }); let errorHandler = @@ -67,42 +68,8 @@ function derive(build: ServerBuild, mode?: string) { ); } }); - return { - routes, - dataRoutes, - serverMode, - staticHandler, - errorHandler, - }; -} - -export const createRequestHandler: CreateRequestHandlerFunction = ( - build, - mode, -) => { - let _build: ServerBuild; - let routes: ServerRoute[]; - let serverMode: ServerMode; - let staticHandler: StaticHandler; - let errorHandler: HandleErrorFunction; - - return async function requestHandler(request, initialContext) { - _build = typeof build === "function" ? await build() : build; - - if (typeof build === "function") { - let derived = derive(_build, mode); - routes = derived.routes; - serverMode = derived.serverMode; - staticHandler = derived.staticHandler; - errorHandler = derived.errorHandler; - } else if (!routes || !serverMode || !staticHandler || !errorHandler) { - let derived = derive(_build, mode); - routes = derived.routes; - serverMode = derived.serverMode; - staticHandler = derived.staticHandler; - errorHandler = derived.errorHandler; - } + let requestHandler: RequestHandler = async (request, initialContext) => { let params: RouteMatch["params"] = {}; let loadContext: AppLoadContext | RouterContextProvider; @@ -118,7 +85,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( }); }; - if (_build.future.v8_middleware) { + if (build.future.v8_middleware) { if ( initialContext && !(initialContext instanceof RouterContextProvider) @@ -138,7 +105,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( let url = new URL(request.url); - let normalizedBasename = _build.basename || "/"; + let normalizedBasename = build.basename || "/"; let normalizedPath = url.pathname; if (stripBasename(normalizedPath, normalizedBasename) === "/_root.data") { normalizedPath = normalizedBasename; @@ -158,7 +125,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( // When runtime SSR is disabled, make our dev server behave like the deployed // pre-rendered site would - if (!_build.ssr) { + if (!build.ssr) { // Decode the URL path before checking against the prerender config let decodedPath = decodeURI(normalizedPath); @@ -188,12 +155,12 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( // When SSR is disabled this, file can only ever run during dev because we // delete the server build at the end of the build - if (_build.prerender.length === 0) { + if (build.prerender.length === 0) { // ssr:false and no prerender config indicates "SPA Mode" isSpaMode = true; } else if ( - !_build.prerender.includes(decodedPath) && - !_build.prerender.includes(decodedPath + "/") + !build.prerender.includes(decodedPath) && + !build.prerender.includes(decodedPath + "/") ) { if (url.pathname.endsWith(".data")) { // 404 on non-pre-rendered `.data` requests @@ -222,12 +189,12 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( // Manifest request for fog of war let manifestUrl = getManifestPath( - _build.routeDiscovery.manifestPath, + build.routeDiscovery.manifestPath, normalizedBasename, ); if (url.pathname === manifestUrl) { try { - let res = await handleManifestRequest(_build, routes, url); + let res = await handleManifestRequest(build, routes, url); return res; } catch (e) { handleError(e); @@ -235,7 +202,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( } } - let matches = matchServerRoutes(routes, normalizedPath, _build.basename); + let matches = matchServerRoutes(routes, normalizedPath, build.basename); if (matches && matches.length > 0) { Object.assign(params, matches[0].params); } @@ -248,12 +215,12 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( let singleFetchMatches = matchServerRoutes( routes, handlerUrl.pathname, - _build.basename, + build.basename, ); response = await handleSingleFetchRequest( serverMode, - _build, + build, staticHandler, request, handlerUrl, @@ -265,13 +232,13 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( response = generateSingleFetchRedirectResponse( response, request, - _build, + build, serverMode, ); } - if (_build.entry.module.handleDataRequest) { - response = await _build.entry.module.handleDataRequest(response, { + if (build.entry.module.handleDataRequest) { + response = await build.entry.module.handleDataRequest(response, { context: loadContext, params: singleFetchMatches ? singleFetchMatches[0].params : {}, request, @@ -281,7 +248,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( response = generateSingleFetchRedirectResponse( response, request, - _build, + build, serverMode, ); } @@ -294,7 +261,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( ) { response = await handleResourceRequest( serverMode, - _build, + build, staticHandler, matches.slice(-1)[0].route.id, request, @@ -305,8 +272,8 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( let { pathname } = url; let criticalCss: CriticalCss | undefined = undefined; - if (_build.unstable_getCriticalCss) { - criticalCss = await _build.unstable_getCriticalCss({ pathname }); + if (build.unstable_getCriticalCss) { + criticalCss = await build.unstable_getCriticalCss({ pathname }); } else if ( mode === ServerMode.Development && getDevServerHooks()?.getCriticalCss @@ -316,7 +283,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( response = await handleDocumentRequest( serverMode, - _build, + build, staticHandler, request, loadContext, @@ -336,6 +303,60 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( return response; }; + + if (build.entry.module.unstable_instrumentHandler) { + requestHandler = + build.entry.module.unstable_instrumentHandler(requestHandler); + } + + return { + routes, + dataRoutes, + serverMode, + staticHandler, + errorHandler, + requestHandler, + }; +} + +export const createRequestHandler: CreateRequestHandlerFunction = ( + build, + mode, +) => { + let _build: ServerBuild; + let routes: ServerRoute[]; + let serverMode: ServerMode; + let staticHandler: StaticHandler; + let errorHandler: HandleErrorFunction; + let _requestHandler: RequestHandler; + + return async function requestHandler(request, initialContext) { + _build = typeof build === "function" ? await build() : build; + + if (typeof build === "function") { + let derived = derive(_build, mode); + routes = derived.routes; + serverMode = derived.serverMode; + staticHandler = derived.staticHandler; + errorHandler = derived.errorHandler; + _requestHandler = derived.requestHandler; + } else if ( + !routes || + !serverMode || + !staticHandler || + !errorHandler || + !_requestHandler + ) { + let derived = derive(_build, mode); + routes = derived.routes; + serverMode = derived.serverMode; + staticHandler = derived.staticHandler; + errorHandler = derived.errorHandler; + _requestHandler = derived.requestHandler; + } + + return _requestHandler(request, initialContext); + }; }; async function handleManifestRequest( diff --git a/packages/react-router/lib/types/route-data.ts b/packages/react-router/lib/types/route-data.ts index 058a2f5aef..9efebafa8e 100644 --- a/packages/react-router/lib/types/route-data.ts +++ b/packages/react-router/lib/types/route-data.ts @@ -92,6 +92,11 @@ export type ClientDataFunctionArgs = { * } **/ params: Params; + /** + * Matched un-interpolated route pattern for the current path (i.e., /blog/:slug). + * Mostly useful as a identifier to aggregate on for logging/tracing/etc. + */ + pattern: string; /** * When `future.v8_middleware` is not enabled, this is undefined. * @@ -121,6 +126,11 @@ export type ServerDataFunctionArgs = { * } **/ params: Params; + /** + * Matched un-interpolated route pattern for the current path (i.e., /blog/:slug). + * Mostly useful as a identifier to aggregate on for logging/tracing/etc. + */ + pattern: string; /** * Without `future.v8_middleware` enabled, this is the context passed in * to your server adapter's `getLoadContext` function. It's a way to bridge the diff --git a/playground/observability/.gitignore b/playground/observability/.gitignore new file mode 100644 index 0000000000..752e5fe866 --- /dev/null +++ b/playground/observability/.gitignore @@ -0,0 +1,6 @@ +node_modules + +/build +.env + +.react-router/ diff --git a/playground/observability/app/entry.client.tsx b/playground/observability/app/entry.client.tsx new file mode 100644 index 0000000000..c778e9fcb6 --- /dev/null +++ b/playground/observability/app/entry.client.tsx @@ -0,0 +1,99 @@ +import { startTransition, StrictMode } from "react"; +import { hydrateRoot } from "react-dom/client"; +import type { + DataRouteObject, + DataRouter, + RouterNavigateOptions, +} from "react-router"; +import { HydratedRouter } from "react-router/dom"; +import { getPattern, measure, startMeasure } from "./o11y"; + +startTransition(() => { + hydrateRoot( + document, + + + , + ); +}); + +function instrumentRouter(router: DataRouter) { + let initialize = router.initialize; + router.initialize = () => { + let pattern = getPattern(router.routes, router.state.location.pathname); + let end = startMeasure(["initialize", pattern]); + + if (router.state.initialized) { + end(); + } else { + let unsubscribe = router.subscribe((state) => { + if (state.initialized) { + end(); + unsubscribe(); + } + }); + } + return initialize(); + }; + + let navigate = router.navigate; + router.navigate = async (to, opts?: RouterNavigateOptions) => { + let path = + typeof to === "string" + ? to + : typeof to === "number" + ? String(to) + : (to?.pathname ?? "unknown"); + await measure([`navigate`, getPattern(router.routes, path)], () => + typeof to === "number" ? navigate(to) : navigate(to, opts), + ); + }; + + return router; +} + +function instrumentRoute(route: DataRouteObject): DataRouteObject { + if (typeof route.lazy === "function") { + let lazy = route.lazy; + route.lazy = () => measure(["lazy", route.id], () => lazy()); + } + + if ( + route.middleware && + route.middleware.length > 0 && + // @ts-expect-error + route.middleware.instrumented !== true + ) { + route.middleware = route.middleware.map((mw, i) => { + return ({ request, params, pattern, context }, next) => + measure(["middleware", route.id, i.toString(), pattern], async () => + mw({ request, params, pattern, context }, next), + ); + }); + // When `route.lazy` is used alongside a statically defined `loader`, make + // sure we don't double-instrument the `loader` after `route.lazy` completes + // and we re-call `instrumentRoute` via `mapRouteProperties` + // @ts-expect-error + route.middleware.instrumented = true; + } + + // @ts-expect-error + if (typeof route.loader === "function" && !route.loader.instrumented) { + let loader = route.loader; + route.loader = (...args) => { + return measure([`loader:${route.id}`, args[0].pattern], async () => + loader(...args), + ); + }; + // When `route.lazy` is used alongside a statically defined `loader`, make + // sure we don't double-instrument the `loader` after `route.lazy` completes + // and we re-call `instrumentRoute` via `mapRouteProperties` + // @ts-expect-error + route.loader.instrumented = true; + } + + return route; +} diff --git a/playground/observability/app/entry.server.tsx b/playground/observability/app/entry.server.tsx new file mode 100644 index 0000000000..f40c00afe1 --- /dev/null +++ b/playground/observability/app/entry.server.tsx @@ -0,0 +1,119 @@ +import { PassThrough } from "node:stream"; + +import type { AppLoadContext, EntryContext } from "react-router"; +import { createReadableStreamFromReadable } from "@react-router/node"; +import { ServerRouter } from "react-router"; +import { isbot } from "isbot"; +import type { RenderToPipeableStreamOptions } from "react-dom/server"; +import { renderToPipeableStream } from "react-dom/server"; +import type { RequestHandler } from "react-router"; +import { log } from "./o11y"; +import type { ServerBuild } from "react-router"; +import type { DataRouteObject } from "react-router"; +import type { MiddlewareFunction } from "react-router"; + +export const streamTimeout = 5_000; + +export function unstable_instrumentHandler( + handler: RequestHandler, +): RequestHandler { + let instrumented: RequestHandler = async (request, context) => { + let pattern = new URL(request.url).pathname; + return await log([`request`, pattern], () => handler(request, context)); + }; + return instrumented; +} + +export function unstable_instrumentRoute( + route: DataRouteObject, +): DataRouteObject { + if (route.middleware && route.middleware.length > 0) { + route.middleware = route.middleware.map((mw, i) => { + return (...args: Parameters>) => + log(["middleware", route.id, i.toString(), args[0].pattern], async () => + mw(...args), + ); + }) as MiddlewareFunction[]; + } + + if (typeof route.loader === "function") { + let loader = route.loader; + route.loader = (...args) => { + return log([`loader:${route.id}`, args[0].pattern], async () => + loader(...args), + ); + }; + } + + return route; +} + +export default function handleRequest( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + routerContext: EntryContext, + loadContext: AppLoadContext, + // If you have middleware enabled: + // loadContext: RouterContextProvider +) { + return new Promise((resolve, reject) => { + let shellRendered = false; + let userAgent = request.headers.get("user-agent"); + + // Ensure requests from bots and SPA Mode renders wait for all content to load before responding + // https://react.dev/reference/react-dom/server/renderToPipeableStream#waiting-for-all-content-to-load-for-crawlers-and-static-generation + let readyOption: keyof RenderToPipeableStreamOptions = + (userAgent && isbot(userAgent)) || routerContext.isSpaMode + ? "onAllReady" + : "onShellReady"; + + // Abort the rendering stream after the `streamTimeout` so it has time to + // flush down the rejected boundaries + let timeoutId: ReturnType | undefined = setTimeout( + () => abort(), + streamTimeout + 1000, + ); + + const { pipe, abort } = renderToPipeableStream( + , + { + [readyOption]() { + shellRendered = true; + const body = new PassThrough({ + final(callback) { + // Clear the timeout to prevent retaining the closure and memory leak + clearTimeout(timeoutId); + timeoutId = undefined; + callback(); + }, + }); + const stream = createReadableStreamFromReadable(body); + + responseHeaders.set("Content-Type", "text/html"); + + pipe(body); + + resolve( + new Response(stream, { + headers: responseHeaders, + status: responseStatusCode, + }), + ); + }, + onShellError(error: unknown) { + reject(error); + }, + onError(error: unknown) { + responseStatusCode = 500; + // Log streaming rendering errors from inside the shell. Don't log + // errors encountered during initial shell rendering since they'll + // reject and get logged in handleDocumentRequest. + if (shellRendered) { + console.error(error); + } + }, + }, + ); + }); +} diff --git a/playground/observability/app/o11y.ts b/playground/observability/app/o11y.ts new file mode 100644 index 0000000000..83038496fa --- /dev/null +++ b/playground/observability/app/o11y.ts @@ -0,0 +1,49 @@ +import { matchRoutes, type DataRouteObject } from "react-router"; + +export function getPattern(routes: DataRouteObject[], path: string) { + let matches = matchRoutes(routes, path); + if (matches && matches.length > 0) { + return matches + ?.map((m) => m.route.path) + .filter(Boolean) + .join("/") + .replace(/\/\/+/g, "/"); + } + return "unknown-pattern"; +} + +export function startMeasure(label: string[]) { + let strLabel = label.join("--"); + let now = Date.now().toString(); + let start = `start:${strLabel}:${now}`; + performance.mark(start); + return () => { + let end = `end:${strLabel}:${now}`; + performance.mark(end); + performance.measure(strLabel, start, end); + }; +} + +export async function measure( + label: string[], + cb: () => Promise, +): Promise { + let end = startMeasure(label); + try { + return await cb(); + } finally { + end(); + } +} + +export async function log( + label: string[], + cb: () => Promise, +): Promise { + console.log(new Date().toISOString(), "start", label.join("--")); + try { + return await cb(); + } finally { + console.log(new Date().toISOString(), "end", label.join("--")); + } +} diff --git a/playground/observability/app/root.tsx b/playground/observability/app/root.tsx new file mode 100644 index 0000000000..52a3684e85 --- /dev/null +++ b/playground/observability/app/root.tsx @@ -0,0 +1,56 @@ +import { + Link, + Links, + Meta, + Outlet, + Scripts, + ScrollRestoration, + type MiddlewareFunction, +} from "react-router"; + +let sleep = (ms: number = Math.max(100, Math.round(Math.random() * 500))) => + new Promise((r) => setTimeout(r, ms)); + +export const middleware = [ + async (_: unknown, next: Parameters>[1]) => { + await sleep(); + await next(); + await sleep(); + }, +]; + +export async function loader() { + await sleep(); +} + +export function Layout({ children }: { children: React.ReactNode }) { + return ( + + + + + + + + + + {children} + + + + + ); +} + +export default function App() { + return ; +} diff --git a/playground/observability/app/routes.ts b/playground/observability/app/routes.ts new file mode 100644 index 0000000000..3d0c769294 --- /dev/null +++ b/playground/observability/app/routes.ts @@ -0,0 +1,6 @@ +import { type RouteConfig, index, route } from "@react-router/dev/routes"; + +export default [ + index("routes/index.tsx"), + route(":slug", "routes/slug.tsx"), +] satisfies RouteConfig; diff --git a/playground/observability/app/routes/index.tsx b/playground/observability/app/routes/index.tsx new file mode 100644 index 0000000000..7e934a0a6c --- /dev/null +++ b/playground/observability/app/routes/index.tsx @@ -0,0 +1,24 @@ +import type { MiddlewareFunction } from "react-router"; + +let sleep = (ms: number = Math.max(100, Math.round(Math.random() * 500))) => + new Promise((r) => setTimeout(r, ms)); + +export const middleware = [ + async (_: unknown, next: Parameters>[1]) => { + await sleep(); + await next(); + await sleep(); + }, +]; + +export async function loader() { + await sleep(); +} + +export default function Index() { + return ( +
+

Welcome to React Router

+
+ ); +} diff --git a/playground/observability/app/routes/slug.tsx b/playground/observability/app/routes/slug.tsx new file mode 100644 index 0000000000..ce58440893 --- /dev/null +++ b/playground/observability/app/routes/slug.tsx @@ -0,0 +1,46 @@ +import { startMeasure } from "~/o11y"; +import { type Route } from "../../.react-router/types/app/routes/+types/slug"; + +let sleep = (ms: number = Math.max(100, Math.round(Math.random() * 500))) => + new Promise((r) => setTimeout(r, ms)); + +export const middleware: Route.MiddlewareFunction[] = [ + async (_, next) => { + await sleep(); + await next(); + await sleep(); + }, +]; + +export const clientMiddleware: Route.ClientMiddlewareFunction[] = [ + async (_, next) => { + await sleep(); + await next(); + await sleep(); + }, +]; + +export async function loader({ params }: Route.LoaderArgs) { + await sleep(); + return params.slug; +} + +export async function clientLoader({ + serverLoader, + pattern, +}: Route.ClientLoaderArgs) { + await sleep(); + let end = startMeasure(["serverLoader", pattern]); + let value = await serverLoader(); + end(); + await sleep(); + return value; +} + +export default function Slug({ loaderData }: Route.ComponentProps) { + return ( +
+

Slug: {loaderData}

+
+ ); +} diff --git a/playground/observability/package.json b/playground/observability/package.json new file mode 100644 index 0000000000..c0288d54a4 --- /dev/null +++ b/playground/observability/package.json @@ -0,0 +1,39 @@ +{ + "name": "@playground/framework-express", + "version": "0.0.0", + "private": true, + "sideEffects": false, + "type": "module", + "scripts": { + "build": "react-router build", + "dev": "node ./server.js", + "start": "cross-env NODE_ENV=production node ./server.js", + "typecheck": "react-router typegen && tsc" + }, + "dependencies": { + "@react-router/express": "workspace:*", + "@react-router/node": "workspace:*", + "compression": "^1.7.4", + "express": "^4.19.2", + "isbot": "^5.1.11", + "morgan": "^1.10.0", + "react": "^19.1.0", + "react-dom": "^19.1.0", + "react-router": "workspace:*" + }, + "devDependencies": { + "@react-router/dev": "workspace:*", + "@types/compression": "^1.7.5", + "@types/express": "^4.17.20", + "@types/morgan": "^1.9.9", + "@types/react": "^18.2.20", + "@types/react-dom": "^18.2.7", + "cross-env": "^7.0.3", + "typescript": "^5.1.6", + "vite": "^6.1.0", + "vite-tsconfig-paths": "^4.2.1" + }, + "engines": { + "node": ">=20.0.0" + } +} diff --git a/playground/observability/public/favicon.ico b/playground/observability/public/favicon.ico new file mode 100644 index 0000000000..5dbdfcddcb Binary files /dev/null and b/playground/observability/public/favicon.ico differ diff --git a/playground/observability/react-router.config.ts b/playground/observability/react-router.config.ts new file mode 100644 index 0000000000..039108a6a7 --- /dev/null +++ b/playground/observability/react-router.config.ts @@ -0,0 +1,7 @@ +import type { Config } from "@react-router/dev/config"; + +export default { + future: { + v8_middleware: true, + }, +} satisfies Config; diff --git a/playground/observability/server.js b/playground/observability/server.js new file mode 100644 index 0000000000..fa5048f32c --- /dev/null +++ b/playground/observability/server.js @@ -0,0 +1,43 @@ +import { createRequestHandler } from "@react-router/express"; +import compression from "compression"; +import express from "express"; +import morgan from "morgan"; + +const viteDevServer = + process.env.NODE_ENV === "production" + ? undefined + : await import("vite").then((vite) => + vite.createServer({ + server: { middlewareMode: true }, + }) + ); + +const reactRouterHandler = createRequestHandler({ + build: viteDevServer + ? () => viteDevServer.ssrLoadModule("virtual:react-router/server-build") + : await import("./build/server/index.js"), +}); + +const app = express(); + +app.use(compression()); +app.disable("x-powered-by"); + +if (viteDevServer) { + app.use(viteDevServer.middlewares); +} else { + app.use( + "/assets", + express.static("build/client/assets", { immutable: true, maxAge: "1y" }) + ); +} + +app.use(express.static("build/client", { maxAge: "1h" })); +app.use(morgan("tiny")); + +app.all("*", reactRouterHandler); + +const port = process.env.PORT || 3000; +app.listen(port, () => + console.log(`Express server listening at http://localhost:${port}`) +); diff --git a/playground/observability/tsconfig.json b/playground/observability/tsconfig.json new file mode 100644 index 0000000000..79cf7b5af6 --- /dev/null +++ b/playground/observability/tsconfig.json @@ -0,0 +1,31 @@ +{ + "include": [ + "**/*.ts", + "**/*.tsx", + "**/.server/**/*.ts", + "**/.server/**/*.tsx", + "**/.client/**/*.ts", + "**/.client/**/*.tsx", + "./.react-router/types/**/*" + ], + "compilerOptions": { + "lib": ["DOM", "DOM.Iterable", "ES2022"], + "types": ["@react-router/node", "vite/client"], + "verbatimModuleSyntax": true, + "esModuleInterop": true, + "jsx": "react-jsx", + "module": "ESNext", + "moduleResolution": "Bundler", + "resolveJsonModule": true, + "target": "ES2022", + "strict": true, + "allowJs": true, + "skipLibCheck": true, + "baseUrl": ".", + "paths": { + "~/*": ["./app/*"] + }, + "noEmit": true, + "rootDirs": [".", "./.react-router/types"] + } +} diff --git a/playground/observability/vite.config.ts b/playground/observability/vite.config.ts new file mode 100644 index 0000000000..f910ad4c18 --- /dev/null +++ b/playground/observability/vite.config.ts @@ -0,0 +1,7 @@ +import { reactRouter } from "@react-router/dev/vite"; +import { defineConfig } from "vite"; +import tsconfigPaths from "vite-tsconfig-paths"; + +export default defineConfig({ + plugins: [reactRouter(), tsconfigPaths()], +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 182ceea34b..18a86c76fd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1688,6 +1688,67 @@ importers: specifier: ^4.2.1 version: 4.3.2(typescript@5.4.5)(vite@6.2.5(@types/node@20.11.30)(jiti@2.4.2)(lightningcss@1.30.1)(tsx@4.19.3)(yaml@2.8.0)) + playground/observability: + dependencies: + '@react-router/express': + specifier: workspace:* + version: link:../../packages/react-router-express + '@react-router/node': + specifier: workspace:* + version: link:../../packages/react-router-node + compression: + specifier: ^1.7.4 + version: 1.8.0 + express: + specifier: ^4.19.2 + version: 4.21.2 + isbot: + specifier: ^5.1.11 + version: 5.1.11 + morgan: + specifier: ^1.10.0 + version: 1.10.0 + react: + specifier: ^19.1.0 + version: 19.1.0 + react-dom: + specifier: ^19.1.0 + version: 19.1.0(react@19.1.0) + react-router: + specifier: workspace:* + version: link:../../packages/react-router + devDependencies: + '@react-router/dev': + specifier: workspace:* + version: link:../../packages/react-router-dev + '@types/compression': + specifier: ^1.7.5 + version: 1.7.5 + '@types/express': + specifier: ^4.17.20 + version: 4.17.21 + '@types/morgan': + specifier: ^1.9.9 + version: 1.9.9 + '@types/react': + specifier: ^18.2.18 + version: 18.2.18 + '@types/react-dom': + specifier: ^18.2.7 + version: 18.2.7 + cross-env: + specifier: ^7.0.3 + version: 7.0.3 + typescript: + specifier: ^5.1.6 + version: 5.4.5 + vite: + specifier: ^6.1.0 + version: 6.2.5(@types/node@22.14.0)(jiti@2.4.2)(lightningcss@1.30.1)(tsx@4.19.3)(yaml@2.8.0) + vite-tsconfig-paths: + specifier: ^4.2.1 + version: 4.3.2(typescript@5.4.5)(vite@6.2.5(@types/node@22.14.0)(jiti@2.4.2)(lightningcss@1.30.1)(tsx@4.19.3)(yaml@2.8.0)) + playground/rsc-parcel: dependencies: '@mjackson/node-fetch-server':