Skip to content

Commit d7df7d4

Browse files
Lms24cursoragent
andauthored
ref(sveltekit): Use untrack to read route id without invalidation (#19272)
This patch refactors how we look up SvelteKit event properties. We now use SvelteKit 2's `untrack` utility when available to read `event.route.id` without triggering route invalidation in load wrappers. Previously, we relied on a hack to read the route id via `Object.getOwnPropertyDescriptor`, which didn't trigger the Proxy on the route object set by SvelteKit. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 24b2ef2 commit d7df7d4

File tree

6 files changed

+154
-18
lines changed

6 files changed

+154
-18
lines changed

packages/sveltekit/src/client/load.ts

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
import { captureException } from '@sentry/svelte';
1010
import type { LoadEvent } from '@sveltejs/kit';
1111
import type { SentryWrappedFlag } from '../common/utils';
12-
import { isHttpError, isRedirect } from '../common/utils';
12+
import { getRouteId, isHttpError, isRedirect } from '../common/utils';
1313

1414
type PatchedLoadEvent = LoadEvent & Partial<SentryWrappedFlag>;
1515

@@ -72,16 +72,7 @@ export function wrapLoadWithSentry<T extends (...args: any) => any>(origLoad: T)
7272

7373
addNonEnumerableProperty(patchedEvent as unknown as Record<string, unknown>, '__sentry_wrapped__', true);
7474

75-
// Accessing any member of `event.route` causes SvelteKit to invalidate the
76-
// client-side universal `load` function's data prefetched data, causing another reload on the actual navigation.
77-
// To work around this, we use `Object.getOwnPropertyDescriptor` which doesn't invoke the proxy.
78-
const routeIdDescriptor = event.route && Object.getOwnPropertyDescriptor(event.route, 'id');
79-
// First, we try to access the route id from the property descriptor.
80-
// This will only work for @sveltejs/kit >= 1.24.0
81-
const routeIdFromDescriptor = routeIdDescriptor && (routeIdDescriptor.value as string | undefined);
82-
// If routeIdFromDescriptor is undefined, we fall back to the old behavior of accessing
83-
// `event.route.id` directly. This will still cause invalidations but we get a route name.
84-
const routeId = routeIdFromDescriptor || event.route.id;
75+
const routeId = getRouteId(event);
8576

8677
return startSpan(
8778
{

packages/sveltekit/src/common/utils.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,38 @@ export type SentryWrappedFlag = {
1010
__sentry_wrapped__?: true;
1111
};
1212

13+
interface EventLike {
14+
route?: {
15+
id?: string | null;
16+
};
17+
untrack<T>(fn: () => T): T;
18+
}
19+
20+
/**
21+
* Get route.id from a load event without triggering SvelteKit's route proxy
22+
* (which would cause unwanted invalidations). Uses `untrack` when available (SvelteKit 2+),
23+
* otherwise falls back to getOwnPropertyDescriptor for SvelteKit 1.x.
24+
*/
25+
export function getRouteId(event: EventLike): string | void {
26+
if (typeof event.untrack === 'function') {
27+
return event.untrack(() => event.route?.id ?? undefined);
28+
}
29+
30+
const route = event.route;
31+
if (!route) {
32+
return;
33+
}
34+
35+
const descriptor = Object.getOwnPropertyDescriptor(route, 'id');
36+
const fromDescriptor = descriptor?.value as string | null | undefined;
37+
38+
if (fromDescriptor !== undefined && fromDescriptor !== null) {
39+
return fromDescriptor;
40+
}
41+
42+
return;
43+
}
44+
1345
/**
1446
* Determines if a thrown "error" is a Redirect object which SvelteKit users can throw to redirect to another route
1547
* see: https://kit.svelte.dev/docs/modules#sveltejs-kit-redirect

packages/sveltekit/src/server-common/load.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
} from '@sentry/core';
88
import type { LoadEvent, ServerLoadEvent } from '@sveltejs/kit';
99
import type { SentryWrappedFlag } from '../common/utils';
10+
import { getRouteId } from '../common/utils';
1011
import { sendErrorToSentry } from './utils';
1112

1213
type PatchedLoadEvent = LoadEvent & SentryWrappedFlag;
@@ -33,7 +34,7 @@ export function wrapLoadWithSentry<T extends (...args: any) => any>(origLoad: T)
3334

3435
addNonEnumerableProperty(event as unknown as Record<string, unknown>, '__sentry_wrapped__', true);
3536

36-
const routeId = event.route?.id;
37+
const routeId = getRouteId(event);
3738

3839
try {
3940
// We need to await before returning, otherwise we won't catch any errors thrown by the load function
@@ -94,10 +95,9 @@ export function wrapServerLoadWithSentry<T extends (...args: any) => any>(origSe
9495
addNonEnumerableProperty(event as unknown as Record<string, unknown>, '__sentry_wrapped__', true);
9596

9697
// Accessing any member of `event.route` causes SvelteKit to invalidate the
97-
// server `load` function's data on every route change.
98-
// To work around this, we use `Object.getOwnPropertyDescriptor` which doesn't invoke the proxy.
99-
// https://github.com/sveltejs/kit/blob/e133aba479fa9ba0e7f9e71512f5f937f0247e2c/packages/kit/src/runtime/server/page/load_data.js#L111C3-L124
100-
const routeId = event.route && (Object.getOwnPropertyDescriptor(event.route, 'id')?.value as string | undefined);
98+
// server `load` function's data on every route change. We use `getRouteId` which uses
99+
// SvelteKit 2's `untrack` when available, otherwise getOwnPropertyDescriptor for 1.x.
100+
const routeId = getRouteId(event);
101101

102102
try {
103103
// We need to await before returning, otherwise we won't catch any errors thrown by the load function

packages/sveltekit/test/client/load.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,35 @@ describe('wrapLoadWithSentry', () => {
139139
expect.any(Function),
140140
);
141141
});
142+
143+
it('uses untrack when present (SvelteKit 2+) to get route id without triggering invalidation', async () => {
144+
const routeIdFromUntrack = '/users/[id]';
145+
const untrack = vi.fn(<T>(fn: () => T) => fn());
146+
const eventWithUntrack = {
147+
params: MOCK_LOAD_ARGS.params,
148+
route: { id: routeIdFromUntrack },
149+
url: MOCK_LOAD_ARGS.url,
150+
untrack,
151+
};
152+
153+
async function load({ params }: Parameters<Load>[0]): Promise<ReturnType<Load>> {
154+
return { post: params.id };
155+
}
156+
157+
const wrappedLoad = wrapLoadWithSentry(load);
158+
await wrappedLoad(eventWithUntrack);
159+
160+
expect(untrack).toHaveBeenCalledTimes(1);
161+
expect(mockStartSpan).toHaveBeenCalledWith(
162+
expect.objectContaining({
163+
name: routeIdFromUntrack,
164+
attributes: expect.objectContaining({
165+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
166+
}),
167+
}),
168+
expect.any(Function),
169+
);
170+
});
142171
});
143172

144173
it('adds an exception mechanism', async () => {

packages/sveltekit/test/common/utils.test.ts

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { describe, expect, it } from 'vitest';
2-
import { isHttpError, isRedirect } from '../../src/common/utils';
1+
import { describe, expect, it, vi } from 'vitest';
2+
import { getRouteId, isHttpError, isRedirect } from '../../src/common/utils';
33

44
describe('isRedirect', () => {
55
it.each([
@@ -40,3 +40,65 @@ describe('isHttpError', () => {
4040
},
4141
);
4242
});
43+
44+
describe('getRouteId', () => {
45+
it('returns route.id when event has untrack (SvelteKit 2+)', () => {
46+
const untrack = vi.fn(<T>(fn: () => T) => fn());
47+
const event = {
48+
route: { id: '/blog/[slug]' },
49+
untrack,
50+
};
51+
52+
// @ts-expect-error - only passing a partial load event here
53+
expect(getRouteId(event)).toBe('/blog/[slug]');
54+
55+
expect(untrack).toHaveBeenCalledTimes(1);
56+
expect(untrack).toHaveBeenCalledWith(expect.any(Function));
57+
});
58+
59+
it('returns undefined when event has untrack but route.id is null', () => {
60+
const untrack = vi.fn(<T>(fn: () => T) => fn());
61+
const event = {
62+
route: { id: null },
63+
untrack,
64+
};
65+
66+
// @ts-expect-error - only passing a partial load event here
67+
expect(getRouteId(event)).toBeUndefined();
68+
69+
expect(untrack).toHaveBeenCalledTimes(1);
70+
expect(untrack).toHaveBeenCalledWith(expect.any(Function));
71+
});
72+
73+
it('falls back to getOwnPropertyDescriptor and avoids triggering the proxy', () => {
74+
const routeId = '/users/[id]';
75+
76+
let routeIdAccessed = false;
77+
const route = { id: routeId };
78+
79+
// taken from https://github.com/sveltejs/kit/blob/159aece0654db020f95bc414f6a21f25fbc5f22f/packages/kit/src/runtime/client/client.js#L783-L790
80+
const proxiedRoute = new Proxy(route, {
81+
get: (target, key) => {
82+
routeIdAccessed = true;
83+
// @ts-expect-error - this is fine for the test
84+
return target[key];
85+
},
86+
});
87+
88+
const event = { route: proxiedRoute };
89+
// @ts-expect-error - only passing a partial load event here
90+
expect(getRouteId(event)).toBe(routeId);
91+
expect(routeIdAccessed).toBe(false);
92+
93+
// sanity check that the proxying mechanism works
94+
expect(event.route.id).toBe(routeId);
95+
expect(routeIdAccessed).toBe(true);
96+
});
97+
98+
it('returns undefined when event has no route', () => {
99+
// @ts-expect-error - only passing a partial load event here
100+
expect(getRouteId({})).toBeUndefined();
101+
// @ts-expect-error - only passing a partial load event here
102+
expect(getRouteId({ route: null })).toBeUndefined();
103+
});
104+
});

packages/sveltekit/test/server-common/load.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,4 +304,26 @@ describe('wrapServerLoadWithSentry calls `startSpan`', () => {
304304

305305
expect(proxyFn).not.toHaveBeenCalled();
306306
});
307+
308+
it('uses untrack when present (SvelteKit 2+) to get route id without triggering invalidation', async () => {
309+
const untrack = vi.fn(<T>(fn: () => T) => fn());
310+
const eventWithUntrack = {
311+
...getServerOnlyArgs(),
312+
untrack,
313+
};
314+
315+
const wrappedLoad = wrapServerLoadWithSentry(serverLoad);
316+
await wrappedLoad(eventWithUntrack);
317+
318+
expect(untrack).toHaveBeenCalledTimes(1);
319+
expect(mockStartSpan).toHaveBeenCalledWith(
320+
expect.objectContaining({
321+
name: '/users/[id]',
322+
attributes: expect.objectContaining({
323+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
324+
}),
325+
}),
326+
expect.any(Function),
327+
);
328+
});
307329
});

0 commit comments

Comments
 (0)