Skip to content

fix(react): Support wildcard routes on React Router 6 #14205

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.

Sign up for GitHub

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 1 commit into from
Nov 11, 2024
Merged

fix(react): Support wildcard routes on React Router 6 #14205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
38 changes: 35 additions & 3 deletions packages/react/src/reactrouterv6.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable max-lines */
// Inspired from Donnie McNeal's solution:
// https://gist.github.com/wontondon/e8c4bdf2888875e4c755712e99279536

Expand Down Expand Up @@ -141,6 +142,29 @@ function stripBasenameFromPathname(pathname: string, basename: string): string {
return pathname.slice(startIndex) || '/';
}

function sendIndexPath(pathBuilder: string, pathname: string, basename: string): [string, TransactionSource] {
const reconstructedPath = pathBuilder || _stripBasename ? stripBasenameFromPathname(pathname, basename) : pathname;

const formattedPath =
// If the path ends with a slash, remove it
reconstructedPath[reconstructedPath.length - 1] === '/'
? reconstructedPath.slice(0, -1)
: // If the path ends with a wildcard, remove it
reconstructedPath.slice(-2) === '/*'
? reconstructedPath.slice(0, -1)
: reconstructedPath;
Comment on lines +148 to +155
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if a regex replace is less expensive here. Would be interesting to benchmark.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tried now, looks like it's slower in this case https://jsperf.app/gemolu


return [formattedPath, 'route'];
}

function pathEndsWithWildcard(path: string, branch: RouteMatch<string>): boolean {
return (path.slice(-2) === '/*' && branch.route.children && branch.route.children.length > 0) || false;
}

function pathIsWildcardAndHasChildren(path: string, branch: RouteMatch<string>): boolean {
return (path === '*' && branch.route.children && branch.route.children.length > 0) || false;
}

function getNormalizedName(
routes: RouteObject[],
location: Location,
Expand All @@ -158,14 +182,16 @@ function getNormalizedName(
if (route) {
// Early return if index route
if (route.index) {
return [_stripBasename ? stripBasenameFromPathname(branch.pathname, basename) : branch.pathname, 'route'];
return sendIndexPath(pathBuilder, branch.pathname, basename);
}

const path = route.path;
if (path) {

// If path is not a wildcard and has no child routes, append the path
if (path && !pathIsWildcardAndHasChildren(path, branch)) {
const newPath = path[0] === '/' || pathBuilder[pathBuilder.length - 1] === '/' ? path : `/${path}`;
pathBuilder += newPath;

// If the path matches the current location, return the path
if (basename + branch.pathname === location.pathname) {
if (
// If the route defined on the element is something like
Expand All @@ -177,6 +203,12 @@ function getNormalizedName(
) {
return [(_stripBasename ? '' : basename) + newPath, 'route'];
}

// if the last character of the pathbuilder is a wildcard and there are children, remove the wildcard
if (pathEndsWithWildcard(pathBuilder, branch)) {
pathBuilder = pathBuilder.slice(0, -1);
}

return [(_stripBasename ? '' : basename) + pathBuilder, 'route'];
}
}
Expand Down
178 changes: 178 additions & 0 deletions packages/react/test/reactrouterv6.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,106 @@ describe('reactRouterV6BrowserTracingIntegration', () => {
});
});

it('works with wildcard routes', () => {
const client = createMockBrowserClient();
setCurrentClient(client);

client.addIntegration(
reactRouterV6BrowserTracingIntegration({
useEffect: React.useEffect,
useLocation,
useNavigationType,
createRoutesFromChildren,
matchRoutes,
}),
);
const SentryRoutes = withSentryReactRouterV6Routing(Routes);

render(
<MemoryRouter initialEntries={['/']}>
<SentryRoutes>
<Route path="*" element={<Outlet />}>
<Route index element={<Navigate to="/projects/123/views/234" />} />
<Route path="account" element={<div>Account Page</div>} />
<Route path="projects">
<Route path="*" element={<Outlet />}>
<Route path=":projectId" element={<div>Project Page</div>}>
<Route index element={<div>Project Page Root</div>} />
<Route element={<div>Editor</div>}>
<Route path="views/:viewId" element={<div>View Canvas</div>} />
<Route path="spaces/:spaceId" element={<div>Space Canvas</div>} />
</Route>
</Route>
</Route>
</Route>
<Route path="*" element={<div>No Match Page</div>} />
</Route>
</SentryRoutes>
</MemoryRouter>,
);

expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1);
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), {
name: '/projects/:projectId/views/:viewId',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6',
},
});
});

it('works with nested wildcard routes', () => {
const client = createMockBrowserClient();
setCurrentClient(client);

client.addIntegration(
reactRouterV6BrowserTracingIntegration({
useEffect: React.useEffect,
useLocation,
useNavigationType,
createRoutesFromChildren,
matchRoutes,
}),
);
const SentryRoutes = withSentryReactRouterV6Routing(Routes);

render(
<MemoryRouter initialEntries={['/']}>
<SentryRoutes>
<Route path="*" element={<Outlet />}>
<Route index element={<Navigate to="/projects/123/views/234" />} />
<Route path="account" element={<div>Account Page</div>} />
<Route path="projects">
<Route path="*" element={<Outlet />}>
<Route path=":projectId" element={<div>Project Page</div>}>
<Route index element={<div>Project Page Root</div>} />
<Route element={<div>Editor</div>}>
<Route path="*" element={<Outlet />}>
<Route path="views/:viewId" element={<div>View Canvas</div>} />
<Route path="spaces/:spaceId" element={<div>Space Canvas</div>} />
</Route>
</Route>
</Route>
</Route>
</Route>
<Route path="*" element={<div>No Match Page</div>} />
</Route>
</SentryRoutes>
</MemoryRouter>,
);

expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1);
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), {
name: '/projects/:projectId/views/:viewId',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6',
},
});
});

it("updates the scope's `transactionName` on a navigation", () => {
const client = createMockBrowserClient();
setCurrentClient(client);
Expand Down Expand Up @@ -849,6 +949,84 @@ describe('reactRouterV6BrowserTracingIntegration', () => {
});
});

it('works with wildcard routes', () => {
const client = createMockBrowserClient();
setCurrentClient(client);

client.addIntegration(
reactRouterV6BrowserTracingIntegration({
useEffect: React.useEffect,
useLocation,
useNavigationType,
createRoutesFromChildren,
matchRoutes,
}),
);
const wrappedUseRoutes = wrapUseRoutes(useRoutes);

const Routes = () =>
wrappedUseRoutes([
{
index: true,
element: <Navigate to="/param-page/1231/details/3212" />,
},
{
path: '*',
element: <></>,
children: [
{
path: 'profile',
element: <></>,
},
{
path: 'param-page',
element: <Outlet />,
children: [
{
path: '*',
element: <Outlet />,
children: [
{
path: ':id',
element: <></>,
children: [
{
element: <></>,
path: 'details',
children: [
{
element: <></>,
path: ':superId',
},
],
},
],
},
],
},
],
},
],
},
]);

render(
<MemoryRouter initialEntries={['/']}>
<Routes />
</MemoryRouter>,
);

expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1);
expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), {
name: '/param-page/:id/details/:superId',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v6',
},
});
});

it('does not add double slashes to URLS', () => {
const client = createMockBrowserClient();
setCurrentClient(client);
Expand Down
Loading