From ee906968ad91c43a2672849d5ddb345f3d5268bf Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 5 May 2022 13:28:51 +0100 Subject: [PATCH 01/13] feat(tracing): Add react-router-v6 integration. --- packages/react/src/reactrouterv6.tsx | 158 +++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 packages/react/src/reactrouterv6.tsx diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx new file mode 100644 index 000000000000..4f8787a3917e --- /dev/null +++ b/packages/react/src/reactrouterv6.tsx @@ -0,0 +1,158 @@ +import { Transaction, TransactionContext } from '@sentry/types'; +import hoistNonReactStatics from 'hoist-non-react-statics'; +import * as React from 'react'; + +import { Action, Location } from './types'; + +interface RouteObject { + caseSensitive?: boolean; + children?: RouteObject[]; + element?: React.ReactNode; + index?: boolean; + path?: string; +} + +type Params = { + readonly [key in Key]: string | undefined; +}; + +interface RouteMatch { + params: Params; + pathname: string; + route: RouteObject; +} + +type UseLocation = () => Location; +type UseNavigationType = () => Action; +type CreateRoutesFromChildren = (children: JSX.Element[]) => RouteObject[]; +type MatchRoutes = (routes: RouteObject[], location: Location) => RouteMatch[]; + +interface ReactRouterV6InstrumentationOptions { + customStartTransaction: (context: TransactionContext) => Transaction | undefined; + useLocation: UseLocation; + useNavigationType: UseNavigationType; + createRoutesFromChildren: CreateRoutesFromChildren; + matchRoutes: MatchRoutes; + startTransactionOnLocationChange?: boolean; + startTransactionOnPageLoad?: boolean; +} + +const instrumentationOptions: Partial = { + customStartTransaction: () => undefined, +}; + +export function reactRouterV6Instrumentation( + useLocation: UseLocation, + useNavigationType: UseNavigationType, + createRoutesFromChildren: CreateRoutesFromChildren, + matchRoutes: MatchRoutes, +) { + return ( + customStartTransaction: (context: TransactionContext) => Transaction | undefined, + startTransactionOnPageLoad = true, + startTransactionOnLocationChange = true, + ): void => { + instrumentationOptions.useLocation = useLocation; + instrumentationOptions.useNavigationType = useNavigationType; + instrumentationOptions.matchRoutes = matchRoutes; + instrumentationOptions.createRoutesFromChildren = createRoutesFromChildren; + + instrumentationOptions.customStartTransaction = customStartTransaction; + instrumentationOptions.startTransactionOnLocationChange = startTransactionOnLocationChange; + instrumentationOptions.startTransactionOnPageLoad = startTransactionOnPageLoad; + }; +} + +const getTransactionName = (routes: RouteObject[], location: Location, matchRoutes: MatchRoutes): string => { + if (!routes || routes.length === 0 || !matchRoutes) { + return location.pathname; + } + + const branches = matchRoutes(routes, location); + + // eslint-disable-next-line @typescript-eslint/prefer-for-of + for (let x = 0; x < branches.length; x++) { + if (branches[x].route && branches[x].route.path) { + return branches[x].route.path || location.pathname; + } + } + + return location.pathname; +}; + +const SENTRY_TAGS = { + 'routing.instrumentation': 'react-router-v6', +}; + +export function withSentryV6

, R extends React.ComponentType

>(Routes: R): R { + const { useLocation, useNavigationType, createRoutesFromChildren, matchRoutes, customStartTransaction } = + instrumentationOptions; + + if (!useLocation || !useNavigationType || !createRoutesFromChildren || !matchRoutes || !customStartTransaction) { + // Log warning? + return Routes; + } + + const SentryRoutes: React.FC

= (props: P) => { + const location = useLocation(); + const navigationType = useNavigationType(); + const isBaseLocation = React.useRef(false); + const activeTransaction = React.useRef(); + const routes = React.useRef([]); + + React.useEffect(() => { + // Performance concern: + // This is repeated when is rendered. + routes.current = createRoutesFromChildren(props.children); + }, [props.children]); + + React.useEffect(() => { + const transactionName = getTransactionName(routes.current, location, matchRoutes); + + isBaseLocation.current = true; + + activeTransaction.current = customStartTransaction({ + name: transactionName, + op: 'pageload', + tags: SENTRY_TAGS, + }); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + React.useEffect(() => { + if (isBaseLocation.current) { + if (activeTransaction.current) { + activeTransaction.current.finish(); + } + + return; + } + + if (navigationType === 'PUSH' || navigationType === 'POP') { + if (activeTransaction.current) { + activeTransaction.current.finish(); + } + + const transactionName = getTransactionName(routes.current, location, matchRoutes); + + activeTransaction.current = customStartTransaction({ + name: transactionName, + op: 'navigation', + tags: SENTRY_TAGS, + }); + } + }, [navigationType, location, isBaseLocation]); + + // @ts-ignore Setting more specific React Component typing for `R` generic above + // will break advanced type inference done by react router params: + return ; + }; + + hoistNonReactStatics(SentryRoutes, Routes); + + // @ts-ignore Setting more specific React Component typing for `R` generic above + // will break advanced type inference done by react router params: + return SentryRoutes; +} + +export default SentryRoutes; From 921ea552dd5a419611f4c4beb6c25ed7864b7685 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 6 May 2022 09:07:13 +0100 Subject: [PATCH 02/13] Refactor globals. --- packages/react/src/reactrouterv6.tsx | 71 ++++++++++++---------------- 1 file changed, 31 insertions(+), 40 deletions(-) diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index 4f8787a3917e..aa6e45826453 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -27,19 +27,13 @@ type UseNavigationType = () => Action; type CreateRoutesFromChildren = (children: JSX.Element[]) => RouteObject[]; type MatchRoutes = (routes: RouteObject[], location: Location) => RouteMatch[]; -interface ReactRouterV6InstrumentationOptions { - customStartTransaction: (context: TransactionContext) => Transaction | undefined; - useLocation: UseLocation; - useNavigationType: UseNavigationType; - createRoutesFromChildren: CreateRoutesFromChildren; - matchRoutes: MatchRoutes; - startTransactionOnLocationChange?: boolean; - startTransactionOnPageLoad?: boolean; -} - -const instrumentationOptions: Partial = { - customStartTransaction: () => undefined, -}; +let _useLocation: UseLocation; +let _useNavigationType: UseNavigationType; +let _createRoutesFromChildren: CreateRoutesFromChildren; +let _matchRoutes: MatchRoutes; +let _customStartTransaction: (context: TransactionContext) => Transaction | undefined; +let _startTransactionOnLocationChange: boolean; +let _startTransactionOnPageLoad: boolean; export function reactRouterV6Instrumentation( useLocation: UseLocation, @@ -52,14 +46,14 @@ export function reactRouterV6Instrumentation( startTransactionOnPageLoad = true, startTransactionOnLocationChange = true, ): void => { - instrumentationOptions.useLocation = useLocation; - instrumentationOptions.useNavigationType = useNavigationType; - instrumentationOptions.matchRoutes = matchRoutes; - instrumentationOptions.createRoutesFromChildren = createRoutesFromChildren; - - instrumentationOptions.customStartTransaction = customStartTransaction; - instrumentationOptions.startTransactionOnLocationChange = startTransactionOnLocationChange; - instrumentationOptions.startTransactionOnPageLoad = startTransactionOnPageLoad; + _useLocation = useLocation; + _useNavigationType = useNavigationType; + _matchRoutes = matchRoutes; + _createRoutesFromChildren = createRoutesFromChildren; + + _customStartTransaction = customStartTransaction; + _startTransactionOnLocationChange = startTransactionOnLocationChange; + _startTransactionOnPageLoad = startTransactionOnPageLoad; }; } @@ -85,17 +79,14 @@ const SENTRY_TAGS = { }; export function withSentryV6

, R extends React.ComponentType

>(Routes: R): R { - const { useLocation, useNavigationType, createRoutesFromChildren, matchRoutes, customStartTransaction } = - instrumentationOptions; - - if (!useLocation || !useNavigationType || !createRoutesFromChildren || !matchRoutes || !customStartTransaction) { + if (!_useLocation || !_useNavigationType || !_createRoutesFromChildren || !_matchRoutes || !_customStartTransaction) { // Log warning? return Routes; } const SentryRoutes: React.FC

= (props: P) => { - const location = useLocation(); - const navigationType = useNavigationType(); + const location = _useLocation(); + const navigationType = _useNavigationType(); const isBaseLocation = React.useRef(false); const activeTransaction = React.useRef(); const routes = React.useRef([]); @@ -103,19 +94,21 @@ export function withSentryV6

, R extends React.Comp React.useEffect(() => { // Performance concern: // This is repeated when is rendered. - routes.current = createRoutesFromChildren(props.children); + routes.current = _createRoutesFromChildren(props.children); }, [props.children]); React.useEffect(() => { - const transactionName = getTransactionName(routes.current, location, matchRoutes); + if (_startTransactionOnPageLoad) { + const transactionName = getTransactionName(routes.current, location, _matchRoutes); - isBaseLocation.current = true; + isBaseLocation.current = true; - activeTransaction.current = customStartTransaction({ - name: transactionName, - op: 'pageload', - tags: SENTRY_TAGS, - }); + activeTransaction.current = _customStartTransaction({ + name: transactionName, + op: 'pageload', + tags: SENTRY_TAGS, + }); + } // eslint-disable-next-line react-hooks/exhaustive-deps }, []); @@ -128,14 +121,14 @@ export function withSentryV6

, R extends React.Comp return; } - if (navigationType === 'PUSH' || navigationType === 'POP') { + if (_startTransactionOnLocationChange && (navigationType === 'PUSH' || navigationType === 'POP')) { if (activeTransaction.current) { activeTransaction.current.finish(); } - const transactionName = getTransactionName(routes.current, location, matchRoutes); + const transactionName = getTransactionName(routes.current, location, _matchRoutes); - activeTransaction.current = customStartTransaction({ + activeTransaction.current = _customStartTransaction({ name: transactionName, op: 'navigation', tags: SENTRY_TAGS, @@ -154,5 +147,3 @@ export function withSentryV6

, R extends React.Comp // will break advanced type inference done by react router params: return SentryRoutes; } - -export default SentryRoutes; From b14fbce0b690b3553c93ae7c068dc30fddce9e4a Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 6 May 2022 12:48:45 +0100 Subject: [PATCH 03/13] Support nested routes. --- packages/react/src/reactrouterv6.tsx | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index aa6e45826453..d9d5a2756ad3 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -25,7 +25,7 @@ interface RouteMatch { type UseLocation = () => Location; type UseNavigationType = () => Action; type CreateRoutesFromChildren = (children: JSX.Element[]) => RouteObject[]; -type MatchRoutes = (routes: RouteObject[], location: Location) => RouteMatch[]; +type MatchRoutes = (routes: RouteObject[], location: Location) => RouteMatch[] | null; let _useLocation: UseLocation; let _useNavigationType: UseNavigationType; @@ -64,10 +64,12 @@ const getTransactionName = (routes: RouteObject[], location: Location, matchRout const branches = matchRoutes(routes, location); - // eslint-disable-next-line @typescript-eslint/prefer-for-of - for (let x = 0; x < branches.length; x++) { - if (branches[x].route && branches[x].route.path) { - return branches[x].route.path || location.pathname; + if (branches) { + // eslint-disable-next-line @typescript-eslint/prefer-for-of + for (let x = 0; x < branches.length; x++) { + if (branches[x].route && branches[x].route.path && branches[x].pathname === location.pathname) { + return branches[x].route.path || location.pathname; + } } } @@ -98,11 +100,11 @@ export function withSentryV6

, R extends React.Comp }, [props.children]); React.useEffect(() => { + isBaseLocation.current = true; + if (_startTransactionOnPageLoad) { const transactionName = getTransactionName(routes.current, location, _matchRoutes); - isBaseLocation.current = true; - activeTransaction.current = _customStartTransaction({ name: transactionName, op: 'pageload', @@ -136,6 +138,8 @@ export function withSentryV6

, R extends React.Comp } }, [navigationType, location, isBaseLocation]); + isBaseLocation.current = false; + // @ts-ignore Setting more specific React Component typing for `R` generic above // will break advanced type inference done by react router params: return ; From cdc09519292b46d6fefe3aff738f15644df828d2 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 6 May 2022 12:48:57 +0100 Subject: [PATCH 04/13] Add tests. --- packages/react/package.json | 1 + packages/react/src/index.ts | 1 + packages/react/test/reactrouterv6.test.tsx | 164 +++++++++++++++++++++ yarn.lock | 21 +++ 4 files changed, 187 insertions(+) create mode 100644 packages/react/test/reactrouterv6.test.tsx diff --git a/packages/react/package.json b/packages/react/package.json index 03f1873aa436..40c5bb3b4883 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -44,6 +44,7 @@ "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", "redux": "^4.0.5" }, "scripts": { diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index 306f90e4f943..bb129e9cf316 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -6,3 +6,4 @@ export { ErrorBoundary, withErrorBoundary } from './errorboundary'; export { createReduxEnhancer } from './redux'; export { reactRouterV3Instrumentation } from './reactrouterv3'; export { reactRouterV4Instrumentation, reactRouterV5Instrumentation, withSentryRouting } from './reactrouter'; +export { reactRouterV6Instrumentation, withSentryV6 } from './reactrouterv6'; diff --git a/packages/react/test/reactrouterv6.test.tsx b/packages/react/test/reactrouterv6.test.tsx new file mode 100644 index 000000000000..3abaf2f9d2f8 --- /dev/null +++ b/packages/react/test/reactrouterv6.test.tsx @@ -0,0 +1,164 @@ +import { render } from '@testing-library/react'; +import * as React from 'react'; +import { + matchPath, + MemoryRouter, + Routes, + Route, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + Navigate, +} from 'react-router-6'; + +import { reactRouterV6Instrumentation } from '../src'; +import { withSentryV6 } from '../src/reactrouterv6'; + +describe('React Router v6', () => { + 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( + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + )(mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange); + return [mockStartTransaction, { mockSetName, mockFinish }]; + } + + it('starts a pageload transaction', () => { + const [mockStartTransaction] = createInstrumentation(); + const SentryRoutes = withSentryV6(Routes); + + render( + + + Home} /> + + , + ); + + expect(mockStartTransaction).toHaveBeenCalledTimes(1); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/', + op: 'pageload', + tags: { 'routing.instrumentation': 'react-router-v6' }, + }); + }); + + it('skips pageload transaction with `startTransactionOnPageLoad: false`', () => { + const [mockStartTransaction] = createInstrumentation({ startTransactionOnPageLoad: false }); + const SentryRoutes = withSentryV6(Routes); + + render( + + + Home} /> + + , + ); + + expect(mockStartTransaction).toHaveBeenCalledTimes(0); + }); + + it('skips navigation transaction, with `startTransactionOnLocationChange: false`', () => { + const [mockStartTransaction] = createInstrumentation({ startTransactionOnLocationChange: false }); + const SentryRoutes = withSentryV6(Routes); + + render( + + + About} /> + } /> + + , + ); + + expect(mockStartTransaction).toHaveBeenCalledTimes(1); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/', + op: 'pageload', + tags: { 'routing.instrumentation': 'react-router-v6' }, + }); + }); + + it('starts a navigation transaction', () => { + const [mockStartTransaction] = createInstrumentation(); + const SentryRoutes = withSentryV6(Routes); + + render( + + + About} /> + } /> + + , + ); + + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/about', + op: 'navigation', + tags: { 'routing.instrumentation': 'react-router-v6' }, + }); + }); + + it('works with nested routes', () => { + const [mockStartTransaction] = createInstrumentation(); + const SentryRoutes = withSentryV6(Routes); + + render( + + + About}> + us} /> + + } /> + + , + ); + + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/about/us', + op: 'navigation', + tags: { 'routing.instrumentation': 'react-router-v6' }, + }); + }); + + it('works with original paths', () => { + const [mockStartTransaction] = createInstrumentation(); + const SentryRoutes = withSentryV6(Routes); + + render( + + + About}> + page} /> + + } /> + + , + ); + + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/about/:page', + op: 'navigation', + tags: { 'routing.instrumentation': 'react-router-v6' }, + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index bf506395377a..4d5ab1bdfa5e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2247,6 +2247,13 @@ dependencies: regenerator-runtime "^0.13.4" +"@babel/runtime@^7.7.6": + version "7.17.9" + resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.17.9.tgz#d19fbf802d01a8cb6cf053a64e472d42c434ba72" + integrity sha512-lSiBBvodq29uShpWGNbgFdKYNiFDo5/HIYsaCEY9ff4sb10x9jizo2+pRrSyF4jKZCXqgzuqBOQKbUm90gQwJg== + dependencies: + regenerator-runtime "^0.13.4" + "@babel/template@7.10.4": version "7.10.4" resolved "https://registry.yarnpkg.com/@babel/template/-/template-7.10.4.tgz#3251996c4200ebc71d1a8fc405fba940f36ba278" @@ -14086,6 +14093,13 @@ history@^4.6.0, history@^4.9.0: tiny-warning "^1.0.0" value-equal "^1.0.1" +history@^5.2.0: + version "5.3.0" + resolved "https://registry.yarnpkg.com/history/-/history-5.3.0.tgz#1548abaa245ba47992f063a0783db91ef201c73b" + integrity sha512-ZqaKwjjrAYUYfLG+htGaIIZ4nioX2L70ZUMIFysS3xvBsSG4x/n1V6TXV3N8ZYNuFGlDirFg32T7B6WOUPDYcQ== + dependencies: + "@babel/runtime" "^7.7.6" + hmac-drbg@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/hmac-drbg/-/hmac-drbg-1.0.1.tgz#d2745701025a6c775a6c545793ed502fc0c649a1" @@ -20989,6 +21003,13 @@ react-refresh@0.8.3: tiny-invariant "^1.0.2" tiny-warning "^1.0.0" +"react-router-6@npm:react-router@6.3.0": + version "6.3.0" + resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.3.0.tgz#3970cc64b4cb4eae0c1ea5203a80334fdd175557" + integrity sha512-7Wh1DzVQ+tlFjkeo+ujvjSqSJmkt1+8JO+T5xklPlgrh70y7ogx75ODRW0ThWhY7S+6yEDks8TYrtQe/aoboBQ== + dependencies: + history "^5.2.0" + react@^18.0.0: version "18.0.0" resolved "https://registry.yarnpkg.com/react/-/react-18.0.0.tgz#b468736d1f4a5891f38585ba8e8fb29f91c3cb96" From 5eb5cb787ac349cda78f8f723fbc3c7da3446e3d Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 6 May 2022 13:23:45 +0100 Subject: [PATCH 05/13] Fix linter. --- packages/react/test/reactrouterv6.test.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react/test/reactrouterv6.test.tsx b/packages/react/test/reactrouterv6.test.tsx index 3abaf2f9d2f8..0a325a8e8e61 100644 --- a/packages/react/test/reactrouterv6.test.tsx +++ b/packages/react/test/reactrouterv6.test.tsx @@ -1,15 +1,15 @@ import { render } from '@testing-library/react'; import * as React from 'react'; import { + createRoutesFromChildren, matchPath, + matchRoutes, MemoryRouter, - Routes, + Navigate, Route, + Routes, useLocation, useNavigationType, - createRoutesFromChildren, - matchRoutes, - Navigate, } from 'react-router-6'; import { reactRouterV6Instrumentation } from '../src'; From 1da5a7fa45f172476efc7eb03b5ff910ec428216 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 6 May 2022 17:04:15 +0100 Subject: [PATCH 06/13] Refactor to avoid mismatched React versions, add `useEffect` to params. --- packages/react/src/reactrouterv6.tsx | 63 ++++++++++++---------- packages/react/test/reactrouterv6.test.tsx | 1 + 2 files changed, 37 insertions(+), 27 deletions(-) diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index d9d5a2756ad3..d370868a3341 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -1,6 +1,6 @@ import { Transaction, TransactionContext } from '@sentry/types'; import hoistNonReactStatics from 'hoist-non-react-statics'; -import * as React from 'react'; +import React from 'react'; import { Action, Location } from './types'; @@ -22,11 +22,13 @@ interface RouteMatch { route: RouteObject; } +type UseEffect = (cb: () => void, deps: unknown[]) => void; type UseLocation = () => Location; type UseNavigationType = () => Action; type CreateRoutesFromChildren = (children: JSX.Element[]) => RouteObject[]; type MatchRoutes = (routes: RouteObject[], location: Location) => RouteMatch[] | null; +let _useEffect: UseEffect; let _useLocation: UseLocation; let _useNavigationType: UseNavigationType; let _createRoutesFromChildren: CreateRoutesFromChildren; @@ -36,6 +38,7 @@ let _startTransactionOnLocationChange: boolean; let _startTransactionOnPageLoad: boolean; export function reactRouterV6Instrumentation( + useEffect: UseEffect, useLocation: UseLocation, useNavigationType: UseNavigationType, createRoutesFromChildren: CreateRoutesFromChildren, @@ -46,6 +49,7 @@ export function reactRouterV6Instrumentation( startTransactionOnPageLoad = true, startTransactionOnLocationChange = true, ): void => { + _useEffect = useEffect; _useLocation = useLocation; _useNavigationType = useNavigationType; _matchRoutes = matchRoutes; @@ -80,74 +84,79 @@ const SENTRY_TAGS = { 'routing.instrumentation': 'react-router-v6', }; -export function withSentryV6

, R extends React.ComponentType

>(Routes: R): R { - if (!_useLocation || !_useNavigationType || !_createRoutesFromChildren || !_matchRoutes || !_customStartTransaction) { +export function withSentryV6

, R extends React.FC

>(Routes: R): R { + if ( + !_useEffect || + !_useLocation || + !_useNavigationType || + !_createRoutesFromChildren || + !_matchRoutes || + !_customStartTransaction + ) { // Log warning? return Routes; } + let isBaseLocation: boolean = false; + let activeTransaction: Transaction | undefined; + let routes: RouteObject[]; + const SentryRoutes: React.FC

= (props: P) => { const location = _useLocation(); const navigationType = _useNavigationType(); - const isBaseLocation = React.useRef(false); - const activeTransaction = React.useRef(); - const routes = React.useRef([]); - React.useEffect(() => { + _useEffect(() => { // Performance concern: // This is repeated when is rendered. - routes.current = _createRoutesFromChildren(props.children); - }, [props.children]); - - React.useEffect(() => { - isBaseLocation.current = true; + routes = _createRoutesFromChildren(props.children); + isBaseLocation = true; if (_startTransactionOnPageLoad) { - const transactionName = getTransactionName(routes.current, location, _matchRoutes); + const transactionName = getTransactionName(routes, location, _matchRoutes); - activeTransaction.current = _customStartTransaction({ + activeTransaction = _customStartTransaction({ name: transactionName, op: 'pageload', tags: SENTRY_TAGS, }); } // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [props.children]); - React.useEffect(() => { - if (isBaseLocation.current) { - if (activeTransaction.current) { - activeTransaction.current.finish(); + _useEffect(() => { + if (isBaseLocation) { + if (activeTransaction) { + activeTransaction.finish(); } return; } if (_startTransactionOnLocationChange && (navigationType === 'PUSH' || navigationType === 'POP')) { - if (activeTransaction.current) { - activeTransaction.current.finish(); + if (activeTransaction) { + activeTransaction.finish(); } - const transactionName = getTransactionName(routes.current, location, _matchRoutes); + const transactionName = getTransactionName(routes, location, _matchRoutes); - activeTransaction.current = _customStartTransaction({ + activeTransaction = _customStartTransaction({ name: transactionName, op: 'navigation', tags: SENTRY_TAGS, }); } - }, [navigationType, location, isBaseLocation]); + }, [props.children, location, navigationType, isBaseLocation]); - isBaseLocation.current = false; + isBaseLocation = false; // @ts-ignore Setting more specific React Component typing for `R` generic above - // will break advanced type inference done by react router params: + // will break advanced type inference done by react router params return ; }; hoistNonReactStatics(SentryRoutes, Routes); // @ts-ignore Setting more specific React Component typing for `R` generic above - // will break advanced type inference done by react router params: + // will break advanced type inference done by react router params return SentryRoutes; } diff --git a/packages/react/test/reactrouterv6.test.tsx b/packages/react/test/reactrouterv6.test.tsx index 0a325a8e8e61..a2f3dfada247 100644 --- a/packages/react/test/reactrouterv6.test.tsx +++ b/packages/react/test/reactrouterv6.test.tsx @@ -31,6 +31,7 @@ describe('React Router v6', () => { const mockStartTransaction = jest.fn().mockReturnValue({ setName: mockSetName, finish: mockFinish }); reactRouterV6Instrumentation( + React.useEffect, useLocation, useNavigationType, createRoutesFromChildren, From e5da79832eabad97b0f88f17a34900f25bca1bf4 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 11 May 2022 10:43:59 +0100 Subject: [PATCH 07/13] Rename wrapper as `withSentryReactRouterV6Routing`. --- packages/react/src/index.ts | 2 +- packages/react/src/reactrouterv6.tsx | 2 +- packages/react/test/reactrouterv6.test.tsx | 14 +++++++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index bb129e9cf316..150f4571ef85 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -6,4 +6,4 @@ export { ErrorBoundary, withErrorBoundary } from './errorboundary'; export { createReduxEnhancer } from './redux'; export { reactRouterV3Instrumentation } from './reactrouterv3'; export { reactRouterV4Instrumentation, reactRouterV5Instrumentation, withSentryRouting } from './reactrouter'; -export { reactRouterV6Instrumentation, withSentryV6 } from './reactrouterv6'; +export { reactRouterV6Instrumentation, withSentryReactRouterV6Routing } from './reactrouterv6'; diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index d370868a3341..0554d4672eea 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -84,7 +84,7 @@ const SENTRY_TAGS = { 'routing.instrumentation': 'react-router-v6', }; -export function withSentryV6

, R extends React.FC

>(Routes: R): R { +export function withSentryReactRouterV6Routing

, R extends React.FC

>(Routes: R): R { if ( !_useEffect || !_useLocation || diff --git a/packages/react/test/reactrouterv6.test.tsx b/packages/react/test/reactrouterv6.test.tsx index a2f3dfada247..442a7302e457 100644 --- a/packages/react/test/reactrouterv6.test.tsx +++ b/packages/react/test/reactrouterv6.test.tsx @@ -13,7 +13,7 @@ import { } from 'react-router-6'; import { reactRouterV6Instrumentation } from '../src'; -import { withSentryV6 } from '../src/reactrouterv6'; +import { withSentryReactRouterV6Routing } from '../src/reactrouterv6'; describe('React Router v6', () => { function createInstrumentation(_opts?: { @@ -42,7 +42,7 @@ describe('React Router v6', () => { it('starts a pageload transaction', () => { const [mockStartTransaction] = createInstrumentation(); - const SentryRoutes = withSentryV6(Routes); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); render( @@ -62,7 +62,7 @@ describe('React Router v6', () => { it('skips pageload transaction with `startTransactionOnPageLoad: false`', () => { const [mockStartTransaction] = createInstrumentation({ startTransactionOnPageLoad: false }); - const SentryRoutes = withSentryV6(Routes); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); render( @@ -77,7 +77,7 @@ describe('React Router v6', () => { it('skips navigation transaction, with `startTransactionOnLocationChange: false`', () => { const [mockStartTransaction] = createInstrumentation({ startTransactionOnLocationChange: false }); - const SentryRoutes = withSentryV6(Routes); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); render( @@ -98,7 +98,7 @@ describe('React Router v6', () => { it('starts a navigation transaction', () => { const [mockStartTransaction] = createInstrumentation(); - const SentryRoutes = withSentryV6(Routes); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); render( @@ -119,7 +119,7 @@ describe('React Router v6', () => { it('works with nested routes', () => { const [mockStartTransaction] = createInstrumentation(); - const SentryRoutes = withSentryV6(Routes); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); render( @@ -142,7 +142,7 @@ describe('React Router v6', () => { it('works with original paths', () => { const [mockStartTransaction] = createInstrumentation(); - const SentryRoutes = withSentryV6(Routes); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); render( From f0bf98b487669d79c398f9251ab237d55ce0111c Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 11 May 2022 10:46:07 +0100 Subject: [PATCH 08/13] Remove unnecessary variables. --- packages/react/src/reactrouterv6.tsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index 0554d4672eea..56eb87dc14cb 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -112,10 +112,8 @@ export function withSentryReactRouterV6Routing

, R isBaseLocation = true; if (_startTransactionOnPageLoad) { - const transactionName = getTransactionName(routes, location, _matchRoutes); - activeTransaction = _customStartTransaction({ - name: transactionName, + name: getTransactionName(routes, location, _matchRoutes), op: 'pageload', tags: SENTRY_TAGS, }); @@ -137,10 +135,8 @@ export function withSentryReactRouterV6Routing

, R activeTransaction.finish(); } - const transactionName = getTransactionName(routes, location, _matchRoutes); - activeTransaction = _customStartTransaction({ - name: transactionName, + name: getTransactionName(routes, location, _matchRoutes), op: 'navigation', tags: SENTRY_TAGS, }); From 7afd96171ff1f629aa23aac025c06f2f14ac44d7 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 11 May 2022 10:47:10 +0100 Subject: [PATCH 09/13] Update packages/react/test/reactrouterv6.test.tsx Co-authored-by: Abhijeet Prasad --- packages/react/test/reactrouterv6.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/test/reactrouterv6.test.tsx b/packages/react/test/reactrouterv6.test.tsx index 442a7302e457..570e742722f5 100644 --- a/packages/react/test/reactrouterv6.test.tsx +++ b/packages/react/test/reactrouterv6.test.tsx @@ -140,7 +140,7 @@ describe('React Router v6', () => { }); }); - it('works with original paths', () => { + it('works with paramaterized paths', () => { const [mockStartTransaction] = createInstrumentation(); const SentryRoutes = withSentryReactRouterV6Routing(Routes); From dee1952c3f7a3da7ea8d63e4f29ad57bc18c4a7d Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 11 May 2022 10:54:23 +0100 Subject: [PATCH 10/13] Add multiple params test case. --- packages/react/test/reactrouterv6.test.tsx | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/packages/react/test/reactrouterv6.test.tsx b/packages/react/test/reactrouterv6.test.tsx index 570e742722f5..5e30d96a83a9 100644 --- a/packages/react/test/reactrouterv6.test.tsx +++ b/packages/react/test/reactrouterv6.test.tsx @@ -162,4 +162,29 @@ describe('React Router v6', () => { tags: { 'routing.instrumentation': 'react-router-v6' }, }); }); + + it('works with paths with multiple parameters', () => { + const [mockStartTransaction] = createInstrumentation(); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); + + render( + + + Stores}> + Store}> + Product} /> + + + } /> + + , + ); + + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/stores/:storeId/products/:productId', + op: 'navigation', + tags: { 'routing.instrumentation': 'react-router-v6' }, + }); + }); }); From 75d9fe473f30b49c82895b48c1e17891137c972d Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 11 May 2022 11:35:08 +0100 Subject: [PATCH 11/13] Start pageload transaction on sentry initialization. --- packages/react/src/reactrouterv6.tsx | 42 +++++++++++++++++++--------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index 56eb87dc14cb..dbbef3a5040e 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -1,4 +1,5 @@ import { Transaction, TransactionContext } from '@sentry/types'; +import { getGlobalObject } from '@sentry/utils'; import hoistNonReactStatics from 'hoist-non-react-statics'; import React from 'react'; @@ -28,6 +29,8 @@ type UseNavigationType = () => Action; type CreateRoutesFromChildren = (children: JSX.Element[]) => RouteObject[]; type MatchRoutes = (routes: RouteObject[], location: Location) => RouteMatch[] | null; +let activeTransaction: Transaction | undefined; + let _useEffect: UseEffect; let _useLocation: UseLocation; let _useNavigationType: UseNavigationType; @@ -35,7 +38,20 @@ let _createRoutesFromChildren: CreateRoutesFromChildren; let _matchRoutes: MatchRoutes; let _customStartTransaction: (context: TransactionContext) => Transaction | undefined; let _startTransactionOnLocationChange: boolean; -let _startTransactionOnPageLoad: boolean; + +const global = getGlobalObject(); + +const SENTRY_TAGS = { + 'routing.instrumentation': 'react-router-v6', +}; + +function getInitPathName(): string | undefined { + if (global && global.location) { + return global.location.pathname; + } + + return undefined; +} export function reactRouterV6Instrumentation( useEffect: UseEffect, @@ -49,6 +65,15 @@ export function reactRouterV6Instrumentation( startTransactionOnPageLoad = true, startTransactionOnLocationChange = true, ): void => { + const initPathName = getInitPathName(); + if (startTransactionOnPageLoad && initPathName) { + activeTransaction = customStartTransaction({ + name: initPathName, + op: 'pageload', + tags: SENTRY_TAGS, + }); + } + _useEffect = useEffect; _useLocation = useLocation; _useNavigationType = useNavigationType; @@ -57,7 +82,6 @@ export function reactRouterV6Instrumentation( _customStartTransaction = customStartTransaction; _startTransactionOnLocationChange = startTransactionOnLocationChange; - _startTransactionOnPageLoad = startTransactionOnPageLoad; }; } @@ -80,10 +104,6 @@ const getTransactionName = (routes: RouteObject[], location: Location, matchRout return location.pathname; }; -const SENTRY_TAGS = { - 'routing.instrumentation': 'react-router-v6', -}; - export function withSentryReactRouterV6Routing

, R extends React.FC

>(Routes: R): R { if ( !_useEffect || @@ -98,7 +118,6 @@ export function withSentryReactRouterV6Routing

, R } let isBaseLocation: boolean = false; - let activeTransaction: Transaction | undefined; let routes: RouteObject[]; const SentryRoutes: React.FC

= (props: P) => { @@ -111,13 +130,10 @@ export function withSentryReactRouterV6Routing

, R routes = _createRoutesFromChildren(props.children); isBaseLocation = true; - if (_startTransactionOnPageLoad) { - activeTransaction = _customStartTransaction({ - name: getTransactionName(routes, location, _matchRoutes), - op: 'pageload', - tags: SENTRY_TAGS, - }); + if (activeTransaction) { + activeTransaction.setName(getTransactionName(routes, location, _matchRoutes)); } + // eslint-disable-next-line react-hooks/exhaustive-deps }, [props.children]); From 8fb7471b58b44f3febabf2a723833675814ba197 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 11 May 2022 11:55:30 +0100 Subject: [PATCH 12/13] Log warning in case of missing parameters on init. --- packages/react/src/reactrouterv6.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index dbbef3a5040e..ddec8766c518 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -1,8 +1,9 @@ import { Transaction, TransactionContext } from '@sentry/types'; -import { getGlobalObject } from '@sentry/utils'; +import { getGlobalObject, logger } from '@sentry/utils'; import hoistNonReactStatics from 'hoist-non-react-statics'; import React from 'react'; +import { IS_DEBUG_BUILD } from './flags'; import { Action, Location } from './types'; interface RouteObject { @@ -113,7 +114,9 @@ export function withSentryReactRouterV6Routing

, R !_matchRoutes || !_customStartTransaction ) { - // Log warning? + IS_DEBUG_BUILD && + logger.warn('reactRouterV6Instrumentation was unable to wrap Routes because of one or more missing parameters.'); + return Routes; } From 48e2c95a9abeeb4e4a3354251d50f51c80e06167 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 11 May 2022 15:38:12 +0100 Subject: [PATCH 13/13] Give attribution to @wontondon. --- packages/react/src/reactrouterv6.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index ddec8766c518..a793f8824e1d 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -1,3 +1,6 @@ +// Inspired from Donnie McNeal's solution: +// https://gist.github.com/wontondon/e8c4bdf2888875e4c755712e99279536 + import { Transaction, TransactionContext } from '@sentry/types'; import { getGlobalObject, logger } from '@sentry/utils'; import hoistNonReactStatics from 'hoist-non-react-statics';