Skip to content

Commit cf15b60

Browse files
Lms24Lms24
committed
fix navigation bug
1 parent 2ab555b commit cf15b60

2 files changed

Lines changed: 148 additions & 25 deletions

File tree

‎packages/solid/src/tanstackrouter.ts‎

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,21 @@ export function tanstackRouterBrowserTracingIntegration<R extends AnyRouter>(
5555
return lastMatch?.routeId !== '__root__' ? lastMatch : undefined;
5656
};
5757

58+
const applyRouteMatch = (
59+
span: NonNullable<ReturnType<typeof startBrowserTracingPageLoadSpan>>,
60+
match: RouteMatch | undefined,
61+
toLocation: TanstackRouterLocation,
62+
fallbackName: string,
63+
): void => {
64+
span.updateName(match ? match.routeId : fallbackName);
65+
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, match ? 'route' : 'url');
66+
span.setAttributes({
67+
...(match && { [URL_TEMPLATE]: match.routeId }),
68+
...locationToSpanUrlAttributes(router, toLocation),
69+
...routeMatchToParamSpanAttributes(match),
70+
});
71+
};
72+
5873
const initialWindowLocation = WINDOW.location;
5974
if (instrumentPageLoad && initialWindowLocation) {
6075
const routeMatch = resolveRouteMatch(
@@ -80,16 +95,9 @@ export function tanstackRouterBrowserTracingIntegration<R extends AnyRouter>(
8095
if (!pageloadSpan) {
8196
return;
8297
}
83-
const resolvedMatch = resolveRouteMatch(onResolvedArgs.toLocation.pathname, onResolvedArgs.toLocation.search);
84-
if (resolvedMatch && resolvedMatch.routeId !== routeMatch?.routeId) {
85-
pageloadSpan.updateName(resolvedMatch.routeId);
86-
pageloadSpan.setAttributes({
87-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
88-
[URL_TEMPLATE]: resolvedMatch.routeId,
89-
...locationToSpanUrlAttributes(router, onResolvedArgs.toLocation),
90-
...routeMatchToParamSpanAttributes(resolvedMatch),
91-
});
92-
}
98+
const { toLocation } = onResolvedArgs;
99+
const resolvedMatch = resolveRouteMatch(toLocation.pathname, toLocation.search);
100+
applyRouteMatch(pageloadSpan, resolvedMatch, toLocation, toLocation.pathname);
93101
});
94102
}
95103

