diff --git a/packages/react/package.json b/packages/react/package.json index 3a12c357f943..7a762d859f44 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -28,13 +28,10 @@ "react-dom": "15.x || 16.x" }, "devDependencies": { - "@sentry/tracing": "5.19.2", "@testing-library/react": "^10.0.6", "@testing-library/react-hooks": "^3.3.0", - "@types/history": "^4.7.6", "@types/hoist-non-react-statics": "^3.3.1", "@types/react": "^16.9.35", - "@types/react-router-3": "npm:@types/react-router@3.0.20", "jest": "^24.7.1", "jsdom": "^16.2.2", "npm-run-all": "^4.1.2", @@ -42,7 +39,7 @@ "prettier-check": "^2.0.0", "react": "^16.0.0", "react-dom": "^16.0.0", - "react-router-3": "npm:react-router@3.2.0", + "react-router-3": "npm:react-router@^3.2.0", "react-test-renderer": "^16.13.1", "redux": "^4.0.5", "rimraf": "^2.6.3", diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index 7a958f30b828..22ce1ce6e8d8 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -26,6 +26,6 @@ export * from '@sentry/browser'; export { Profiler, withProfiler, useProfiler } from './profiler'; export { ErrorBoundary, withErrorBoundary } from './errorboundary'; export { createReduxEnhancer } from './redux'; -export { reactRouterV3Instrumenation } from './reactrouter'; +export { reactRouterV3Instrumentation } from './reactrouter'; createReactEventProcessor(); diff --git a/packages/react/src/reactrouter.tsx b/packages/react/src/reactrouter.tsx index d8441a2e78bf..86ee8302ce9b 100644 --- a/packages/react/src/reactrouter.tsx +++ b/packages/react/src/reactrouter.tsx @@ -1,70 +1,81 @@ import { Transaction, TransactionContext } from '@sentry/types'; +import { getGlobalObject } from '@sentry/utils'; -type routingInstrumentation = ( - startTransaction: (context: TransactionContext) => Transaction, +type ReactRouterInstrumentation = ( + startTransaction: (context: TransactionContext) => T | undefined, startTransactionOnPageLoad?: boolean, startTransactionOnLocationChange?: boolean, ) => void; -// Many of the types below had to be mocked out to lower bundle size. -type PlainRoute = { path: string; childRoutes: PlainRoute[] }; +// Many of the types below had to be mocked out to prevent typescript issues +// these types are required for correct functionality. -type Match = ( - props: { location: Location; routes: PlainRoute[] }, - cb: (error?: Error, _redirectLocation?: Location, renderProps?: { routes?: PlainRoute[] }) => void, +export type Route = { path?: string; childRoutes?: Route[] }; + +export type Match = ( + props: { location: Location; routes: Route[] }, + cb: (error?: Error, _redirectLocation?: Location, renderProps?: { routes?: Route[] }) => void, ) => void; type Location = { pathname: string; - action: 'PUSH' | 'REPLACE' | 'POP'; - key: string; + action?: 'PUSH' | 'REPLACE' | 'POP'; } & Record; type History = { - location: Location; - listen(cb: (location: Location) => void): void; + location?: Location; + listen?(cb: (location: Location) => void): void; } & Record; +const global = getGlobalObject(); + /** * Creates routing instrumentation for React Router v3 + * Works for React Router >= 3.2.0 and < 4.0.0 * * @param history object from the `history` library * @param routes a list of all routes, should be * @param match `Router.match` utility */ -export function reactRouterV3Instrumenation( +export function reactRouterV3Instrumentation( history: History, - routes: PlainRoute[], + routes: Route[], match: Match, -): routingInstrumentation { +): ReactRouterInstrumentation { return ( startTransaction: (context: TransactionContext) => Transaction | undefined, startTransactionOnPageLoad: boolean = true, startTransactionOnLocationChange: boolean = true, ) => { let activeTransaction: Transaction | undefined; - let name = normalizeTransactionName(routes, history.location, match); - if (startTransactionOnPageLoad) { + let prevName: string | undefined; + + if (startTransactionOnPageLoad && global && global.location) { + // Have to use global.location because history.location might not be defined. + prevName = normalizeTransactionName(routes, global.location, match); activeTransaction = startTransaction({ - name, + name: prevName, op: 'pageload', tags: { - from: name, - routingInstrumentation: 'react-router-v3', + 'routing.instrumentation': 'react-router-v3', }, }); } - if (startTransactionOnLocationChange) { + if (startTransactionOnLocationChange && history.listen) { history.listen(location => { if (location.action === 'PUSH') { if (activeTransaction) { activeTransaction.finish(); } - const tags = { from: name, routingInstrumentation: 'react-router-v3' }; - name = normalizeTransactionName(routes, history.location, match); + const tags: Record = { 'routing.instrumentation': 'react-router-v3' }; + if (prevName) { + tags.from = prevName; + } + + prevName = normalizeTransactionName(routes, location, match); activeTransaction = startTransaction({ - name, + name: prevName, op: 'navigation', tags, }); @@ -74,41 +85,47 @@ export function reactRouterV3Instrumenation( }; } -export function normalizeTransactionName(appRoutes: PlainRoute[], location: Location, match: Match): string { - const defaultName = location.pathname; +/** + * Normalize transaction names using `Router.match` + */ +function normalizeTransactionName(appRoutes: Route[], location: Location, match: Match): string { + let name = location.pathname; match( { location, routes: appRoutes, }, - (error?: Error, _redirectLocation?: Location, renderProps?: { routes?: PlainRoute[] }) => { + (error, _redirectLocation, renderProps) => { if (error || !renderProps) { - return defaultName; + return name; } const routePath = getRouteStringFromRoutes(renderProps.routes || []); - if (routePath.length === 0 || routePath === '/*') { - return defaultName; + return name; } - return routePath; + name = routePath; + return name; }, ); - - return defaultName; + return name; } -function getRouteStringFromRoutes(routes: PlainRoute[]): string { - if (!Array.isArray(routes)) { +/** + * Generate route name from array of routes + */ +function getRouteStringFromRoutes(routes: Route[]): string { + if (!Array.isArray(routes) || routes.length === 0) { return ''; } - const routesWithPaths: PlainRoute[] = routes.filter((route: PlainRoute) => !!route.path); + const routesWithPaths: Route[] = routes.filter((route: Route) => !!route.path); let index = -1; - for (let x = routesWithPaths.length; x >= 0; x--) { - if (routesWithPaths[x].path.startsWith('/')) { + for (let x = routesWithPaths.length - 1; x >= 0; x--) { + const route = routesWithPaths[x]; + if (route.path && route.path.startsWith('/')) { index = x; break; } diff --git a/packages/react/test/reactrouter.test.tsx b/packages/react/test/reactrouter.test.tsx deleted file mode 100644 index b3d16b412ff2..000000000000 --- a/packages/react/test/reactrouter.test.tsx +++ /dev/null @@ -1,6 +0,0 @@ -import { browserHistory } from 'react-router-3'; -import { routes } from './routes'; - -describe('React Router V3', () => { - it('works', () => {}); -}); diff --git a/packages/react/test/reactrouterv3.test.tsx b/packages/react/test/reactrouterv3.test.tsx new file mode 100644 index 000000000000..17328229bf49 --- /dev/null +++ b/packages/react/test/reactrouterv3.test.tsx @@ -0,0 +1,131 @@ +import { render } from '@testing-library/react'; +import * as React from 'react'; +import { createMemoryHistory, createRoutes, IndexRoute, match, Route, Router } from 'react-router-3'; + +import { Match, reactRouterV3Instrumentation, Route as RouteType } from '../src/reactrouter'; + +// Have to manually set types because we are using package-alias +declare module 'react-router-3' { + export function createMemoryHistory(): Record; + export const Router: React.ComponentType<{ history: any }>; + export const Route: React.ComponentType<{ path: string; component: React.ComponentType }>; + export const IndexRoute: React.ComponentType<{ component: React.ComponentType }>; + export const match: Match; + export const createRoutes: (routes: any) => RouteType[]; +} + +const App: React.FC = ({ children }) =>
{children}
; + +function Home(): JSX.Element { + return
Home
; +} + +function About(): JSX.Element { + return
About
; +} + +function Features(): JSX.Element { + return
Features
; +} + +function Users({ params }: { params: Record }): JSX.Element { + return
{params.userid}
; +} + +describe('React Router V3', () => { + const routes = ( + + + + + + + ); + const history = createMemoryHistory(); + + const instrumentationRoutes = createRoutes(routes); + const instrumentation = reactRouterV3Instrumentation(history, instrumentationRoutes, match); + + it('starts a pageload transaction when instrumentation is started', () => { + const mockStartTransaction = jest.fn(); + instrumentation(mockStartTransaction); + expect(mockStartTransaction).toHaveBeenCalledTimes(1); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/', + op: 'pageload', + tags: { 'routing.instrumentation': 'react-router-v3' }, + }); + }); + + it('does not start pageload transaction if option is false', () => { + const mockStartTransaction = jest.fn(); + instrumentation(mockStartTransaction, false); + expect(mockStartTransaction).toHaveBeenCalledTimes(0); + }); + + it('starts a navigation transaction', () => { + const mockStartTransaction = jest.fn(); + instrumentation(mockStartTransaction); + render({routes}); + + history.push('/about'); + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/about', + op: 'navigation', + tags: { from: '/', 'routing.instrumentation': 'react-router-v3' }, + }); + + history.push('/features'); + expect(mockStartTransaction).toHaveBeenCalledTimes(3); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/features', + op: 'navigation', + tags: { from: '/about', 'routing.instrumentation': 'react-router-v3' }, + }); + }); + + it('does not start a transaction if option is false', () => { + const mockStartTransaction = jest.fn(); + instrumentation(mockStartTransaction, true, false); + render({routes}); + expect(mockStartTransaction).toHaveBeenCalledTimes(1); + }); + + it('only starts a navigation transaction on push', () => { + const mockStartTransaction = jest.fn(); + instrumentation(mockStartTransaction); + render({routes}); + + history.replace('hello'); + expect(mockStartTransaction).toHaveBeenCalledTimes(1); + }); + + it('finishes a transaction on navigation', () => { + const mockFinish = jest.fn(); + const mockStartTransaction = jest.fn().mockReturnValue({ finish: mockFinish }); + instrumentation(mockStartTransaction); + render({routes}); + expect(mockStartTransaction).toHaveBeenCalledTimes(1); + + history.push('/features'); + expect(mockFinish).toHaveBeenCalledTimes(1); + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + }); + + it('normalizes transaction names', () => { + const mockStartTransaction = jest.fn(); + instrumentation(mockStartTransaction); + const { container } = render({routes}); + + history.push('/users/123'); + expect(container.innerHTML).toContain('123'); + + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/users/:userid', + op: 'navigation', + tags: { from: '/', 'routing.instrumentation': 'react-router-v3' }, + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index 8d9c864740cd..82d58cd3ab97 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1458,16 +1458,6 @@ resolved "https://registry.yarnpkg.com/@types/highlight.js/-/highlight.js-9.12.4.tgz#8c3496bd1b50cc04aeefd691140aa571d4dbfa34" integrity sha512-t2szdkwmg2JJyuCM20e8kR2X59WCE5Zkl4bzm1u1Oukjm79zpbiAv+QjnwLnuuV0WHEcX2NgUItu0pAMKuOPww== -"@types/history@*", "@types/history@^4.7.6": - version "4.7.6" - resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.6.tgz#ed8fc802c45b8e8f54419c2d054e55c9ea344356" - integrity sha512-GRTZLeLJ8ia00ZH8mxMO8t0aC9M1N9bN461Z2eaRurJo6Fpa+utgCwLzI4jQHcrdzuzp5WPN9jRwpsCQ1VhJ5w== - -"@types/history@^3": - version "3.2.4" - resolved "https://registry.yarnpkg.com/@types/history/-/history-3.2.4.tgz#0b6c62240d1fac020853aa5608758991d9f6ef3d" - integrity sha512-q7x8QeCRk2T6DR2UznwYW//mpN5uNlyajkewH2xd1s1ozCS4oHFRg2WMusxwLFlE57EkUYsd/gCapLBYzV3ffg== - "@types/hoist-non-react-statics@^3.3.1": version "3.3.1" resolved "https://registry.yarnpkg.com/@types/hoist-non-react-statics/-/hoist-non-react-statics-3.3.1.tgz#1124aafe5118cb591977aeb1ceaaed1070eb039f" @@ -1604,22 +1594,6 @@ resolved "https://registry.yarnpkg.com/@types/range-parser/-/range-parser-1.2.3.tgz#7ee330ba7caafb98090bece86a5ee44115904c2c" integrity sha512-ewFXqrQHlFsgc09MK5jP5iR7vumV/BYayNC6PgJO2LPe8vrnNFyjQjSppfEngITi0qvfKtzFvgKymGheFM9UOA== -"@types/react-router-3@npm:@types/react-router@3.0.20": - version "3.0.20" - resolved "https://registry.yarnpkg.com/@types/react-router/-/react-router-3.0.20.tgz#a711682475ccef70ad9ad9e459859380221e6ee6" - integrity sha512-0sx2ThGYgblXPf8we/c+umFzP3RCbBp1bbFmd3pO1UaOYnTDno82iql3MQTVqB09rhopKORNfakDU/9xZ4QR6g== - dependencies: - "@types/history" "^3" - "@types/react" "*" - -"@types/react-router@^5.1.8": - version "5.1.8" - resolved "https://registry.yarnpkg.com/@types/react-router/-/react-router-5.1.8.tgz#4614e5ba7559657438e17766bb95ef6ed6acc3fa" - integrity sha512-HzOyJb+wFmyEhyfp4D4NYrumi+LQgQL/68HvJO+q6XtuHSDvw6Aqov7sCAhjbNq3bUPgPqbdvjXC5HeB2oEAPg== - dependencies: - "@types/history" "*" - "@types/react" "*" - "@types/react-test-renderer@*": version "16.9.2" resolved "https://registry.yarnpkg.com/@types/react-test-renderer/-/react-test-renderer-16.9.2.tgz#e1c408831e8183e5ad748fdece02214a7c2ab6c5" @@ -1968,32 +1942,11 @@ after@0.8.2: resolved "https://registry.yarnpkg.com/after/-/after-0.8.2.tgz#fedb394f9f0e02aa9768e702bda23b505fae7e1f" integrity sha1-/ts5T58OAqqXaOcCvaI7UF+ufh8= -agent-base@4, agent-base@^4.3.0: - version "4.3.0" - resolved "https://registry.yarnpkg.com/agent-base/-/agent-base-4.3.0.tgz#8165f01c436009bccad0b1d122f05ed770efc6ee" - integrity sha512-salcGninV0nPrwpGNn4VTXBb1SOuXQBiqbrNXoeizJsHrsL6ERFM2Ne3JUSBWRE6aeNJI2ROP/WEEIDUiDe3cg== - dependencies: - es6-promisify "^5.0.0" - -agent-base@5: +agent-base@4, agent-base@5, agent-base@6, agent-base@^4.3.0, agent-base@~4.2.1: version "5.1.1" resolved "https://registry.yarnpkg.com/agent-base/-/agent-base-5.1.1.tgz#e8fb3f242959db44d63be665db7a8e739537a32c" integrity sha512-TMeqbNl2fMW0nMjTEPOwe3J/PRFP4vqeoNuQMG0HlMrtm5QxKqdvAkZ1pRBQ/ulIyDD5Yq0nJ7YbdD8ey0TO3g== -agent-base@6: - version "6.0.1" - resolved "https://registry.yarnpkg.com/agent-base/-/agent-base-6.0.1.tgz#808007e4e5867decb0ab6ab2f928fbdb5a596db4" - integrity sha512-01q25QQDwLSsyfhrKbn8yuur+JNw0H+0Y4JiGIKd3z9aYk/w/2kxD/Upc+t2ZBBSUNff50VjPsSW2YxM8QYKVg== - dependencies: - debug "4" - -agent-base@~4.2.1: - version "4.2.1" - resolved "https://registry.yarnpkg.com/agent-base/-/agent-base-4.2.1.tgz#d89e5999f797875674c07d87f260fc41e83e8ca9" - integrity sha512-JVwXMr9nHYTUXsBFKUqhJwvlcYU/blreOEUkhNR2eXZIvwd+c+o5V4MgDPKWnMS/56awN3TRzIP+KoPn+roQtg== - dependencies: - es6-promisify "^5.0.0" - agentkeepalive@^3.4.1: version "3.5.2" resolved "https://registry.yarnpkg.com/agentkeepalive/-/agentkeepalive-3.5.2.tgz#a113924dd3fa24a0bc3b78108c450c2abee00f67" @@ -5183,18 +5136,6 @@ es6-object-assign@^1.1.0: resolved "https://registry.yarnpkg.com/es6-object-assign/-/es6-object-assign-1.1.0.tgz#c2c3582656247c39ea107cb1e6652b6f9f24523c" integrity sha1-wsNYJlYkfDnqEHyx5mUrb58kUjw= -es6-promise@^4.0.3: - version "4.2.8" - resolved "https://registry.yarnpkg.com/es6-promise/-/es6-promise-4.2.8.tgz#4eb21594c972bc40553d276e510539143db53e0a" - integrity sha512-HJDGx5daxeIvxdBxvG2cb9g4tEvwIk3i8+nhX0yGrYmZUzbkdg8QbDevheDB8gd0//uPj4c1EQua8Q+MViT0/w== - -es6-promisify@^5.0.0: - version "5.0.0" - resolved "https://registry.yarnpkg.com/es6-promisify/-/es6-promisify-5.0.0.tgz#5109d62f3e56ea967c4b63505aef08291c8a5203" - integrity sha1-UQnWLz5W6pZ8S2NQWu8IKRyKUgM= - dependencies: - es6-promise "^4.0.3" - escalade@^3.0.1: version "3.0.2" resolved "https://registry.yarnpkg.com/escalade/-/escalade-3.0.2.tgz#6a580d70edb87880f22b4c91d0d56078df6962c4" @@ -6417,11 +6358,6 @@ hmac-drbg@^1.0.0: minimalistic-assert "^1.0.0" minimalistic-crypto-utils "^1.0.1" -hoist-non-react-statics@^1.2.0: - version "1.2.0" - resolved "https://registry.yarnpkg.com/hoist-non-react-statics/-/hoist-non-react-statics-1.2.0.tgz#aa448cf0986d55cc40773b17174b7dd066cb7cfb" - integrity sha1-qkSM8JhtVcxAdzsXF0t90GbLfPs= - hoist-non-react-statics@^3.3.0, hoist-non-react-statics@^3.3.2: version "3.3.2" resolved "https://registry.yarnpkg.com/hoist-non-react-statics/-/hoist-non-react-statics-3.3.2.tgz#ece0acaf71d62c2969c2ec59feff42a4b1a85b45" @@ -10623,7 +10559,7 @@ promzard@^0.3.0: dependencies: read "1" -prop-types@^15.5.6, prop-types@^15.6.2: +prop-types@^15.6.2, prop-types@^15.7.2: version "15.7.2" resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.7.2.tgz#52c41e75b8c87e72b9d9360e0206b99dcbffa6c5" integrity sha512-8QQikdH7//R2vurIJSutZ1smHYTcLpRWEOlHnzcWHmBYrOGUysKwSsrC89BCiFj3CbrfJ/nXFdJepOVrY1GCHQ== @@ -10861,22 +10797,23 @@ react-dom@^16.0.0: prop-types "^15.6.2" scheduler "^0.19.1" -react-is@^16.12.0, react-is@^16.7.0, react-is@^16.8.1, react-is@^16.8.4, react-is@^16.8.6: +react-is@^16.12.0, react-is@^16.13.0, react-is@^16.7.0, react-is@^16.8.1, react-is@^16.8.4, react-is@^16.8.6: version "16.13.1" resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.13.1.tgz#789729a4dc36de2999dc156dd6c1d9c18cea56a4" integrity sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ== -"react-router-3@npm:react-router@3.2.0": - version "3.2.0" - resolved "https://registry.yarnpkg.com/react-router/-/react-router-3.2.0.tgz#62b6279d589b70b34e265113e4c0a9261a02ed36" - integrity sha512-sXlLOg0TRCqnjCVskqBHGjzNjcJKUqXEKnDSuxMYJSPJNq9hROE9VsiIW2kfIq7Ev+20Iz0nxayekXyv0XNmsg== +"react-router-3@npm:react-router@^3.2.0": + version "3.2.6" + resolved "https://registry.yarnpkg.com/react-router/-/react-router-3.2.6.tgz#cad202796a7bba3efc2100da453b3379c9d4aeb4" + integrity sha512-nlxtQE8B22hb/JxdaslI1tfZacxFU8x8BJryXOnR2RxB4vc01zuHYAHAIgmBkdk1kzXaA25hZxK6KAH/+CXArw== dependencies: create-react-class "^15.5.1" history "^3.0.0" - hoist-non-react-statics "^1.2.0" + hoist-non-react-statics "^3.3.2" invariant "^2.2.1" loose-envify "^1.2.0" - prop-types "^15.5.6" + prop-types "^15.7.2" + react-is "^16.13.0" warning "^3.0.0" react-test-renderer@^16.13.1: