From b158b1dda118426bbd0683945b703c50489d769b Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 9 Oct 2026 16:10:28 +0200 Subject: [PATCH 1/3] fix(sveltekit): Read load event pathname untracked The load wrappers read event.url.pathname for the url.path attribute outside of untrack, so SvelteKit marked every wrapped load as depending on the URL and reran it on each navigation. Read the pathname through untrack, or the native URL getter when untrack is unavailable. Fixes #25231 Co-Authored-By: Claude Opus 5.5 --- packages/sveltekit/src/client/load.ts | 7 ++-- packages/sveltekit/src/common/utils.ts | 13 +++++++ packages/sveltekit/src/server-common/load.ts | 12 +++--- packages/sveltekit/test/client/load.test.ts | 23 ++++++++++- packages/sveltekit/test/common/utils.test.ts | 31 ++++++++++++++- .../sveltekit/test/server-common/load.test.ts | 34 +++++++++++++++- packages/sveltekit/test/utils.ts | 39 +++++++++++++++++++ 7 files changed, 147 insertions(+), 12 deletions(-) diff --git a/packages/sveltekit/src/client/load.ts b/packages/sveltekit/src/client/load.ts index a1a78995ce0e..a96634963899 100644 --- a/packages/sveltekit/src/client/load.ts +++ b/packages/sveltekit/src/client/load.ts @@ -19,7 +19,7 @@ import { FUNCTION } from '@sentry/conventions/op'; import { captureException } from '@sentry/svelte'; import type { LoadEvent } from '@sveltejs/kit'; import type { SentryWrappedFlag } from '../common/utils'; -import { getRouteId, isHttpError, isRedirect } from '../common/utils'; +import { getPathname, getRouteId, isHttpError, isRedirect } from '../common/utils'; type PatchedLoadEvent = LoadEvent & Partial; @@ -83,7 +83,8 @@ export function wrapLoadWithSentry any>(origLoad: T) addNonEnumerableProperty(patchedEvent, '__sentry_wrapped__', true); const routeId = getRouteId(event); - const routeOrPathname = routeId ? routeId : event.url.pathname; + const pathname = getPathname(event); + const routeOrPathname = routeId ? routeId : pathname; const client = getClient(); const hasSpanStreaming = !!client && hasSpanStreamingEnabled(client); @@ -97,7 +98,7 @@ export function wrapLoadWithSentry any>(origLoad: T) [CODE_FUNCTION_NAME]: 'load', [SENTRY_ORIGIN]: 'auto.function.sveltekit', [SENTRY_SEGMENT_NAME_SOURCE]: routeId ? 'route' : 'url', - [URL_PATH]: event.url.pathname, + [URL_PATH]: pathname, ...(routeId && { [URL_TEMPLATE]: routeId }), // Relay infers the description from `code.function.name`, which would drop the route. ...(hasSpanStreaming && { [SENTRY_DESCRIPTION]: routeOrPathname }), diff --git a/packages/sveltekit/src/common/utils.ts b/packages/sveltekit/src/common/utils.ts index b5621cddd059..73326f81e146 100644 --- a/packages/sveltekit/src/common/utils.ts +++ b/packages/sveltekit/src/common/utils.ts @@ -15,6 +15,7 @@ interface EventLike { route?: { id?: string | null; }; + url: URL; untrack(fn: () => T): T; } @@ -43,6 +44,18 @@ export function getRouteId(event: EventLike): string | void { return; } +/** + * Get url.pathname from a load event without making the load depend on the URL. + * Without `untrack`, calls the native getter to skip SvelteKit's tracking getter on `event.url`. + */ +export function getPathname(event: EventLike): string { + if (typeof event.untrack === 'function') { + return event.untrack(() => event.url.pathname); + } + + return Reflect.get(URL.prototype, 'pathname', event.url); +} + /** * Determines if a thrown "error" is a Redirect object which SvelteKit users can throw to redirect to another route * see: https://kit.svelte.dev/docs/modules#sveltejs-kit-redirect diff --git a/packages/sveltekit/src/server-common/load.ts b/packages/sveltekit/src/server-common/load.ts index 5a6dfd93100b..cfad7bcf9621 100644 --- a/packages/sveltekit/src/server-common/load.ts +++ b/packages/sveltekit/src/server-common/load.ts @@ -13,7 +13,7 @@ import { import { FUNCTION } from '@sentry/conventions/op'; import type { LoadEvent, ServerLoadEvent } from '@sveltejs/kit'; import type { SentryWrappedFlag } from '../common/utils'; -import { getRouteId } from '../common/utils'; +import { getPathname, getRouteId } from '../common/utils'; import { sendErrorToSentry } from './utils'; type PatchedLoadEvent = LoadEvent & SentryWrappedFlag; @@ -41,7 +41,8 @@ export function wrapLoadWithSentry any>(origLoad: T) addNonEnumerableProperty(event, '__sentry_wrapped__', true); const routeId = getRouteId(event) ?? undefined; - const routeOrPathname = routeId ? routeId : event.url.pathname; + const pathname = getPathname(event); + const routeOrPathname = routeId ? routeId : pathname; const client = getClient(); const hasSpanStreaming = !!client && hasSpanStreamingEnabled(client); @@ -56,7 +57,7 @@ export function wrapLoadWithSentry any>(origLoad: T) [CODE_FUNCTION_NAME]: 'load', [SENTRY_ORIGIN]: 'auto.function.sveltekit', [SENTRY_SEGMENT_NAME_SOURCE]: routeId ? 'route' : 'url', - [URL_PATH]: event.url.pathname, + [URL_PATH]: pathname, [HTTP_ROUTE]: routeId, // Relay infers the description from `code.function.name`, which would drop the route. ...(hasSpanStreaming && { [SENTRY_DESCRIPTION]: routeOrPathname }), @@ -113,7 +114,8 @@ export function wrapServerLoadWithSentry any>(origSe // server `load` function's data on every route change. We use `getRouteId` which uses // SvelteKit 2's `untrack` when available, otherwise getOwnPropertyDescriptor for 1.x. const routeId = getRouteId(event) ?? undefined; - const routeOrPathname = routeId ? routeId : event.url.pathname; + const pathname = getPathname(event); + const routeOrPathname = routeId ? routeId : pathname; const client = getClient(); const hasSpanStreaming = !!client && hasSpanStreamingEnabled(client); @@ -128,7 +130,7 @@ export function wrapServerLoadWithSentry any>(origSe [SENTRY_ORIGIN]: 'auto.function.sveltekit.server', [SENTRY_SEGMENT_NAME_SOURCE]: routeId ? 'route' : 'url', [HTTP_REQUEST_METHOD]: event.request.method, - [URL_PATH]: event.url.pathname, + [URL_PATH]: pathname, [HTTP_ROUTE]: routeId, // Relay infers the description from `code.function.name`, which would drop the route. ...(hasSpanStreaming && { [SENTRY_DESCRIPTION]: routeOrPathname }), diff --git a/packages/sveltekit/test/client/load.test.ts b/packages/sveltekit/test/client/load.test.ts index 5978a2bffc01..cd3cf233486a 100644 --- a/packages/sveltekit/test/client/load.test.ts +++ b/packages/sveltekit/test/client/load.test.ts @@ -6,6 +6,7 @@ import type { Load } from '@sveltejs/kit'; import { redirect } from '@sveltejs/kit'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { wrapLoadWithSentry } from '../../src/client/load'; +import { createTrackedLoadEvent } from '../utils'; const mockCaptureException = vi.spyOn(SentrySvelte, 'captureException').mockImplementation(() => 'xx'); @@ -164,7 +165,7 @@ describe('wrapLoadWithSentry', () => { const wrappedLoad = wrapLoadWithSentry(load); await wrappedLoad(eventWithUntrack); - expect(untrack).toHaveBeenCalledTimes(1); + expect(untrack).toHaveBeenCalledTimes(2); expect(mockStartSpan).toHaveBeenCalledWith( expect.objectContaining({ name: routeIdFromUntrack, @@ -176,6 +177,26 @@ describe('wrapLoadWithSentry', () => { ); }); + it("doesn't add route or url dependencies to the load function when untrack is present", async () => { + const { event, trackedReads } = createTrackedLoadEvent({ + params: { id: '123' }, + route: { id: '/users/[id]' }, + url: new URL('http://localhost:3000/users/123'), + }); + + const wrappedLoad = wrapLoadWithSentry(async () => ({})); + await wrappedLoad(event); + + expect(trackedReads).toEqual([]); + expect(mockStartSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: '/users/[id]', + attributes: expect.objectContaining({ 'url.path': '/users/123' }), + }), + expect.any(Function), + ); + }); + describe('with span streaming enabled', () => { beforeEach(() => { vi.spyOn(SentryCore, 'getClient').mockImplementation( diff --git a/packages/sveltekit/test/common/utils.test.ts b/packages/sveltekit/test/common/utils.test.ts index 6650a491c80f..015f6b8ac573 100644 --- a/packages/sveltekit/test/common/utils.test.ts +++ b/packages/sveltekit/test/common/utils.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest'; -import { getRouteId, isHttpError, isRedirect } from '../../src/common/utils'; +import { getPathname, getRouteId, isHttpError, isRedirect } from '../../src/common/utils'; describe('isRedirect', () => { it.each([ @@ -102,3 +102,32 @@ describe('getRouteId', () => { expect(getRouteId({ route: null })).toBeUndefined(); }); }); + +describe('getPathname', () => { + it('reads url.pathname through untrack when available (SvelteKit 2+)', () => { + const untrack = vi.fn((fn: () => T) => fn()); + const event = { + url: new URL('http://localhost:3000/users/123'), + untrack, + }; + + // @ts-expect-error - only passing a partial load event here + expect(getPathname(event)).toBe('/users/123'); + expect(untrack).toHaveBeenCalledTimes(1); + }); + + it('bypasses tracked url getters without untrack', () => { + const tracked = vi.fn(); + const url = new URL('http://localhost:3000/users/123'); + Object.defineProperty(url, 'pathname', { + get() { + tracked(); + return '/tracked'; + }, + }); + + // @ts-expect-error - only passing a partial load event here + expect(getPathname({ url })).toBe('/users/123'); + expect(tracked).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/sveltekit/test/server-common/load.test.ts b/packages/sveltekit/test/server-common/load.test.ts index 97b3a6390c08..9ebd9aa2a266 100644 --- a/packages/sveltekit/test/server-common/load.test.ts +++ b/packages/sveltekit/test/server-common/load.test.ts @@ -6,7 +6,7 @@ import type { Load, ServerLoad } from '@sveltejs/kit'; import { error, redirect } from '@sveltejs/kit'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { wrapLoadWithSentry, wrapServerLoadWithSentry } from '../../src/server-common/load'; -import { getDefaultNodeClientOptions } from '../utils'; +import { createTrackedLoadEvent, getDefaultNodeClientOptions } from '../utils'; const mockCaptureException = vi.spyOn(SentryCore, 'captureException').mockImplementation(() => 'xx'); @@ -320,7 +320,7 @@ describe('wrapServerLoadWithSentry calls `startSpan`', () => { const wrappedLoad = wrapServerLoadWithSentry(serverLoad); await wrappedLoad(eventWithUntrack); - expect(untrack).toHaveBeenCalledTimes(1); + expect(untrack).toHaveBeenCalledTimes(2); expect(mockStartSpan).toHaveBeenCalledWith( expect.objectContaining({ name: '/users/[id]', @@ -333,6 +333,36 @@ describe('wrapServerLoadWithSentry calls `startSpan`', () => { }); }); +describe.each([ + ['wrapLoadWithSentry', wrapLoadWithSentry], + ['wrapServerLoadWithSentry', wrapServerLoadWithSentry], +])('%s with untrack (SvelteKit 2+)', (_, sentryLoadWrapperFn) => { + it("doesn't add route or url dependencies to the load function", async () => { + const { event, trackedReads } = createTrackedLoadEvent(getServerOnlyArgs()); + + const wrappedLoad = sentryLoadWrapperFn(() => ({})); + await wrappedLoad(event); + + expect(trackedReads).toEqual([]); + expect(mockStartSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: '/users/[id]', + attributes: expect.objectContaining({ 'url.path': '/users/123' }), + }), + expect.any(Function), + ); + }); + + it('still tracks url reads made by the load function itself', async () => { + const { event, trackedReads } = createTrackedLoadEvent(getServerOnlyArgs()); + + const wrappedLoad = sentryLoadWrapperFn(({ url }: { url: URL }) => ({ path: url.pathname })); + await wrappedLoad(event); + + expect(trackedReads).toEqual(['url.pathname']); + }); +}); + describe('with span streaming enabled', () => { beforeEach(() => { vi.spyOn(SentryCore, 'getClient').mockImplementation( diff --git a/packages/sveltekit/test/utils.ts b/packages/sveltekit/test/utils.ts index 29d32e8df2c7..3f8e0deca433 100644 --- a/packages/sveltekit/test/utils.ts +++ b/packages/sveltekit/test/utils.ts @@ -11,3 +11,42 @@ export function getDefaultNodeClientOptions(options: Partial = {} ...options, }; } + +/** + * Mimics SvelteKit's dependency tracking: reads of `url` and `route` members are recorded + * unless they happen inside `untrack`. + */ +export function createTrackedLoadEvent( + event: T & { url: URL; route: { id: string | null } }, +): { event: T & { untrack: (fn: () => R) => R }; trackedReads: string[] } { + const trackedReads: string[] = []; + let tracking = true; + + const track = (target: O, name: string): O => + new Proxy(target, { + get(target, prop) { + if (tracking && typeof prop === 'string') { + trackedReads.push(`${name}.${prop}`); + } + const value = Reflect.get(target, prop); + return typeof value === 'function' ? value.bind(target) : value; + }, + }); + + return { + event: { + ...event, + url: track(event.url, 'url'), + route: track(event.route, 'route'), + untrack: (fn: () => R): R => { + tracking = false; + try { + return fn(); + } finally { + tracking = true; + } + }, + }, + trackedReads, + }; +} From fa1eba636a117dc74fe96619441196e1a68ea638 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 9 Oct 2026 16:22:00 +0200 Subject: [PATCH 2/3] test(sveltekit): Use URL_PATH constant in load assertions Co-Authored-By: Claude Opus 5.5 --- packages/sveltekit/test/client/load.test.ts | 4 ++-- packages/sveltekit/test/server-common/load.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/sveltekit/test/client/load.test.ts b/packages/sveltekit/test/client/load.test.ts index cd3cf233486a..c975b7e9fcd9 100644 --- a/packages/sveltekit/test/client/load.test.ts +++ b/packages/sveltekit/test/client/load.test.ts @@ -1,4 +1,4 @@ -import { SENTRY_SEGMENT_NAME_SOURCE, SENTRY_OP, SENTRY_ORIGIN } from '@sentry/conventions/attributes'; +import { SENTRY_SEGMENT_NAME_SOURCE, SENTRY_OP, SENTRY_ORIGIN, URL_PATH } from '@sentry/conventions/attributes'; import type { Client } from '@sentry/core'; import * as SentryCore from '@sentry/core'; import * as SentrySvelte from '@sentry/svelte'; @@ -191,7 +191,7 @@ describe('wrapLoadWithSentry', () => { expect(mockStartSpan).toHaveBeenCalledWith( expect.objectContaining({ name: '/users/[id]', - attributes: expect.objectContaining({ 'url.path': '/users/123' }), + attributes: expect.objectContaining({ [URL_PATH]: '/users/123' }), }), expect.any(Function), ); diff --git a/packages/sveltekit/test/server-common/load.test.ts b/packages/sveltekit/test/server-common/load.test.ts index 9ebd9aa2a266..9758c4852903 100644 --- a/packages/sveltekit/test/server-common/load.test.ts +++ b/packages/sveltekit/test/server-common/load.test.ts @@ -1,4 +1,4 @@ -import { SENTRY_SEGMENT_NAME_SOURCE, SENTRY_OP, SENTRY_ORIGIN } from '@sentry/conventions/attributes'; +import { SENTRY_SEGMENT_NAME_SOURCE, SENTRY_OP, SENTRY_ORIGIN, URL_PATH } from '@sentry/conventions/attributes'; import type { Client, Event } from '@sentry/core'; import * as SentryCore from '@sentry/core'; import { NodeClient, setCurrentClient } from '@sentry/node'; @@ -347,7 +347,7 @@ describe.each([ expect(mockStartSpan).toHaveBeenCalledWith( expect.objectContaining({ name: '/users/[id]', - attributes: expect.objectContaining({ 'url.path': '/users/123' }), + attributes: expect.objectContaining({ [URL_PATH]: '/users/123' }), }), expect.any(Function), ); From d66cf2b5a228d3b76f0d6abc79119498067e1ae9 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Mon, 12 Oct 2026 11:22:02 +0200 Subject: [PATCH 3/3] test(sveltekit): Add e2e test for layout loads not rerunning on navigation Covers #25231 in a real app: a wrapped layout server load that doesn't read url must not rerun when navigating between its child pages. Co-Authored-By: Claude Opus 5.5 --- .../load-dependencies/+layout.server.ts | 7 +++++++ .../routes/load-dependencies/+layout.svelte | 6 ++++++ .../routes/load-dependencies/a/+page.svelte | 2 ++ .../routes/load-dependencies/b/+page.svelte | 1 + .../tests/load-dependencies.test.ts | 20 +++++++++++++++++++ 5 files changed, 36 insertions(+) create mode 100644 dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/+layout.server.ts create mode 100644 dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/+layout.svelte create mode 100644 dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/a/+page.svelte create mode 100644 dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/b/+page.svelte create mode 100644 dev-packages/e2e-tests/test-applications/sveltekit-2/tests/load-dependencies.test.ts diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/+layout.server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/+layout.server.ts new file mode 100644 index 000000000000..dc09d4b51030 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/+layout.server.ts @@ -0,0 +1,7 @@ +let layoutLoadCount = 0; + +// Doesn't read `url`, so SvelteKit should only run it once across child navigations. +export const load = async () => { + layoutLoadCount++; + return { layoutLoadCount }; +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/+layout.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/+layout.svelte new file mode 100644 index 000000000000..44e7f425d54b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/+layout.svelte @@ -0,0 +1,6 @@ + + +

