diff --git a/packages/react/package.json b/packages/react/package.json index 508758dd34bc..32b9b8d33dfc 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -31,6 +31,7 @@ "@types/history-4": "npm:@types/history@4.7.8", "@types/history-5": "npm:@types/history@4.7.8", "@types/hoist-non-react-statics": "^3.3.1", + "@types/node-fetch": "^2.6.0", "@types/react": "^17.0.3", "@types/react-router-3": "npm:@types/react-router@3.0.24", "@types/react-router-4": "npm:@types/react-router@5.1.14", @@ -39,12 +40,14 @@ "eslint-plugin-react-hooks": "^4.0.8", "history-4": "npm:history@4.6.0", "history-5": "npm:history@4.9.0", + "node-fetch": "^2.6.0", "react": "^18.0.0", "react-dom": "^18.0.0", "react-router-3": "npm:react-router@3.2.0", "react-router-4": "npm:react-router@4.1.0", "react-router-5": "npm:react-router@5.0.0", "react-router-6": "npm:react-router@6.3.0", + "react-router-6.4": "npm:react-router@6.4.2", "redux": "^4.0.5" }, "scripts": { diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index 976929e1ca4b..ad66d1e77801 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -7,4 +7,9 @@ export { ErrorBoundary, withErrorBoundary } from './errorboundary'; export { createReduxEnhancer } from './redux'; export { reactRouterV3Instrumentation } from './reactrouterv3'; export { reactRouterV4Instrumentation, reactRouterV5Instrumentation, withSentryRouting } from './reactrouter'; -export { reactRouterV6Instrumentation, withSentryReactRouterV6Routing, wrapUseRoutes } from './reactrouterv6'; +export { + reactRouterV6Instrumentation, + withSentryReactRouterV6Routing, + wrapUseRoutes, + wrapCreateBrowserRouter, +} from './reactrouterv6'; diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index 6ff7d09b1278..39bb473b6443 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -7,68 +7,22 @@ import { getNumberOfUrlSegments, logger } from '@sentry/utils'; import hoistNonReactStatics from 'hoist-non-react-statics'; import React from 'react'; -import { Action, Location } from './types'; - -interface NonIndexRouteObject { - caseSensitive?: boolean; - children?: RouteObject[]; - element?: React.ReactNode | null; - index?: false; - path?: string; -} - -interface IndexRouteObject { - caseSensitive?: boolean; - children?: undefined; - element?: React.ReactNode | null; - index?: true; - path?: string; -} - -// This type was originally just `type RouteObject = IndexRouteObject`, but this was changed -// in https://github.com/remix-run/react-router/pull/9366, which was released with `6.4.2` -// See https://github.com/remix-run/react-router/issues/9427 for a discussion on this. -type RouteObject = IndexRouteObject | NonIndexRouteObject; - -type Params = { - readonly [key in Key]: string | undefined; -}; - -type UseRoutes = (routes: RouteObject[], locationArg?: Partial | string) => React.ReactElement | null; - -// https://github.com/remix-run/react-router/blob/9fa54d643134cd75a0335581a75db8100ed42828/packages/react-router/lib/router.ts#L114-L134 -interface RouteMatch { - /** - * The names and values of dynamic parameters in the URL. - */ - params: Params; - /** - * The portion of the URL pathname that was matched. - */ - pathname: string; - /** - * The portion of the URL pathname that was matched before child routes. - */ - pathnameBase: string; - /** - * The route object that was used to match. - */ - route: RouteObject; -} - -type UseEffect = (cb: () => void, deps: unknown[]) => void; -type UseLocation = () => Location; -type UseNavigationType = () => Action; - -// For both of these types, use `any` instead of `RouteObject[]` or `RouteMatch[]`. -// Have to do this so we maintain backwards compatability between -// react-router > 6.0.0 and >= 6.4.2. -// eslint-disable-next-line @typescript-eslint/no-explicit-any -type RouteObjectArrayAlias = any; -// eslint-disable-next-line @typescript-eslint/no-explicit-any -type RouteMatchAlias = any; -type CreateRoutesFromChildren = (children: JSX.Element[]) => RouteObjectArrayAlias; -type MatchRoutes = (routes: RouteObjectArrayAlias, location: Location) => RouteMatchAlias[] | null; +import { + Action, + AgnosticDataRouteMatch, + CreateRouterFunction, + CreateRoutesFromChildren, + Location, + MatchRoutes, + RouteMatch, + RouteObject, + Router, + RouterState, + UseEffect, + UseLocation, + UseNavigationType, + UseRoutes, +} from './types'; let activeTransaction: Transaction | undefined; @@ -122,14 +76,12 @@ export function reactRouterV6Instrumentation( function getNormalizedName( routes: RouteObject[], location: Location, - matchRoutes: MatchRoutes, + branches: RouteMatch[], ): [string, TransactionSource] { - if (!routes || routes.length === 0 || !matchRoutes) { + if (!routes || routes.length === 0) { return [location.pathname, 'url']; } - const branches = matchRoutes(routes, location) as unknown as RouteMatch[]; - let pathBuilder = ''; if (branches) { // eslint-disable-next-line @typescript-eslint/prefer-for-of @@ -167,9 +119,11 @@ function getNormalizedName( return [location.pathname, 'url']; } -function updatePageloadTransaction(location: Location, routes: RouteObject[]): void { - if (activeTransaction) { - activeTransaction.setName(...getNormalizedName(routes, location, _matchRoutes)); +function updatePageloadTransaction(location: Location, routes: RouteObject[], matches?: AgnosticDataRouteMatch): void { + const branches = Array.isArray(matches) ? matches : (_matchRoutes(routes, location) as unknown as RouteMatch[]); + + if (activeTransaction && branches) { + activeTransaction.setName(...getNormalizedName(routes, location, branches)); } } @@ -178,6 +132,7 @@ function handleNavigation( routes: RouteObject[], navigationType: Action, isBaseLocation: boolean, + matches?: AgnosticDataRouteMatch, ): void { if (isBaseLocation) { if (activeTransaction) { @@ -187,12 +142,14 @@ function handleNavigation( return; } - if (_startTransactionOnLocationChange && (navigationType === 'PUSH' || navigationType === 'POP')) { + const branches = Array.isArray(matches) ? matches : _matchRoutes(routes, location); + + if (_startTransactionOnLocationChange && (navigationType === 'PUSH' || navigationType === 'POP') && branches) { if (activeTransaction) { activeTransaction.finish(); } - const [name, source] = getNormalizedName(routes, location, _matchRoutes); + const [name, source] = getNormalizedName(routes, location, branches); activeTransaction = _customStartTransaction({ name, op: 'navigation', @@ -294,3 +251,32 @@ export function wrapUseRoutes(origUseRoutes: UseRoutes): UseRoutes { return ; }; } + +export function wrapCreateBrowserRouter(createRouterFunction: CreateRouterFunction): CreateRouterFunction { + // `opts` for createBrowserHistory and createMemoryHistory are different, but also not relevant for us at the moment. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (routes: RouteObject[], opts?: any): Router { + const router = createRouterFunction(routes, opts); + + // The initial load ends when `createBrowserRouter` is called. + // This is the earliest convenient time to update the transaction name. + // Callbacks to `router.subscribe` are not called for the initial load. + if (router.state.historyAction === 'POP' && activeTransaction) { + updatePageloadTransaction(router.state.location, routes); + } + + router.subscribe((state: RouterState) => { + const location = state.location; + + if ( + _startTransactionOnLocationChange && + (state.historyAction === 'PUSH' || state.historyAction === 'POP') && + activeTransaction + ) { + handleNavigation(location, routes, state.historyAction, false); + } + }); + + return router; + }; +} diff --git a/packages/react/src/types.ts b/packages/react/src/types.ts index 1f619ccf5883..2c6eaa68362e 100644 --- a/packages/react/src/types.ts +++ b/packages/react/src/types.ts @@ -1,3 +1,5 @@ +// Disabling `no-explicit-any` for the whole file as `any` has became common requirement. +/* eslint-disable @typescript-eslint/no-explicit-any */ import { Transaction, TransactionContext } from '@sentry/types'; export type Action = 'PUSH' | 'REPLACE' | 'POP'; @@ -5,7 +7,6 @@ export type Action = 'PUSH' | 'REPLACE' | 'POP'; export type Location = { pathname: string; action?: Action; - // eslint-disable-next-line @typescript-eslint/no-explicit-any } & Record; export type ReactRouterInstrumentation = ( @@ -13,3 +14,226 @@ export type ReactRouterInstrumentation = ( startTransactionOnPageLoad?: boolean, startTransactionOnLocationChange?: boolean, ) => void; + +// React Router v6 Vendored Types +export interface NonIndexRouteObject { + caseSensitive?: boolean; + children?: RouteObject[]; + element?: React.ReactNode | null; + index?: any; + path?: string; +} + +export interface IndexRouteObject { + caseSensitive?: boolean; + children?: undefined; + element?: React.ReactNode | null; + index: any; + path?: string; +} + +// This type was originally just `type RouteObject = IndexRouteObject`, but this was changed +// in https://github.com/remix-run/react-router/pull/9366, which was released with `6.4.2` +// See https://github.com/remix-run/react-router/issues/9427 for a discussion on this. +export type RouteObject = IndexRouteObject | NonIndexRouteObject; + +export type Params = { + readonly [key in Key]: string | undefined; +}; + +export type UseRoutes = (routes: RouteObject[], locationArg?: Partial | string) => React.ReactElement | null; + +// https://github.com/remix-run/react-router/blob/9fa54d643134cd75a0335581a75db8100ed42828/packages/react-router/lib/router.ts#L114-L134 +export interface RouteMatch { + /** + * The names and values of dynamic parameters in the URL. + */ + params: Params; + /** + * The portion of the URL pathname that was matched. + */ + pathname: string; + /** + * The portion of the URL pathname that was matched before child routes. + */ + pathnameBase: string; + /** + * The route object that was used to match. + */ + route: RouteObject; +} + +export type UseEffect = (cb: () => void, deps: unknown[]) => void; +export type UseLocation = () => Location; +export type UseNavigationType = () => Action; + +// For both of these types, use `any` instead of `RouteObject[]` or `RouteMatch[]`. +// Have to do this so we maintain backwards compatability between +// react-router > 6.0.0 and >= 6.4.2. +export type RouteObjectArrayAlias = any; +export type RouteMatchAlias = any; +export type CreateRoutesFromChildren = (children: JSX.Element[]) => RouteObjectArrayAlias; +export type MatchRoutes = (routes: RouteObjectArrayAlias, location: Location) => RouteMatchAlias[] | null; + +// Types for react-router >= 6.4.2 +export type ShouldRevalidateFunction = (args: any) => boolean; + +interface DataFunctionArgs { + request: Request; + params: Params; +} + +type LoaderFunctionArgs = DataFunctionArgs; +type ActionFunctionArgs = DataFunctionArgs; + +export interface LoaderFunction { + (args: LoaderFunctionArgs): Promise | Response | Promise | any; +} +export interface ActionFunction { + (args: ActionFunctionArgs): Promise | Response | Promise | any; +} +declare type AgnosticBaseRouteObject = { + caseSensitive?: boolean; + path?: string; + id?: string; + loader?: LoaderFunction; + action?: ActionFunction; + hasErrorBoundary?: boolean; + shouldRevalidate?: ShouldRevalidateFunction; + handle?: any; +}; + +export declare type AgnosticIndexRouteObject = AgnosticBaseRouteObject & Record; + +export declare type AgnosticNonIndexRouteObject = AgnosticBaseRouteObject & Record; + +export declare type AgnosticDataIndexRouteObject = AgnosticIndexRouteObject & { + id: string; +}; + +export declare type AgnosticDataNonIndexRouteObject = AgnosticNonIndexRouteObject & { + children?: AgnosticDataRouteObject[]; + id: string; +}; + +export interface AgnosticRouteMatch< + ParamKey extends string = string, + RouteObjectType extends AgnosticRouteObject = AgnosticRouteObject, +> { + params: Params; + pathname: string; + pathnameBase: string; + route: RouteObjectType; +} + +export type AgnosticDataRouteMatch = AgnosticRouteMatch; + +interface UseMatchesMatch { + id: string; + pathname: string; + params: AgnosticRouteMatch['params']; + data: unknown; + handle: unknown; +} + +export interface GetScrollRestorationKeyFunction { + (location: Location, matches: UseMatchesMatch[]): string | null; +} + +export interface Path { + pathname: string; + search: string; + hash: string; +} + +export interface RouterSubscriber { + (state: RouterState): void; +} +export interface GetScrollPositionFunction { + (): number; +} + +declare type LinkNavigateOptions = { + replace?: boolean; + state?: any; + preventScrollReset?: boolean; +}; + +export declare type AgnosticDataRouteObject = AgnosticDataIndexRouteObject | AgnosticDataNonIndexRouteObject; +export declare type To = string | Partial; +export declare type HydrationState = any; +export declare type FormMethod = 'get' | 'post' | 'put' | 'patch' | 'delete'; +export declare type FormEncType = 'application/x-www-form-urlencoded' | 'multipart/form-data'; +export declare type RouterNavigateOptions = LinkNavigateOptions | SubmissionNavigateOptions; +export declare type AgnosticRouteObject = AgnosticIndexRouteObject | AgnosticNonIndexRouteObject; + +declare type SubmissionNavigateOptions = { + replace?: boolean; + state?: any; + formMethod?: FormMethod; + formEncType?: FormEncType; + formData: FormData; +}; + +export interface RouterInit { + basename: string; + routes: AgnosticRouteObject[]; + history: History; + hydrationData?: HydrationState; +} + +export type NavigationStates = { + Idle: any; + Loading: any; + Submitting: any; +}; + +export type Navigation = NavigationStates[keyof NavigationStates]; + +export type RouteData = any; +export type Fetcher = any; + +export declare enum HistoryAction { + Pop = 'POP', + Push = 'PUSH', + Replace = 'REPLACE', +} + +export interface RouterState { + historyAction: Action | HistoryAction | any; + location: any; + matches: AgnosticDataRouteMatch[]; + initialized: boolean; + navigation: Navigation; + restoreScrollPosition: number | false | null; + preventScrollReset: boolean; + revalidation: any; + loaderData: RouteData; + actionData: RouteData | null; + errors: RouteData | null; + fetchers: Map; +} +export interface Router { + basename: string | undefined; + state: RouterState; + routes: AgnosticDataRouteObject[]; + _internalFetchControllers: any; + _internalActiveDeferreds: any; + initialize(): Router; + subscribe(fn: RouterSubscriber): () => void; + enableScrollRestoration( + savedScrollPositions: Record, + getScrollPosition: GetScrollPositionFunction, + getKey?: any, + ): () => void; + navigate(to: number): void; + navigate(to: To, opts?: RouterNavigateOptions): void; + fetch(key: string, routeId: string, href: string, opts?: RouterNavigateOptions): void; + revalidate(): void; + createHref(location: Location | URL): string; + getFetcher(key?: string): any; + deleteFetcher(key?: string): void; + dispose(): void; +} + +export type CreateRouterFunction = (routes: RouteObject[], opts?: any) => Router; diff --git a/packages/react/test/reactrouterv6.4.test.tsx b/packages/react/test/reactrouterv6.4.test.tsx new file mode 100644 index 000000000000..aeb7953ea1ab --- /dev/null +++ b/packages/react/test/reactrouterv6.4.test.tsx @@ -0,0 +1,263 @@ +import { render } from '@testing-library/react'; +import { Request } from 'node-fetch'; +import * as React from 'react'; +import { + createMemoryRouter, + createRoutesFromChildren, + matchPath, + matchRoutes, + Navigate, + RouterProvider, + useLocation, + useNavigationType, +} from 'react-router-6.4'; + +import { reactRouterV6Instrumentation,wrapCreateBrowserRouter } from '../src'; +import { CreateRouterFunction } from '../src/types'; + +beforeAll(() => { + // @ts-ignore need to override global Request because it's not in the jest environment (even with an + // `@jest-environment jsdom` directive, for some reason) + global.Request = Request; +}); + +describe('React Router v6.4', () => { + function createInstrumentation(_opts?: { + startTransactionOnPageLoad?: boolean; + startTransactionOnLocationChange?: boolean; + }): [jest.Mock, { mockSetName: jest.Mock; mockFinish: jest.Mock }] { + const options = { + matchPath: _opts ? matchPath : undefined, + startTransactionOnLocationChange: true, + startTransactionOnPageLoad: true, + ..._opts, + }; + const mockFinish = jest.fn(); + const mockSetName = jest.fn(); + const mockStartTransaction = jest.fn().mockReturnValue({ setName: mockSetName, finish: mockFinish }); + + reactRouterV6Instrumentation( + React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + )(mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange); + return [mockStartTransaction, { mockSetName, mockFinish }]; + } + + describe('wrapCreateBrowserRouter', () => { + it('starts a pageload transaction', () => { + const [mockStartTransaction] = createInstrumentation(); + const sentryCreateBrowserRouter = wrapCreateBrowserRouter(createMemoryRouter as CreateRouterFunction); + + const router = sentryCreateBrowserRouter( + [ + { + path: '/', + element:
TEST
, + }, + ], + { + initialEntries: ['/'], + }, + ); + + render(); + + expect(mockStartTransaction).toHaveBeenCalledTimes(1); + expect(mockStartTransaction).toHaveBeenCalledWith({ + name: '/', + op: 'pageload', + tags: { + 'routing.instrumentation': 'react-router-v6', + }, + metadata: { + source: 'url', + }, + }); + }); + + it('starts a navigation transaction', () => { + const [mockStartTransaction] = createInstrumentation(); + const sentryCreateBrowserRouter = wrapCreateBrowserRouter(createMemoryRouter as CreateRouterFunction); + + const router = sentryCreateBrowserRouter( + [ + { + path: '/', + element: , + }, + { + path: 'about', + element:
About
, + }, + ], + { + initialEntries: ['/'], + }, + ); + + render(); + + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/about', + op: 'navigation', + tags: { 'routing.instrumentation': 'react-router-v6' }, + metadata: { source: 'route' }, + }); + }); + + it('works with nested routes', () => { + const [mockStartTransaction] = createInstrumentation(); + const sentryCreateBrowserRouter = wrapCreateBrowserRouter(createMemoryRouter as CreateRouterFunction); + + const router = sentryCreateBrowserRouter( + [ + { + path: '/', + element: , + }, + { + path: 'about', + element:
About
, + children: [ + { + path: 'us', + element:
Us
, + }, + ], + }, + ], + { + initialEntries: ['/'], + }, + ); + + render(); + + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/about/us', + op: 'navigation', + tags: { 'routing.instrumentation': 'react-router-v6' }, + metadata: { source: 'route' }, + }); + }); + + it('works with parameterized paths', () => { + const [mockStartTransaction] = createInstrumentation(); + const sentryCreateBrowserRouter = wrapCreateBrowserRouter(createMemoryRouter as CreateRouterFunction); + + const router = sentryCreateBrowserRouter( + [ + { + path: '/', + element: , + }, + { + path: 'about', + element:
About
, + children: [ + { + path: ':page', + element:
Page
, + }, + ], + }, + ], + { + initialEntries: ['/'], + }, + ); + + render(); + + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/about/:page', + op: 'navigation', + tags: { 'routing.instrumentation': 'react-router-v6' }, + metadata: { source: 'route' }, + }); + }); + + it('works with paths with multiple parameters', () => { + const [mockStartTransaction] = createInstrumentation(); + const sentryCreateBrowserRouter = wrapCreateBrowserRouter(createMemoryRouter as CreateRouterFunction); + + const router = sentryCreateBrowserRouter( + [ + { + path: '/', + element: , + }, + { + path: 'stores', + element:
Stores
, + children: [ + { + path: ':storeId', + element:
Store
, + children: [ + { + path: 'products', + element:
Products
, + children: [ + { + path: ':productId', + element:
Product
, + }, + ], + }, + ], + }, + ], + }, + ], + { + initialEntries: ['/'], + }, + ); + + render(); + + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/stores/:storeId/products/:productId', + op: 'navigation', + tags: { 'routing.instrumentation': 'react-router-v6' }, + metadata: { source: 'route' }, + }); + }); + + it('updates pageload transaction to a parameterized route', () => { + const [mockStartTransaction, { mockSetName }] = createInstrumentation(); + const sentryCreateBrowserRouter = wrapCreateBrowserRouter(createMemoryRouter as CreateRouterFunction); + + const router = sentryCreateBrowserRouter( + [ + { + path: 'about', + element:
About
, + children: [ + { + path: ':page', + element:
page
, + }, + ], + }, + ], + { + initialEntries: ['/about/us'], + }, + ); + + render(); + + expect(mockStartTransaction).toHaveBeenCalledTimes(1); + expect(mockSetName).toHaveBeenLastCalledWith('/about/:page', 'route'); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index 5246d1c54bbc..3fc2b4409ba1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4140,6 +4140,11 @@ history "^5.3.0" react-router-dom "^6.2.2" +"@remix-run/router@1.0.2": + version "1.0.2" + resolved "https://registry.yarnpkg.com/@remix-run/router/-/router-1.0.2.tgz#1c17eadb2fa77f80a796ad5ea9bf108e6993ef06" + integrity sha512-GRSOFhJzjGN+d4sKHTMSvNeUPoZiDHWmRnXfzaxrqe7dE/Nzlc8BiMSJdLDESZlndM7jIUrZ/F4yWqVYlI0rwQ== + "@remix-run/server-runtime@1.5.1": version "1.5.1" resolved "https://registry.yarnpkg.com/@remix-run/server-runtime/-/server-runtime-1.5.1.tgz#5272b01e6dce109dc10bd68447ceae2d039315b2" @@ -5140,6 +5145,14 @@ dependencies: "@types/node" "*" +"@types/node-fetch@^2.6.0": + version "2.6.2" + resolved "https://registry.yarnpkg.com/@types/node-fetch/-/node-fetch-2.6.2.tgz#d1a9c5fd049d9415dce61571557104dec3ec81da" + integrity sha512-DHqhlq5jeESLy19TYhLakJ07kNumXWjcDdxXsLUMJZ6ue8VZJj4kLPQVE/2mdHh3xZziNF1xppu5lwmS53HR+A== + dependencies: + "@types/node" "*" + form-data "^3.0.0" + "@types/node@*", "@types/node@>= 8", "@types/node@>=10.0.0", "@types/node@>=12.12.47", "@types/node@>=13.7.0": version "17.0.38" resolved "https://registry.yarnpkg.com/@types/node/-/node-17.0.38.tgz#f8bb07c371ccb1903f3752872c89f44006132947" @@ -21284,6 +21297,13 @@ react-refresh@0.8.3: tiny-invariant "^1.0.2" tiny-warning "^1.0.0" +"react-router-6.4@npm:react-router@6.4.2": + version "6.4.2" + resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.4.2.tgz#300628ee9ed81b8ef1597b5cb98b474efe9779b8" + integrity sha512-Rb0BAX9KHhVzT1OKhMvCDMw776aTYM0DtkxqUBP8dNBom3mPXlfNs76JNGK8wKJ1IZEY1+WGj+cvZxHVk/GiKw== + dependencies: + "@remix-run/router" "1.0.2" + "react-router-6@npm:react-router@6.3.0", react-router@6.3.0: name react-router-6 version "6.3.0"