Skip to content

Commit 1d42620

Browse files
onurtemizkanonurtemizkan
committed
Alternative implementation.
1 parent 73fbd62 commit 1d42620

File tree

2 files changed

+141
-47
lines changed

2 files changed

+141
-47
lines changed

‎packages/react/src/reactrouterv6.tsx

Lines changed: 92 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ export function reactRouterV6BrowserTracingIntegration(
9898
integration.afterAllSetup(client);
9999

100100
const initPathName = WINDOW && WINDOW.location && WINDOW.location.pathname;
101+
101102
if (instrumentPageLoad && initPathName) {
102103
startBrowserTracingPageLoadSpan(client, {
103104
name: initPathName,
@@ -158,16 +159,16 @@ function sendIndexPath(pathBuilder: string, pathname: string, basename: string):
158159
return [formattedPath, 'route'];
159160
}
160161

161-
function pathEndsWithWildcard(path: string): boolean {
162-
return path.endsWith('*');
162+
function matchHasWildcard(match: RouteMatch<string>): boolean {
163+
return !!match.params['*'];
163164
}
164165

165-
function pathIsWildcardAndHasChildren(path: string, branch: RouteMatch<string>): boolean {
166-
return (pathEndsWithWildcard(path) && branch.route.children && branch.route.children.length > 0) || false;
166+
function pathEndsWithWildcard(path: string): boolean {
167+
return path.endsWith('*');
167168
}
168169

169-
function pathIsWildcardWithNoChildren(path: string, branch: RouteMatch<string>): boolean {
170-
return (pathEndsWithWildcard(path) && (!branch.route.children || branch.route.children.length === 0)) || false;
170+
function matchIsWildcardAndHasChildren(path: string, match: RouteMatch<string>): boolean {
171+
return (matchHasWildcard(match) && match.route.children && match.route.children.length > 0) || false;
171172
}
172173

173174
function getNormalizedName(
@@ -178,24 +179,31 @@ function getNormalizedName(
178179
allRoutes: RouteObject[] = routes,
179180
): [string, TransactionSource] {
180181
if (!routes || routes.length === 0) {
182+
debugger;
181183
return [_stripBasename ? stripBasenameFromPathname(location.pathname, basename) : location.pathname, 'url'];
182184
}
183185

184-
const matchedRoutes = _matchRoutes(routes, location);
186+
const matchedRoutes = _matchRoutes(allRoutes, location, basename);
185187

186188
if (matchedRoutes) {
187-
const wildCardRoutes: RouteMatch[] = matchedRoutes.filter(
188-
(match: RouteMatch) => match.route.path && pathIsWildcardWithNoChildren(match.route.path, match),
189-
);
189+
const wildCardRoutes: RouteMatch[] = matchedRoutes.filter((match: RouteMatch) => {
190+
return matchHasWildcard(match);
191+
});
190192

191193
for (const wildCardRoute of wildCardRoutes) {
192194
const wildCardRouteMatch = _matchRoutes(allRoutes, location, wildCardRoute.pathnameBase);
193195

194196
if (wildCardRouteMatch) {
195-
const [name, source] = getNormalizedName(wildCardRoutes, location, wildCardRouteMatch, basename, allRoutes);
197+
const [name, source] = getNormalizedName(
198+
wildCardRoutes,
199+
location,
200+
wildCardRouteMatch,
201+
wildCardRoute.pathnameBase,
202+
allRoutes,
203+
);
196204

197205
if (wildCardRoute.pathnameBase && name) {
198-
return [wildCardRoute.pathnameBase + name, source];
206+
return [basename + wildCardRoute.pathnameBase + name, source];
199207
}
200208
}
201209
}
@@ -208,12 +216,13 @@ function getNormalizedName(
208216
if (route) {
209217
// Early return if index route
210218
if (route.index) {
219+
debugger;
211220
return sendIndexPath(pathBuilder, branch.pathname, basename);
212221
}
213222
const path = route.path;
214223

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

@@ -236,7 +245,7 @@ function getNormalizedName(
236245
}
237246

238247
// if the last character of the pathbuilder is a wildcard and there are children, remove the wildcard
239-
if (pathIsWildcardAndHasChildren(pathBuilder, branch)) {
248+
if (matchIsWildcardAndHasChildren(pathBuilder, branch)) {
240249
pathBuilder = pathBuilder.slice(0, -1);
241250
}
242251

@@ -247,23 +256,27 @@ function getNormalizedName(
247256
}
248257
}
249258

259+
debugger;
250260
return [_stripBasename ? stripBasenameFromPathname(location.pathname, basename) : location.pathname, 'url'];
251261
}
252262

253-
function updatePageloadTransaction(
254-
activeRootSpan: Span | undefined,
255-
location: Location,
256-
routes: RouteObject[],
257-
matches?: AgnosticDataRouteMatch,
258-
basename?: string,
259-
allRoutes?: RouteObject[],
260-
): void {
263+
function updatePageloadTransaction(options: {
264+
activeRootSpan: Span | undefined;
265+
location: Location;
266+
routes: RouteObject[];
267+
matches?: AgnosticDataRouteMatch;
268+
basename?: string;
269+
allRoutes?: RouteObject[];
270+
}): void {
271+
debugger;
272+
const { activeRootSpan, location, routes, matches, basename, allRoutes } = options;
273+
261274
const branches = Array.isArray(matches)
262275
? matches
263-
: (_matchRoutes(routes, location, basename) as unknown as RouteMatch[]);
276+
: (_matchRoutes(allRoutes, location, basename) as unknown as RouteMatch[]);
264277

265278
if (branches) {
266-
const [name, source] = getNormalizedName(routes, location, branches, basename, allRoutes);
279+
const [name, source] = getNormalizedName(allRoutes || routes, location, branches, basename, allRoutes);
267280

268281
getCurrentScope().setTransactionName(name);
269282

@@ -274,14 +287,16 @@ function updatePageloadTransaction(
274287
}
275288
}
276289

277-
function handleNavigation(
278-
location: Location,
279-
routes: RouteObject[],
280-
navigationType: Action,
281-
matches?: AgnosticDataRouteMatch,
282-
basename?: string,
283-
allRoutes?: RouteObject[],
284-
): void {
290+
function handleNavigation(options: {
291+
location: Location;
292+
routes: RouteObject[];
293+
navigationType: Action;
294+
matches?: AgnosticDataRouteMatch;
295+
basename?: string;
296+
allRoutes?: RouteObject[];
297+
}): void {
298+
const { location, routes, navigationType, matches, basename, allRoutes } = options;
299+
285300
const branches = Array.isArray(matches) ? matches : _matchRoutes(routes, location, basename);
286301

287302
const client = getClient();
@@ -339,15 +354,25 @@ export function withSentryReactRouterV6Routing<P extends Record<string, any>, R
339354
() => {
340355
const routes = _createRoutesFromChildren(props.children) as RouteObject[];
341356

342-
routes.forEach(route => {
343-
allRoutes.push(...getChildRoutesRecursively(route));
344-
});
345-
346357
if (isMountRenderPass) {
347-
updatePageloadTransaction(getActiveRootSpan(), location, routes, undefined, undefined, allRoutes);
358+
routes.forEach(route => {
359+
allRoutes.push(...getChildRoutesRecursively(route));
360+
});
361+
362+
updatePageloadTransaction({
363+
activeRootSpan: getActiveRootSpan(),
364+
location,
365+
routes,
366+
allRoutes,
367+
});
348368
isMountRenderPass = false;
349369
} else {
350-
handleNavigation(location, routes, navigationType, undefined, undefined, allRoutes);
370+
handleNavigation({
371+
location,
372+
routes,
373+
navigationType,
374+
allRoutes,
375+
});
351376
}
352377
},
353378
// `props.children` is purposely not included in the dependency array, because we do not want to re-run this effect
@@ -402,15 +427,26 @@ export function wrapUseRoutes(origUseRoutes: UseRoutes): UseRoutes {
402427
const normalizedLocation =
403428
typeof stableLocationParam === 'string' ? { pathname: stableLocationParam } : stableLocationParam;
404429

405-
routes.forEach(route => {
406-
allRoutes.push(...getChildRoutesRecursively(route));
407-
});
408-
409430
if (isMountRenderPass) {
410-
updatePageloadTransaction(getActiveRootSpan(), normalizedLocation, routes, undefined, undefined, allRoutes);
431+
routes.forEach(route => {
432+
allRoutes.push(...getChildRoutesRecursively(route));
433+
});
434+
435+
updatePageloadTransaction({
436+
activeRootSpan: getActiveRootSpan(),
437+
location: normalizedLocation,
438+
routes,
439+
allRoutes,
440+
});
441+
411442
isMountRenderPass = false;
412443
} else {
413-
handleNavigation(normalizedLocation, routes, navigationType, undefined, undefined, allRoutes);
444+
handleNavigation({
445+
location: normalizedLocation,
446+
routes,
447+
navigationType,
448+
allRoutes,
449+
});
414450
}
415451
}, [navigationType, stableLocationParam]);
416452

@@ -449,13 +485,23 @@ export function wrapCreateBrowserRouter<
449485
// This is the earliest convenient time to update the transaction name.
450486
// Callbacks to `router.subscribe` are not called for the initial load.
451487
if (router.state.historyAction === 'POP' && activeRootSpan) {
452-
updatePageloadTransaction(activeRootSpan, router.state.location, routes, undefined, basename);
488+
updatePageloadTransaction({
489+
activeRootSpan,
490+
location: router.state.location,
491+
routes,
492+
basename,
493+
});
453494
}
454495

455496
router.subscribe((state: RouterState) => {
456497
const location = state.location;
457498
if (state.historyAction === 'PUSH' || state.historyAction === 'POP') {
458-
handleNavigation(location, routes, state.historyAction, undefined, basename);
499+
handleNavigation({
500+
location,
501+
routes,
502+
navigationType: state.historyAction,
503+
basename,
504+
});
459505
}
460506
});
461507

‎packages/react/test/reactrouterv6.test.tsx

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,55 @@ describe('reactRouterV6BrowserTracingIntegration', () => {
491491
});
492492
});
493493

494-
it('works with descendant wildcard routes', () => {
494+
it('works with descendant wildcard routes - pageload', () => {
495+
const client = createMockBrowserClient();
496+
setCurrentClient(client);
497+
498+
client.addIntegration(
499+
reactRouterV6BrowserTracingIntegration({
500+
useEffect: React.useEffect,
501+
useLocation,
502+
useNavigationType,
503+
createRoutesFromChildren,
504+
matchRoutes,
505+
}),
506+
);
507+
const SentryRoutes = withSentryReactRouterV6Routing(Routes);
508+
509+
const ProjectsRoutes = () => (
510+
<SentryRoutes>
511+
<Route path=":projectId" element={<div>Project Page</div>}>
512+
<Route index element={<div>Project Page Root</div>} />
513+
<Route element={<div>Editor</div>}>
514+
<Route path="*" element={<Outlet />}>
515+
<Route path="views/:viewId" element={<div>View Canvas</div>} />
516+
</Route>
517+
</Route>
518+
</Route>
519+
<Route path="*" element={<div>No Match Page</div>} />
520+
</SentryRoutes>
521+
);
522+
523+
render(
524+
<MemoryRouter initialEntries={['/projects/foo/views/bar']}>
525+
<SentryRoutes>
526+
<Route path="projects/*" element={<ProjectsRoutes />}></Route>
527+
</SentryRoutes>
528+
</MemoryRouter>,
529+
);
530+
531+
expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenCalledTimes(1);
532+
expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), {
533+
name: '/projects/:projectId/views/:viewId',
534+
attributes: {
535+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
536+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
537+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.react.reactrouter_v6',
538+
},
539+
});
540+
});
541+
542+
it('works with descendant wildcard routes - navigation', () => {
495543
const client = createMockBrowserClient();
496544
setCurrentClient(client);
497545

0 commit comments

Comments
 (0)