Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fluffy-coats-laugh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@sveltejs/kit": patch
---

fix: support absolute URLs with `data-sveltekit-preload-code="viewport"`
41 changes: 27 additions & 14 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -407,9 +407,14 @@ async function _preload_data(intent) {
return load_cache.promise;
}

/** @param {string} pathname */
async function _preload_code(pathname) {
const route = routes.find((route) => route.exec(get_url_path(pathname)));
/** @param {string} href */
async function _preload_code(href) {
const url = get_navigation_url(resolve_url(href));
if (!url) return;

const pathname = get_url_path(url);

const route = routes.find((route) => route.exec(pathname));

if (route) {
await Promise.all([...route.layouts, route.leaf].map((load) => load?.[1]()));
Expand Down Expand Up @@ -1139,20 +1144,13 @@ async function load_root_error_page({ status, error, url, route }) {
}

/**
* Resolve the full info (which route, params, etc.) for a client-side navigation from the URL,
* taking the reroute hook into account. If this isn't a client-side-navigation (or the URL is undefined),
* returns undefined.
* @param {URL | undefined} url
* @param {boolean} invalidating
* Resolve the relative rerouted URL for a client-side navigation from the URL
* @param {URL} url
*/
function get_navigation_intent(url, invalidating) {
if (!url) return undefined;
if (is_external_url(url, base)) return;

function get_navigation_url(url) {
// reroute could alter the given URL, so we pass a copy
let rerouted;
try {
rerouted = app.hooks.reroute({ url: new URL(url) }) ?? url.pathname;
return app.hooks.reroute({ url: new URL(url) }) ?? url.pathname;
} catch (e) {
if (DEV) {
// in development, print the error...
Expand All @@ -1165,6 +1163,21 @@ function get_navigation_intent(url, invalidating) {
// fall back to native navigation
return undefined;
}
}

/**
* Resolve the full info (which route, params, etc.) for a client-side navigation from the URL,
* taking the reroute hook into account. If this isn't a client-side-navigation (or the URL is undefined),
* returns undefined.
* @param {URL | undefined} url
* @param {boolean} invalidating
*/
function get_navigation_intent(url, invalidating) {
if (!url) return;
if (is_external_url(url, base)) return;

const rerouted = get_navigation_url(url);
if (!rerouted) return;

const path = get_url_path(rerouted);

Expand Down
2 changes: 1 addition & 1 deletion packages/kit/test/ambient.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ declare global {
const preloadData: (url: string) => Promise<void>;
const beforeNavigate: (fn: (url: URL) => void | boolean) => void;
const afterNavigate: (fn: () => void) => void;
const preloadCode: (...urls: string[]) => Promise<void>;
const preloadCode: (pathname: string) => Promise<void>;
}

export {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<div style="height: 200vh"></div>

<a
id="viewport"
href="/data-sveltekit/preload-code/target/viewport"
data-sveltekit-preload-code="viewport">viewport</a
>
<a id="hover" href="/data-sveltekit/preload-code/target/hover" data-sveltekit-preload-code="hover"
>hover</a
>
<a id="tap" href="/data-sveltekit/preload-code/target/tap" data-sveltekit-preload-code="tap">tap</a>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export function load() {
return {
name: 'eager'
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
export let data;
</script>

<h1>{data.name}</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export function load() {
return {
name: 'hover'
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
export let data;
</script>

<h1>{data.name}</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export function load() {
return {
name: 'tap'
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
export let data;
</script>

<h1>{data.name}</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export function load() {
return {
name: 'hover'
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
export let data;
</script>

<h1>{data.name}</h1>
30 changes: 30 additions & 0 deletions packages/kit/test/apps/basics/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,36 @@ test.describe('Invalidation', () => {
});

test.describe('data-sveltekit attributes', () => {
test('data-sveltekit-preload-code', async ({ page }) => {
/** @type {string[]} */
const requests = [];
page.on('request', (r) => {
requests.push(r.url());
});

// eager
await page.goto('/data-sveltekit/preload-code');
expect(requests.length).toBeGreaterThanOrEqual(1);

// viewport
requests.length = 0;
page.locator('#viewport').scrollIntoViewIfNeeded();
await Promise.all([page.waitForTimeout(100), page.waitForLoadState('networkidle')]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@teemingc teemingc Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

expect(requests.length).toBeGreaterThanOrEqual(1);

// hover
requests.length = 0;
await page.locator('#hover').dispatchEvent('mousemove');
await Promise.all([page.waitForTimeout(100), page.waitForLoadState('networkidle')]);
expect(requests.length).toBeGreaterThanOrEqual(1);

// tap
requests.length = 0;
await page.locator('#tap').dispatchEvent('touchstart');
await Promise.all([page.waitForTimeout(100), page.waitForLoadState('networkidle')]);
expect(requests.length).toBeGreaterThanOrEqual(1);
});

test('data-sveltekit-preload-data', async ({ page }) => {
/** @type {string[]} */
const requests = [];
Expand Down
34 changes: 33 additions & 1 deletion packages/kit/test/apps/basics/test/cross-platform/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,39 @@ test.describe.serial('Errors', () => {
});

test.describe('Prefetching', () => {
test('prefetches programmatically', async ({ baseURL, page, app }) => {
test('prefetches code programmatically', async ({ page, app }) => {
await page.goto('/routing/a');

/** @type {string[]} */
const requests = [];
page.on('request', (r) => {
requests.push(r.url());
});

await app.preloadCode('/routing/b');

// svelte request made is environment dependent
if (process.env.DEV) {
expect(requests.filter((req) => req.endsWith('routing/b/+page.js')).length).toBe(1);
expect(requests.filter((req) => req.endsWith('routing/b/+page.svelte')).length).toBe(1);
} else {
// request should match "routing/b/[node].js"
expect(
requests.filter((req) => /.*?\/routing\/b\/\d+\.js$/.test(req)).length
).toBeGreaterThan(1);
}

if (process.env.DEV) {
try {
await app.preloadCode('https://example.com');
throw new Error('Error was not thrown');
} catch (/** @type {any} */ e) {
expect(e.message).toMatch("'https://example.com' did not match any routes");
}
}
});

test('prefetches data programmatically', async ({ baseURL, page, app }) => {
await page.goto('/routing/a');

/** @type {string[]} */
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/test/utils.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const test: TestType<
invalidate(url: string): Promise<void>;
beforeNavigate(url: URL): void | boolean;
afterNavigate(url: URL): void;
preloadCode(...urls: string[]): Promise<void>;
preloadCode(pathname: string): Promise<void>;
preloadData(url: string): Promise<void>;
};
clicknav(selector: string, options?: { timeout?: number }): Promise<void>;
Expand Down
5 changes: 3 additions & 2 deletions packages/kit/test/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,11 @@ export const test = base.extend({
afterNavigate: () => page.evaluate(() => afterNavigate(() => {})),

/**
* @param {string[]} urls
* @param {string} pathname
* @returns {Promise<void>}
*/
preloadCode: (...urls) => page.evaluate((urls) => preloadCode(...urls), urls),
preloadCode: (pathname) =>
page.evaluate((/** @type {string} */ pathname) => preloadCode(pathname), pathname),

/**
* @param {string} url
Expand Down