-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(react): Add react-router-v6 integration. #5042
Changes from all commits
ee90696
921ea55
b14fbce
cdc0951
5eb5cb7
1da5a7f
e5da798
f0bf98b
7afd961
dee1952
75d9fe4
8fb7471
48e2c95
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,6 +44,7 @@ | |
"react-router-3": "npm:[email protected]", | ||
"react-router-4": "npm:[email protected]", | ||
"react-router-5": "npm:[email protected]", | ||
"react-router-6": "npm:[email protected]", | ||
"redux": "^4.0.5" | ||
}, | ||
"scripts": { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,180 @@ | ||
// 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'; | ||
import React from 'react'; | ||
|
||
import { IS_DEBUG_BUILD } from './flags'; | ||
import { Action, Location } from './types'; | ||
|
||
interface RouteObject { | ||
caseSensitive?: boolean; | ||
children?: RouteObject[]; | ||
element?: React.ReactNode; | ||
index?: boolean; | ||
path?: string; | ||
} | ||
|
||
type Params<Key extends string = string> = { | ||
readonly [key in Key]: string | undefined; | ||
}; | ||
|
||
interface RouteMatch<ParamKey extends string = string> { | ||
params: Params<ParamKey>; | ||
pathname: string; | ||
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 activeTransaction: Transaction | undefined; | ||
|
||
let _useEffect: UseEffect; | ||
let _useLocation: UseLocation; | ||
let _useNavigationType: UseNavigationType; | ||
let _createRoutesFromChildren: CreateRoutesFromChildren; | ||
let _matchRoutes: MatchRoutes; | ||
let _customStartTransaction: (context: TransactionContext) => Transaction | undefined; | ||
let _startTransactionOnLocationChange: boolean; | ||
|
||
const global = getGlobalObject<Window>(); | ||
|
||
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, | ||
useLocation: UseLocation, | ||
useNavigationType: UseNavigationType, | ||
createRoutesFromChildren: CreateRoutesFromChildren, | ||
matchRoutes: MatchRoutes, | ||
) { | ||
return ( | ||
customStartTransaction: (context: TransactionContext) => Transaction | undefined, | ||
startTransactionOnPageLoad = true, | ||
startTransactionOnLocationChange = true, | ||
): void => { | ||
const initPathName = getInitPathName(); | ||
if (startTransactionOnPageLoad && initPathName) { | ||
activeTransaction = customStartTransaction({ | ||
name: initPathName, | ||
op: 'pageload', | ||
tags: SENTRY_TAGS, | ||
}); | ||
} | ||
|
||
_useEffect = useEffect; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we start the transaction here and just update the transaction name in the useeffect call? Also, is there a way we can listen directly onto There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated to start pageload transaction on init 👍 Tried to find a simple solution to access There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok - no worries, let's keep using the hooks then! |
||
_useLocation = useLocation; | ||
_useNavigationType = useNavigationType; | ||
_matchRoutes = matchRoutes; | ||
_createRoutesFromChildren = createRoutesFromChildren; | ||
|
||
_customStartTransaction = customStartTransaction; | ||
_startTransactionOnLocationChange = startTransactionOnLocationChange; | ||
}; | ||
} | ||
|
||
const getTransactionName = (routes: RouteObject[], location: Location, matchRoutes: MatchRoutes): string => { | ||
if (!routes || routes.length === 0 || !matchRoutes) { | ||
return location.pathname; | ||
} | ||
|
||
const branches = matchRoutes(routes, location); | ||
|
||
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; | ||
} | ||
} | ||
} | ||
|
||
return location.pathname; | ||
}; | ||
|
||
export function withSentryReactRouterV6Routing<P extends Record<string, any>, R extends React.FC<P>>(Routes: R): R { | ||
if ( | ||
!_useEffect || | ||
!_useLocation || | ||
!_useNavigationType || | ||
!_createRoutesFromChildren || | ||
!_matchRoutes || | ||
!_customStartTransaction | ||
) { | ||
IS_DEBUG_BUILD && | ||
logger.warn('reactRouterV6Instrumentation was unable to wrap Routes because of one or more missing parameters.'); | ||
|
||
return Routes; | ||
} | ||
|
||
let isBaseLocation: boolean = false; | ||
let routes: RouteObject[]; | ||
|
||
const SentryRoutes: React.FC<P> = (props: P) => { | ||
const location = _useLocation(); | ||
const navigationType = _useNavigationType(); | ||
|
||
_useEffect(() => { | ||
// Performance concern: | ||
// This is repeated when <Routes /> is rendered. | ||
routes = _createRoutesFromChildren(props.children); | ||
isBaseLocation = true; | ||
|
||
if (activeTransaction) { | ||
activeTransaction.setName(getTransactionName(routes, location, _matchRoutes)); | ||
} | ||
|
||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
}, [props.children]); | ||
|
||
_useEffect(() => { | ||
if (isBaseLocation) { | ||
if (activeTransaction) { | ||
activeTransaction.finish(); | ||
} | ||
|
||
return; | ||
} | ||
|
||
if (_startTransactionOnLocationChange && (navigationType === 'PUSH' || navigationType === 'POP')) { | ||
if (activeTransaction) { | ||
activeTransaction.finish(); | ||
} | ||
|
||
activeTransaction = _customStartTransaction({ | ||
name: getTransactionName(routes, location, _matchRoutes), | ||
op: 'navigation', | ||
tags: SENTRY_TAGS, | ||
}); | ||
} | ||
}, [props.children, location, navigationType, isBaseLocation]); | ||
|
||
isBaseLocation = false; | ||
|
||
// @ts-ignore Setting more specific React Component typing for `R` generic above | ||
// will break advanced type inference done by react router params | ||
return <Routes {...props} />; | ||
}; | ||
|
||
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; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,190 @@ | ||
import { render } from '@testing-library/react'; | ||
import * as React from 'react'; | ||
import { | ||
createRoutesFromChildren, | ||
matchPath, | ||
matchRoutes, | ||
MemoryRouter, | ||
Navigate, | ||
Route, | ||
Routes, | ||
useLocation, | ||
useNavigationType, | ||
} from 'react-router-6'; | ||
|
||
import { reactRouterV6Instrumentation } from '../src'; | ||
import { withSentryReactRouterV6Routing } 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( | ||
React.useEffect, | ||
useLocation, | ||
useNavigationType, | ||
createRoutesFromChildren, | ||
matchRoutes, | ||
)(mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange); | ||
return [mockStartTransaction, { mockSetName, mockFinish }]; | ||
} | ||
|
||
it('starts a pageload transaction', () => { | ||
const [mockStartTransaction] = createInstrumentation(); | ||
const SentryRoutes = withSentryReactRouterV6Routing(Routes); | ||
|
||
render( | ||
<MemoryRouter initialEntries={['/']}> | ||
<SentryRoutes> | ||
<Route path="/" element={<div>Home</div>} /> | ||
</SentryRoutes> | ||
</MemoryRouter>, | ||
); | ||
|
||
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 = withSentryReactRouterV6Routing(Routes); | ||
|
||
render( | ||
<MemoryRouter initialEntries={['/']}> | ||
<SentryRoutes> | ||
<Route path="/" element={<div>Home</div>} /> | ||
</SentryRoutes> | ||
</MemoryRouter>, | ||
); | ||
|
||
expect(mockStartTransaction).toHaveBeenCalledTimes(0); | ||
}); | ||
|
||
it('skips navigation transaction, with `startTransactionOnLocationChange: false`', () => { | ||
const [mockStartTransaction] = createInstrumentation({ startTransactionOnLocationChange: false }); | ||
const SentryRoutes = withSentryReactRouterV6Routing(Routes); | ||
|
||
render( | ||
<MemoryRouter initialEntries={['/']}> | ||
<SentryRoutes> | ||
<Route path="/about" element={<div>About</div>} /> | ||
<Route path="/" element={<Navigate to="/about" />} /> | ||
</SentryRoutes> | ||
</MemoryRouter>, | ||
); | ||
|
||
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 = withSentryReactRouterV6Routing(Routes); | ||
|
||
render( | ||
<MemoryRouter initialEntries={['/']}> | ||
<SentryRoutes> | ||
<Route path="/about" element={<div>About</div>} /> | ||
<Route path="/" element={<Navigate to="/about" />} /> | ||
</SentryRoutes> | ||
</MemoryRouter>, | ||
); | ||
|
||
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 = withSentryReactRouterV6Routing(Routes); | ||
|
||
render( | ||
<MemoryRouter initialEntries={['/']}> | ||
<SentryRoutes> | ||
<Route path="/about" element={<div>About</div>}> | ||
<Route path="/about/us" element={<div>us</div>} /> | ||
</Route> | ||
<Route path="/" element={<Navigate to="/about/us" />} /> | ||
</SentryRoutes> | ||
</MemoryRouter>, | ||
); | ||
|
||
expect(mockStartTransaction).toHaveBeenCalledTimes(2); | ||
expect(mockStartTransaction).toHaveBeenLastCalledWith({ | ||
name: '/about/us', | ||
op: 'navigation', | ||
tags: { 'routing.instrumentation': 'react-router-v6' }, | ||
}); | ||
}); | ||
|
||
it('works with paramaterized paths', () => { | ||
const [mockStartTransaction] = createInstrumentation(); | ||
const SentryRoutes = withSentryReactRouterV6Routing(Routes); | ||
|
||
render( | ||
<MemoryRouter initialEntries={['/']}> | ||
<SentryRoutes> | ||
<Route path="/about" element={<div>About</div>}> | ||
<Route path="/about/:page" element={<div>page</div>} /> | ||
</Route> | ||
<Route path="/" element={<Navigate to="/about/us" />} /> | ||
</SentryRoutes> | ||
</MemoryRouter>, | ||
); | ||
|
||
expect(mockStartTransaction).toHaveBeenCalledTimes(2); | ||
expect(mockStartTransaction).toHaveBeenLastCalledWith({ | ||
name: '/about/:page', | ||
op: 'navigation', | ||
tags: { 'routing.instrumentation': 'react-router-v6' }, | ||
}); | ||
}); | ||
|
||
it('works with paths with multiple parameters', () => { | ||
const [mockStartTransaction] = createInstrumentation(); | ||
const SentryRoutes = withSentryReactRouterV6Routing(Routes); | ||
|
||
render( | ||
<MemoryRouter initialEntries={['/']}> | ||
<SentryRoutes> | ||
<Route path="/stores" element={<div>Stores</div>}> | ||
<Route path="/stores/:storeId" element={<div>Store</div>}> | ||
<Route path="/stores/:storeId/products/:productId" element={<div>Product</div>} /> | ||
</Route> | ||
</Route> | ||
<Route path="/" element={<Navigate to="/stores/foo/products/234" />} /> | ||
</SentryRoutes> | ||
</MemoryRouter>, | ||
); | ||
|
||
expect(mockStartTransaction).toHaveBeenCalledTimes(2); | ||
expect(mockStartTransaction).toHaveBeenLastCalledWith({ | ||
name: '/stores/:storeId/products/:productId', | ||
op: 'navigation', | ||
tags: { 'routing.instrumentation': 'react-router-v6' }, | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make sure we attribute to whom this implementation was inspired/based on?