Skip to content

feat(v8/react): Delete react router exports #10532

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

Merged
merged 4 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as Sentry from '@sentry/react';
import { BrowserTracing } from '@sentry/tracing';
import React from 'react';
import ReactDOM from 'react-dom/client';
import {
Expand All @@ -18,14 +17,12 @@ Sentry.init({
environment: 'qa', // dynamic sampling bias to keep transactions
dsn: process.env.REACT_APP_E2E_TEST_DSN,
integrations: [
new BrowserTracing({
routingInstrumentation: Sentry.reactRouterV6Instrumentation(
React.useEffect,
useLocation,
useNavigationType,
createRoutesFromChildren,
matchRoutes,
),
Sentry.reactRouterV6BrowserTracingIntegration({
useEffect: React.useEffect,
useLocation,
useNavigationType,
createRoutesFromChildren,
matchRoutes,
}),
],
// We recommend adjusting this value in production, or using tracesSampler
Expand Down
12 changes: 1 addition & 11 deletions packages/react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,13 @@ export { Profiler, withProfiler, useProfiler } from './profiler';
export type { ErrorBoundaryProps, FallbackRender } from './errorboundary';
export { ErrorBoundary, withErrorBoundary } from './errorboundary';
export { createReduxEnhancer } from './redux';
export { reactRouterV3BrowserTracingIntegration } from './reactrouterv3';
export {
// eslint-disable-next-line deprecation/deprecation
reactRouterV3Instrumentation,
reactRouterV3BrowserTracingIntegration,
} from './reactrouterv3';
export {
// eslint-disable-next-line deprecation/deprecation
reactRouterV4Instrumentation,
// eslint-disable-next-line deprecation/deprecation
reactRouterV5Instrumentation,
withSentryRouting,
reactRouterV4BrowserTracingIntegration,
reactRouterV5BrowserTracingIntegration,
} from './reactrouter';
export {
// eslint-disable-next-line deprecation/deprecation
reactRouterV6Instrumentation,
reactRouterV6BrowserTracingIntegration,
withSentryReactRouterV6Routing,
wrapUseRoutes,
Expand Down
28 changes: 2 additions & 26 deletions packages/react/src/reactrouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ export function reactRouterV4BrowserTracingIntegration(
return undefined;
};

// eslint-disable-next-line deprecation/deprecation
const instrumentation = reactRouterV4Instrumentation(history, routes, matchPath);
const instrumentation = createReactRouterInstrumentation(history, 'reactrouter_v4', routes, matchPath);

// Now instrument page load & navigation with correct settings
instrumentation(startPageloadCallback, instrumentPageLoad, false);
Expand Down Expand Up @@ -115,8 +114,7 @@ export function reactRouterV5BrowserTracingIntegration(
return undefined;
};

// eslint-disable-next-line deprecation/deprecation
const instrumentation = reactRouterV5Instrumentation(history, routes, matchPath);
const instrumentation = createReactRouterInstrumentation(history, 'reactrouter_v5', routes, matchPath);

// Now instrument page load & navigation with correct settings
instrumentation(startPageloadCallback, options.instrumentPageLoad, false);
Expand All @@ -125,28 +123,6 @@ export function reactRouterV5BrowserTracingIntegration(
};
}

/**
* @deprecated Use `browserTracingReactRouterV4()` instead.
*/
export function reactRouterV4Instrumentation(
history: RouterHistory,
routes?: RouteConfig[],
matchPath?: MatchPath,
): ReactRouterInstrumentation {
return createReactRouterInstrumentation(history, 'reactrouter_v4', routes, matchPath);
}

/**
* @deprecated Use `browserTracingReactRouterV5()` instead.
*/
export function reactRouterV5Instrumentation(
history: RouterHistory,
routes?: RouteConfig[],
matchPath?: MatchPath,
): ReactRouterInstrumentation {
return createReactRouterInstrumentation(history, 'reactrouter_v5', routes, matchPath);
}

function createReactRouterInstrumentation(
history: RouterHistory,
instrumentationName: string,
Expand Down
9 changes: 1 addition & 8 deletions packages/react/src/reactrouterv3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ export function reactRouterV3BrowserTracingIntegration(
return undefined;
};

// eslint-disable-next-line deprecation/deprecation
const instrumentation = reactRouterV3Instrumentation(history, routes, match);

// Now instrument page load & navigation with correct settings
Expand All @@ -91,14 +90,8 @@ export function reactRouterV3BrowserTracingIntegration(
* @param history object from the `history` library
* @param routes a list of all routes, should be
* @param match `Router.match` utility
*
* @deprecated Use `reactRouterV3BrowserTracingIntegration()` instead
*/
export function reactRouterV3Instrumentation(
history: HistoryV3,
routes: Route[],
match: Match,
): ReactRouterInstrumentation {
function reactRouterV3Instrumentation(history: HistoryV3, routes: Route[], match: Match): ReactRouterInstrumentation {
return (
startTransaction: (context: TransactionContext) => Transaction | undefined,
startTransactionOnPageLoad: boolean = true,
Expand Down
214 changes: 0 additions & 214 deletions packages/react/test/reactrouterv3.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { IndexRoute, Route, Router, createMemoryHistory, createRoutes, match } f

import type { Match, Route as RouteType } from '../src/reactrouterv3';
import { reactRouterV3BrowserTracingIntegration } from '../src/reactrouterv3';
import { reactRouterV3Instrumentation } from '../src/reactrouterv3';

// Have to manually set types because we are using package-alias
declare module 'react-router-3' {
Expand All @@ -26,219 +25,6 @@ declare module 'react-router-3' {
export const createRoutes: (routes: any) => RouteType[];
}

function createMockStartTransaction(opts: { finish?: jest.FunctionLike; setMetadata?: jest.FunctionLike } = {}) {
const { finish = jest.fn(), setMetadata = jest.fn() } = opts;
return jest.fn().mockReturnValue({
end: finish,
setMetadata,
});
}

describe('reactRouterV3Instrumentation', () => {
const routes = (
<Route path="/" component={({ children }: { children: JSX.Element }) => <div>{children}</div>}>
<IndexRoute component={() => <div>Home</div>} />
<Route path="about" component={() => <div>About</div>} />
<Route path="features" component={() => <div>Features</div>} />
<Route
path="users/:userid"
component={({ params }: { params: Record<string, string> }) => <div>{params.userid}</div>}
/>
<Route path="organizations/">
<Route path=":orgid" component={() => <div>OrgId</div>} />
<Route path=":orgid/v1/:teamid" component={() => <div>Team</div>} />
</Route>
</Route>
);
const history = createMemoryHistory();

const instrumentationRoutes = createRoutes(routes);
// eslint-disable-next-line deprecation/deprecation
const instrumentation = reactRouterV3Instrumentation(history, instrumentationRoutes, match);

it('starts a pageload transaction when instrumentation is started', () => {
const mockStartTransaction = createMockStartTransaction();
instrumentation(mockStartTransaction);
expect(mockStartTransaction).toHaveBeenCalledTimes(1);
expect(mockStartTransaction).toHaveBeenLastCalledWith({
name: '/',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.react.reactrouter_v3',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
},
});
});

it('does not start pageload transaction if option is false', () => {
const mockStartTransaction = createMockStartTransaction();
instrumentation(mockStartTransaction, false);
expect(mockStartTransaction).toHaveBeenCalledTimes(0);
});

it('starts a navigation transaction', () => {
const mockStartTransaction = createMockStartTransaction();
instrumentation(mockStartTransaction);
render(<Router history={history}>{routes}</Router>);

act(() => {
history.push('/about');
});
expect(mockStartTransaction).toHaveBeenCalledTimes(2);
expect(mockStartTransaction).toHaveBeenLastCalledWith({
name: '/about',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
},
});

act(() => {
history.push('/features');
});
expect(mockStartTransaction).toHaveBeenCalledTimes(3);
expect(mockStartTransaction).toHaveBeenLastCalledWith({
name: '/features',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
},
});
});

it('does not start a transaction if option is false', () => {
const mockStartTransaction = createMockStartTransaction();
instrumentation(mockStartTransaction, true, false);
render(<Router history={history}>{routes}</Router>);
expect(mockStartTransaction).toHaveBeenCalledTimes(1);
});

it('only starts a navigation transaction on push', () => {
const mockStartTransaction = createMockStartTransaction();
instrumentation(mockStartTransaction);
render(<Router history={history}>{routes}</Router>);

act(() => {
history.replace('hello');
});
expect(mockStartTransaction).toHaveBeenCalledTimes(1);
});

it('finishes a transaction on navigation', () => {
const mockFinish = jest.fn();
const mockStartTransaction = createMockStartTransaction({ finish: mockFinish });
instrumentation(mockStartTransaction);
render(<Router history={history}>{routes}</Router>);
expect(mockStartTransaction).toHaveBeenCalledTimes(1);

act(() => {
history.push('/features');
});
expect(mockFinish).toHaveBeenCalledTimes(1);
expect(mockStartTransaction).toHaveBeenCalledTimes(2);
});

it('normalizes transaction names', () => {
const mockStartTransaction = createMockStartTransaction();
instrumentation(mockStartTransaction);
const { container } = render(<Router history={history}>{routes}</Router>);

act(() => {
history.push('/users/123');
});
expect(container.innerHTML).toContain('123');

expect(mockStartTransaction).toHaveBeenCalledTimes(2);
expect(mockStartTransaction).toHaveBeenLastCalledWith({
name: '/users/:userid',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
},
});
});

it('normalizes nested transaction names', () => {
const mockStartTransaction = createMockStartTransaction();
instrumentation(mockStartTransaction);
const { container } = render(<Router history={history}>{routes}</Router>);

act(() => {
history.push('/organizations/1234/v1/758');
});
expect(container.innerHTML).toContain('Team');

expect(mockStartTransaction).toHaveBeenCalledTimes(2);
expect(mockStartTransaction).toHaveBeenLastCalledWith({
name: '/organizations/:orgid/v1/:teamid',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
},
});

act(() => {
history.push('/organizations/543');
});
expect(container.innerHTML).toContain('OrgId');

expect(mockStartTransaction).toHaveBeenCalledTimes(3);
expect(mockStartTransaction).toHaveBeenLastCalledWith({
name: '/organizations/:orgid',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
},
});
});

it('sets metadata to url if on an unknown route', () => {
const mockStartTransaction = createMockStartTransaction();
instrumentation(mockStartTransaction);
render(<Router history={history}>{routes}</Router>);

act(() => {
history.push('/organizations/1234/some/other/route');
});

expect(mockStartTransaction).toHaveBeenCalledTimes(2);
expect(mockStartTransaction).toHaveBeenLastCalledWith({
name: '/organizations/1234/some/other/route',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
},
});
});

it('sets metadata to url if no routes are provided', () => {
const fakeRoutes = <div>hello</div>;
const mockStartTransaction = createMockStartTransaction();
// eslint-disable-next-line deprecation/deprecation
const mockInstrumentation = reactRouterV3Instrumentation(history, createRoutes(fakeRoutes), match);
mockInstrumentation(mockStartTransaction);
// We render here with `routes` instead of `fakeRoutes` from above to validate the case
// where users provided the instrumentation with a bad set of routes.
render(<Router history={history}>{routes}</Router>);

expect(mockStartTransaction).toHaveBeenCalledTimes(1);
expect(mockStartTransaction).toHaveBeenLastCalledWith({
name: '/',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.react.reactrouter_v3',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
},
});
});
});

const mockStartBrowserTracingPageLoadSpan = jest.fn();
const mockStartBrowserTracingNavigationSpan = jest.fn();

Expand Down
Loading