@@ -100,21 +108,6 @@ export function tanstackRouterBrowserTracingIntegration<R extends AnyRouter>(
100108
// span on the first `onBeforeLoad`, rename it on later ones, and clear it on `onResolved`.
101109
let inFlightNavigationSpan: ReturnType<typeof startBrowserTracingNavigationSpan> | undefined;
102110

103-
const applyRouteMatch = (
104-
span: NonNullable<typeof inFlightNavigationSpan>,
105-
match: RouteMatch | undefined,
106-
toLocation: TanstackRouterLocation,
107-
fallbackName: string,
108-
): void => {
109-
span.updateName(match ? match.routeId : fallbackName);
110-
span.setAttributes({
111-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: match ? 'route' : 'url',
112-
...(match && { [URL_TEMPLATE]: match.routeId }),
113-
...locationToSpanUrlAttributes(router, toLocation),
114-
...routeMatchToParamSpanAttributes(match),
115-
});
116-
};
117-
118111
router.subscribe('onBeforeLoad', onBeforeLoadArgs => {
119112
const { toLocation, fromLocation } = onBeforeLoadArgs;
120113
// Skip the initial pageload (no fromLocation) and no-op reloads (same state).

‎packages/solid/test/tanstackrouter.test.ts‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
import * as SentryBrowser from '@sentry/browser';
2+
import { URL_TEMPLATE } from '@sentry/conventions/attributes';
3+
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
4+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
5+
import { tanstackRouterBrowserTracingIntegration } from '../src/tanstackrouter';
6+
7+
vi.mock('@sentry/browser', async () => {
8+
const actual = await vi.importActual('@sentry/browser');
9+
return {
10+
...actual,
11+
WINDOW: {
12+
location: {
13+
pathname: '/posts/999',
14+
search: '',
15+
},
16+
},
17+
};
18+
});
19+
20+
const startBrowserTracingPageLoadSpanSpy = vi.spyOn(SentryBrowser, 'startBrowserTracingPageLoadSpan');
21+
22+
const mockPageloadSpan = {
23+
updateName: vi.fn(),
24+
setAttribute: vi.fn(),
25+
setAttributes: vi.fn(),
26+
};
27+
28+
describe('tanstackRouterBrowserTracingIntegration', () => {
29+
const mockMatchedRoutes = [
30+
{
31+
routeId: '/posts/$postId',
32+
pathname: '/posts/999',
33+
params: { postId: '999' },
34+
},
35+
];
36+
37+
const mockRouter = {
38+
options: {
39+
parseSearch: vi.fn(() => ({})),
40+
stringifySearch: vi.fn(() => ''),
41+
},
42+
matchRoutes: vi.fn(() => mockMatchedRoutes),
43+
subscribe: vi.fn(() => vi.fn()),
44+
};
45+
46+
const mockClient = {
47+
on: vi.fn(),
48+
emit: vi.fn(),
49+
getOptions: vi.fn(() => ({})),
50+
addEventProcessor: vi.fn(),
51+
};
52+
53+
const getSubscribeCallback = (eventType: string): ((...args: any[]) => void) =>
54+
(mockRouter.subscribe as any).mock.calls.find(
55+
(call: [string, (...args: any[]) => void]) => call[0] === eventType,
56+
)?.[1];
57+
58+
beforeEach(() => {
59+
vi.clearAllMocks();
60+
startBrowserTracingPageLoadSpanSpy.mockReturnValue(mockPageloadSpan as any);
61+
62+
vi.stubGlobal('window', {
63+
location: {
64+
pathname: '/posts/999',
65+
search: '',
66+
},
67+
});
68+
});
69+
70+
afterEach(() => {
71+
vi.clearAllMocks();
72+
vi.unstubAllGlobals();
73+
});
74+
75+
it('instruments pageload on setup', () => {
76+
const integration = tanstackRouterBrowserTracingIntegration(mockRouter, {
77+
instrumentPageLoad: true,
78+
instrumentNavigation: false,
79+
});
80+
81+
integration.afterAllSetup!(mockClient as any);
82+
83+
expect(startBrowserTracingPageLoadSpanSpy).toHaveBeenCalledWith(mockClient, {
84+
name: '/posts/$postId',
85+
attributes: expect.objectContaining({
86+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.solid.tanstack_router',
87+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
88+
[URL_TEMPLATE]: '/posts/$postId',
89+
'url.path.parameter.postId': '999',
90+
}),
91+
});
92+
});
93+
94+
it('updates pageload span URL attributes on redirect to the same route template', () => {
95+
const integration = tanstackRouterBrowserTracingIntegration(mockRouter, {
96+
instrumentPageLoad: true,
97+
instrumentNavigation: false,
98+
});
99+
100+
integration.afterAllSetup!(mockClient as any);
101+
102+
const onResolvedCallback = getSubscribeCallback('onResolved');
103+
expect(onResolvedCallback).toBeDefined();
104+
105+
(mockRouter.matchRoutes as any).mockReturnValueOnce([
106+
{
107+
routeId: '/posts/$postId',
108+
pathname: '/posts/2',
109+
params: { postId: '2' },
110+
},
111+
]);
112+
113+
onResolvedCallback({
114+
toLocation: {
115+
pathname: '/posts/2',
116+
search: {},
117+
},
118+
});
119+
120+
expect(mockPageloadSpan.setAttributes).toHaveBeenCalledWith(
121+
expect.objectContaining({
122+
[URL_TEMPLATE]: '/posts/$postId',
123+
'url.path': '/posts/2',
124+
'url.full': expect.any(String),
125+
'url.path.parameter.postId': '2',
126+
'params.postId': '2',
127+
}),
128+
);
129+
});
130+
});

0 commit comments

Comments
 (0)