Skip to content
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

fix(react/v8): Support lazy-loaded routes and components #15281

Open
wants to merge 1 commit into
base: v8
Choose a base branch
from
Open
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,5 @@
import * as Sentry from '@sentry/react';
import React from 'react';
import React, { lazy, Suspense } from 'react';
import ReactDOM from 'react-dom/client';
import {
RouterProvider,
Expand Down Expand Up @@ -42,13 +42,22 @@ Sentry.init({
});

const sentryCreateBrowserRouter = Sentry.wrapCreateBrowserRouterV6(createBrowserRouter);
const LazyLoadedUser = lazy(() => import('./pages/LazyLoadedUser'));

const router = sentryCreateBrowserRouter(
[
{
path: '/',
element: <Index />,
},
{
path: '/lazy-loaded-user/*',
element: (
<Suspense fallback={<div>Loading...</div>}>
<LazyLoadedUser />
</Suspense>
),
},
{
path: '/user/:id',
element: <User />,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ const Index = () => {
<Link to="/user/5" id="navigation">
navigate
</Link>
<Link to="/lazy-loaded-user/5/foo" id="lazy-navigation">
lazy navigate
</Link>
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import * as Sentry from '@sentry/react';
// biome-ignore lint/nursery/noUnusedImports: Need React import for JSX
import * as React from 'react';
import { Route, Routes } from 'react-router-dom';

const SentryRoutes = Sentry.withSentryReactRouterV6Routing(Routes);

const InnerRoute = () => (
<SentryRoutes>
<Route path=":innerId" element={<p id="content">I am a lazy loaded user</p>} />
</SentryRoutes>
);

export default InnerRoute;
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import * as Sentry from '@sentry/react';
import * as React from 'react';
import { Route, Routes } from 'react-router-dom';

const SentryRoutes = Sentry.withSentryReactRouterV6Routing(Routes);
const InnerRoute = React.lazy(() => import('./LazyLoadedInnerRoute'));

const LazyLoadedUser = () => {
return (
<SentryRoutes>
<Route
path=":id/*"
element={
<React.Suspense fallback={<p>Loading...</p>}>
<InnerRoute />
</React.Suspense>
}
/>
</SentryRoutes>
);
};

export default LazyLoadedUser;
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,105 @@ test('Captures a navigation transaction', async ({ page }) => {

expect(transactionEvent.spans).toEqual([]);
});

test('Captures a lazy pageload transaction', async ({ page }) => {
const transactionEventPromise = waitForTransaction('react-create-browser-router', event => {
return event.contexts?.trace?.op === 'pageload';
});

await page.goto('/lazy-loaded-user/5/foo');

const transactionEvent = await transactionEventPromise;
expect(transactionEvent.contexts?.trace).toEqual({
data: expect.objectContaining({
'sentry.idle_span_finish_reason': 'idleTimeout',
'sentry.op': 'pageload',
'sentry.origin': 'auto.pageload.react.reactrouter_v6',
'sentry.sample_rate': 1,
'sentry.source': 'route',
}),
op: 'pageload',
span_id: expect.stringMatching(/[a-f0-9]{16}/),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
origin: 'auto.pageload.react.reactrouter_v6',
});

expect(transactionEvent).toEqual(
expect.objectContaining({
transaction: '/lazy-loaded-user/:id/:innerId',
type: 'transaction',
transaction_info: {
source: 'route',
},
}),
);

expect(await page.innerText('id=content')).toContain('I am a lazy loaded user');

expect(transactionEvent.spans).toEqual(
expect.arrayContaining([
// This one is the outer lazy route
expect.objectContaining({
op: 'resource.script',
origin: 'auto.resource.browser.metrics',
}),
// This one is the inner lazy route
expect.objectContaining({
op: 'resource.script',
origin: 'auto.resource.browser.metrics',
}),
]),
);
});

test('Captures a lazy navigation transaction', async ({ page }) => {
const transactionEventPromise = waitForTransaction('react-create-browser-router', event => {
return event.contexts?.trace?.op === 'navigation';
});

await page.goto('/');
const linkElement = page.locator('id=lazy-navigation');
await linkElement.click();

const transactionEvent = await transactionEventPromise;
expect(transactionEvent.contexts?.trace).toEqual({
data: expect.objectContaining({
'sentry.idle_span_finish_reason': 'idleTimeout',
'sentry.op': 'navigation',
'sentry.origin': 'auto.navigation.react.reactrouter_v6',
'sentry.sample_rate': 1,
'sentry.source': 'route',
}),
op: 'navigation',
span_id: expect.stringMatching(/[a-f0-9]{16}/),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
origin: 'auto.navigation.react.reactrouter_v6',
});

expect(transactionEvent).toEqual(
expect.objectContaining({
transaction: '/lazy-loaded-user/:id/:innerId',
type: 'transaction',
transaction_info: {
source: 'route',
},
}),
);

expect(await page.innerText('id=content')).toContain('I am a lazy loaded user');

expect(transactionEvent.spans).toEqual(
expect.arrayContaining([
// This one is the outer lazy route
expect.objectContaining({
op: 'resource.script',
origin: 'auto.resource.browser.metrics',
}),
// This one is the inner lazy route
expect.objectContaining({
op: 'resource.script',
origin: 'auto.resource.browser.metrics',
}),
]),
);
});
62 changes: 45 additions & 17 deletions packages/react/src/reactrouterv6-compat-utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ export interface ReactRouterOptions {

type V6CompatibleVersion = '6' | '7';

// Keeping as a global variable for cross-usage in multiple functions
const allRoutes = new Set<RouteObject>();

/**
* Creates a wrapCreateBrowserRouter function that can be used with all React Router v6 compatible versions.
*/
Expand All @@ -82,6 +85,10 @@ export function createV6CompatibleWrapCreateBrowserRouter<
}

return function (routes: RouteObject[], opts?: Record<string, unknown> & { basename?: string }): TRouter {
routes.forEach(route => {
allRoutes.add(route);
});

const router = createRouterFunction(routes, opts);
const basename = opts && opts.basename;

Expand All @@ -91,19 +98,40 @@ export function createV6CompatibleWrapCreateBrowserRouter<
// 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' && activeRootSpan) {
updatePageloadTransaction(activeRootSpan, router.state.location, routes, undefined, basename);
updatePageloadTransaction(
activeRootSpan,
router.state.location,
routes,
undefined,
basename,
Array.from(allRoutes),
);
}

router.subscribe((state: RouterState) => {
const location = state.location;
if (state.historyAction === 'PUSH' || state.historyAction === 'POP') {
handleNavigation({
location,
routes,
navigationType: state.historyAction,
version,
basename,
});
// Wait for the next render if loading an unsettled route
if (state.navigation.state !== 'idle') {
requestAnimationFrame(() => {
handleNavigation({
location: state.location,
routes,
navigationType: state.historyAction,
version,
basename,
allRoutes: Array.from(allRoutes),
});
});
} else {
handleNavigation({
location: state.location,
routes,
navigationType: state.historyAction,
version,
basename,
allRoutes: Array.from(allRoutes),
});
}
}
});

Expand Down Expand Up @@ -138,6 +166,10 @@ export function createV6CompatibleWrapCreateMemoryRouter<
initialIndex?: number;
},
): TRouter {
routes.forEach(route => {
allRoutes.add(route);
});

const router = createRouterFunction(routes, opts);
const basename = opts ? opts.basename : undefined;

Expand All @@ -163,7 +195,7 @@ export function createV6CompatibleWrapCreateMemoryRouter<
: router.state.location;

if (router.state.historyAction === 'POP' && activeRootSpan) {
updatePageloadTransaction(activeRootSpan, location, routes, undefined, basename);
updatePageloadTransaction(activeRootSpan, location, routes, undefined, basename, Array.from(allRoutes));
}

router.subscribe((state: RouterState) => {
Expand All @@ -175,6 +207,7 @@ export function createV6CompatibleWrapCreateMemoryRouter<
navigationType: state.historyAction,
version,
basename,
allRoutes: Array.from(allRoutes),
});
}
});
Expand Down Expand Up @@ -249,8 +282,6 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio
return origUseRoutes;
}