Layout loads: {data.layoutLoadCount}

+ diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/a/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/a/+page.svelte new file mode 100644 index 000000000000..6039c292a519 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/a/+page.svelte @@ -0,0 +1,2 @@ +

Page A

+Go to B diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/b/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/b/+page.svelte new file mode 100644 index 000000000000..f298fcc8d37b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/load-dependencies/b/+page.svelte @@ -0,0 +1 @@ +

Page B

diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/tests/load-dependencies.test.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2/tests/load-dependencies.test.ts new file mode 100644 index 000000000000..1d1cf49de0f0 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/tests/load-dependencies.test.ts @@ -0,0 +1,20 @@ +import { expect, test } from '@playwright/test'; +import { getSpanOp, waitForStreamedSpan } from '@sentry-internal/test-utils'; +import { waitForInitialPageload } from './utils'; + +test("doesn't rerun a wrapped layout server load that doesn't depend on the url", async ({ page }) => { + await waitForInitialPageload(page, { route: '/load-dependencies/a' }); + + const layoutLoadCount = page.locator('#layout-load-count'); + const initialText = await layoutLoadCount.textContent(); + + const navigationSpanPromise = waitForStreamedSpan('sveltekit-2', span => { + return span.name === '/load-dependencies/b' && getSpanOp(span) === 'navigation' && span.is_segment; + }); + + await page.getByText('Go to B').click(); + await expect(page.getByText('Page B')).toBeVisible(); + await navigationSpanPromise; + + await expect(layoutLoadCount).toHaveText(initialText!); +});