fix: support absolute URLs with data-sveltekit-preload-code="viewport"

[cooolbros]

Currently, the IntersectionObserver used for viewport code preloading uses the .href attribute of the anchor tag as the pathname to find the route to preload the code for.

Internally, the InterSectionObserver gets the .href and passes the value to _preload_code which calls routes.find, finding the first route where the route.exec regex match is true

In Chrome and Firefox, the .href value is an absolute URL of the anchor tag value (for example http://localhost:5173/post/123), and not the relative pathname (/post/123), resulting in the route.exec regex check failing for every route, as it is comparing the relative URLs (from dictionary in app.js) with the absolute URL provided by the browser.

This pull request converts the href provided by the browser to a relative URL which then correctly preloads code, as Sveltekit can find the route in the dictionary.

The pull request uses the existing method for converting browser hrefs to relative pathnames as used in get_navigation_intent, meaning it will also ignore external URLs and preload code for routes that are rewritten using Sveltekit rewrites

---

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.

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:.

Edits

• Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

[changeset-bot]

🦋 Changeset detected

Latest commit: d12c376

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

[benmccann]

It would be good to have a test for this. Do we have any tests currently for preloading code? I'm a bit surprised this wouldn't have been caught by our test suite, but maybe this is a hard thing to test?

[cooolbros]

I added tests for preloadCode and data-sveltekit-preload-code.

Note that because the files are named like nodes/103.js, we can't know the file name at test time and can only guess that the file was preloaded by checking the page network requests. This is the same method used in the data-sveltekit-preload-data tests.

There are still times when the IntersectionObserver preload can silently fail, such as if the user tries to preload an internal pathname that doesn't exist. If it is a good idea to add a warning in DEV when this happens, I can update the pull request with that warning.

[PatrickG]

Wouldn't it be enough to change line 1531?

-_preload_code(/** @type {HTMLAnchorElement} */ (entry.target).href);
+_preload_code(/** @type {HTMLAnchorElement} */ (entry.target).pathname);

nvm. It wouldn't run the reroute hook.

BTW: _preload_code() in line 1586 does not run the reroute hook either.
Does it need to run at all for preloading the code? yes

[cooolbros]

I moved the href resolver to the _preload_code function so that this fix will also apply for data-sveltekit-preload-code="eager", instead of doing the conversion in the IntersectionObserver.

However this also means the reroute hook will also run for the preloadCode exported function, which could cause a mismatch between the real requested route and the error messages displayed by the preloadCode DEV mode checks

[benmccann]

waitForTimeout is an anti-pattern. can we avoid it? why do we need both it and networkidle?

[cooolbros]

waitForTimeout is an anti-pattern. can we avoid it? why do we need both it and networkidle?

It was used in the data-sveltekit-preload-data tests because the hover preload waits before preloading a link. I just copied the test conditions for consistency.

I'll remove the waitForTimeout because they are not needed here

[eltigerchino]

I think it may be needed here just so that there's time for the network requests to fire. Removing them causes the tests to fail with 0 network requests. Other preload tests have the same pattern.

[benmccann]

The build test failures seem to be real since they're failing in every browser. Could you take a look at those?

[cooolbros]

The build test failures seem to be real since they're failing in every browser. Could you take a look at those?

I'll take a look

[eltigerchino]

I've fixed the build test that was failing. It involved the newly added test expecting a request URL from the route path instead of the chunk path (which is used for CSR).

[Rich-Harris]

preview: https://svelte-dev-git-preview-kit-12217-svelte.vercel.app/

this is an automated message

[dummdidumm]

Thank you @cooolbros for the PR and @eltigerchino for finishing my bad merge 😄

btw it may be that this was already fixed in the meantime when we switched _preload_code from a pathname string to the full URL, but this PR also fixes the case where a reroute would change the destination route, so it's a good fix regardless

unavoidable in this case