Skip to content

Commit 0dfe3a5

Browse files
onurtemizkanonurtemizkan
committed
Alternative implementation.
1 parent 0bd48c8 commit 0dfe3a5

File tree

2 files changed

+146
-50
lines changed

2 files changed

+146
-50
lines changed

‎packages/react/src/reactrouterv6.tsx

+97-49
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

@@ -226,42 +235,48 @@ function getNormalizedName(
226235
// If the route defined on the element is something like
227236
// <Route path="/stores/:storeId/products/:productId" element={<div>Product</div>} />
228237
// We should check against the branch.pathname for the number of / separators
229-
getNumberOfUrlSegments(pathBuilder) !== getNumberOfUrlSegments(branch.pathname) &&
230-
// We should not count wildcard operators in the url segments calculation
231-
!pathEndsWithWildcard(pathBuilder)
238+
getNumberOfUrlSegments(pathBuilder) === getNumberOfUrlSegments(branch.pathname) // &&
239+
// // We should not count wildcard operators in the url segments calculation
240+
// !pathEndsWithWildcard(pathBuilder)
232241
) {
242+
debugger
233243
return [(_stripBasename ? '' : basename) + newPath, 'route'];
234244
}
235245

236246
// if the last character of the pathbuilder is a wildcard and there are children, remove the wildcard
237-
if (pathIsWildcardAndHasChildren(pathBuilder, branch)) {
247+
if (matchIsWildcardAndHasChildren(pathBuilder, branch)) {
238248
pathBuilder = pathBuilder.slice(0, -1);
239249
}
240250

251+
debugger;
241252
return [(_stripBasename ? '' : basename) + pathBuilder, 'route'];
242253
}
243254
}
244255
}
245256
}
246257
}
247258

259+
debugger;
248260
return [_stripBasename ? stripBasenameFromPathname(location.pathname, basename) : location.pathname, 'url'];
249261
}
250262

251-
function updatePageloadTransaction(
252-
activeRootSpan: Span | undefined,
253-
location: Location,
254-
routes: RouteObject[],
255-
matches?: AgnosticDataRouteMatch,
256-
basename?: string,
257-
allRoutes?: RouteObject[],
258-
): 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+
259274
const branches = Array.isArray(matches)
260275
? matches
261-
: (_matchRoutes(routes, location, basename) as unknown as RouteMatch[]);
276+
: (_matchRoutes(allRoutes, location, basename) as unknown as RouteMatch[]);
262277

263278
if (branches) {
264-
const [name, source] = getNormalizedName(routes, location, branches, basename, allRoutes);
279+
const [name, source] = getNormalizedName(allRoutes || routes, location, branches, basename, allRoutes);
265280

266281
getCurrentScope().setTransactionName(name);
267282

@@ -272,14 +287,16 @@ function updatePageloadTransaction(
272287
}
273288
}
274289

275-
function handleNavigation(
276-
location: Location,
277-
routes: RouteObject[],
278-
navigationType: Action,
279-
matches?: AgnosticDataRouteMatch,
280-
basename?: string,
281-
allRoutes?: RouteObject[],
282-
): 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+
283300
const branches = Array.isArray(matches) ? matches : _matchRoutes(routes, location, basename);
284301

285302
const client = getClient();
@@ -337,15 +354,25 @@ export function withSentryReactRouterV6Routing<P extends Record<string, any>, R
337354
() => {
338355
const routes = _createRoutesFromChildren(props.children) as RouteObject[];
339356

340-
routes.forEach(route => {
341-
allRoutes.push(...getChildRoutesRecursively(route));
342-
});
343-
344357
if (isMountRenderPass) {
345-
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+
});
346368
isMountRenderPass = false;
347369
} else {
348-
handleNavigation(location, routes, navigationType, undefined, undefined, allRoutes);
370+
handleNavigation({
371+
location,
372+
routes,
373+
navigationType,
374+
allRoutes,
375+
});
349376
}
350377
},
351378
// `props.children` is purposely not included in the dependency array, because we do not want to re-run this effect
@@ -400,15 +427,26 @@ export function wrapUseRoutes(origUseRoutes: UseRoutes): UseRoutes {
400427
const normalizedLocation =
401428
typeof stableLocationParam === 'string' ? { pathname: stableLocationParam } : stableLocationParam;
402429

403-
routes.forEach(route => {
404-
allRoutes.push(...getChildRoutesRecursively(route));
405-
});
406-
407430
if (isMountRenderPass) {
408-
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+
409442
isMountRenderPass = false;
410443
} else {
411-
handleNavigation(normalizedLocation, routes, navigationType, undefined, undefined, allRoutes);
444+
handleNavigation({
445+
location: normalizedLocation,
446+
routes,
447+
navigationType,
448+
allRoutes,
449+
});
412450
}
413451
}, [navigationType, stableLocationParam]);
414452

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

453496
router.subscribe((state: RouterState) => {
454497
const location = state.location;
455498
if (state.historyAction === 'PUSH' || state.historyAction === 'POP') {
456-
handleNavigation(location, routes, state.historyAction, undefined, basename);
499+
handleNavigation({
500+
location,
501+
routes,
502+
navigationType: state.historyAction,
503+
basename,
504+
});
457505
}
458506
});
459507

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

+49-1
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)