fix: use Proxy to track usage of client-side load event.route

[Lms24]

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it. (This is already covered by a test)

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Referenced issues:

• Add support for handleLoad hook on client and server hooks #9542 (comment)
• Using the SvelteKit SDK breaks SSR functionality of TanStack Query getsentry/sentry-javascript#8818
• [Sveltekit] Sveltekit auto instrumentation in vite plugin causes load functions to invalidate on every route change getsentry/sentry-javascript#8610

Background

Our Sentry SvelteKit SDK exposes wrap(Server)LoadWithSentry functions that users can manually or with the help of our VIte plugin, automatically wrap around their load functions. In these functions we access the load event's route.id value to create a span that tracks the duration of a load function and what happens inside it (e.g. fetch calls).

This is currently problematic because SvelteKit internally tracks accesses to route.id by setting a getter (client side) or a Proxy (server side for server only loads). If users themselves don't use route but only our wrapper does, they get confused why their loaded data is invalidated on route changes. It's important to note that Sentry doesn't change any behaviour based on route.id, we just need to access it to get the name of the current route, nothing more.

PR Description

This PR changes the client-side uses.route tracking mechanism to use a Proxy instead of directly using a getter. This allows us in Sentry's wrap(Server)LoadWithSentry functions to use Object.getOwnPropertyDescriptor to access event.route.id without triggering the uses.route change. We already did this successfully for the server load wrapper and we'd like to do the same for the client side of universal load functions. This allows us to record a high-fidelity load span (useful for SvelteKit/our users).

I believe correct client-side invalidation based on route is already tested but I'm happy to add more tests if reviewers request it.

Would be awesome if we could get this merged in! 🙏

[changeset-bot]

🦋 Changeset detected

Latest commit: 92328ee

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[eltigerchino]

Would #6294 be a more permanent solution?
One of the suggestions is to have a new property event.untracked that contains all the properties that would usually be tracked but without the tracking. #6294 (comment)
Then, we could just read from event.untracked.route.id

[Lms24]

Oh nice, that would definitely be a great solution. Can we help expedite this?

Otherwise I'd appreciate it if we could get this small fix in first to unblock our users and then work on a more general solution (still happy to help out with that!). Thanks :)

[eltigerchino]

I asked in the maintainer discord and I think we may focus more on solving this with load hooks / opting out of load invalidation tracking. Currently, this workaround would lead to a breaking change, and again when removing it.

[Lms24]

Out of curioisity, how is this PR a breaking change?

[eltigerchino]

Whoops, I misunderstood the discord chat. Only removing it would lead to a breaking change.

[Lms24]

Is there any chance that this will get merged in? The current behaviour is a major blocker for us and for our customers.