const allRoutes: Set<RouteObject> = new Set();

const SentryRoutes: React.FC<{
children?: React.ReactNode;
routes: RouteObject[];
Expand Down Expand Up @@ -322,7 +353,6 @@ export function handleNavigation(opts: {
allRoutes?: RouteObject[];
}): void {
const { location, routes, navigationType, version, matches, basename, allRoutes } = opts;

const branches = Array.isArray(matches) ? matches : _matchRoutes(routes, location, basename);

const client = getClient();
Expand Down Expand Up @@ -558,7 +588,7 @@ function updatePageloadTransaction(
): void {
const branches = Array.isArray(matches)
? matches
: (_matchRoutes(routes, location, basename) as unknown as RouteMatch[]);
: (_matchRoutes(allRoutes || routes, location, basename) as unknown as RouteMatch[]);

if (branches) {
let name,
Expand All @@ -574,7 +604,7 @@ function updatePageloadTransaction(
[name, source] = getNormalizedName(routes, location, branches, basename);
}

getCurrentScope().setTransactionName(name);
getCurrentScope().setTransactionName(name || '/');

if (activeRootSpan) {
activeRootSpan.updateName(name);
Expand All @@ -597,8 +627,6 @@ export function createV6CompatibleWithSentryReactRouterRouting<P extends Record<
return Routes;
}

const allRoutes: Set<RouteObject> = new Set();

const SentryRoutes: React.FC<P> = (props: P) => {
const isMountRenderPass = React.useRef(true);

Expand Down
11 changes: 8 additions & 3 deletions packages/react/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,14 @@ export interface RouterInit {
hydrationData?: HydrationState;
}

export type NavigationState = {
state: 'idle' | 'loading' | 'submitting';
};

export type NavigationStates = {
Idle: any;
Loading: any;
Submitting: any;
Idle: NavigationState;
Loading: NavigationState;
Submitting: NavigationState;
};

export type Navigation = NavigationStates[keyof NavigationStates];
Expand All @@ -202,6 +206,7 @@ export declare enum HistoryAction {
export interface RouterState {
historyAction: Action | HistoryAction | any;
location: Location;
navigation: Navigation;
}
export interface Router<TState extends RouterState = RouterState> {
state: TState;
Expand Down
Loading