Skip to content

Commit

Permalink
fix: tighten up preloadCode (#12217)
Browse files Browse the repository at this point in the history
support absolute URLs with `data-sveltekit-preload-code="viewport"` and reroutes for preloadCode in general

---------

Co-authored-by: Ben McCann <[email protected]>
Co-authored-by: Chew Tee Ming <[email protected]>
Co-authored-by: Simon H <[email protected]>
  • Loading branch information
4 people authored Jan 17, 2025
1 parent 0142dd8 commit 777c8ef
Show file tree
Hide file tree
Showing 16 changed files with 163 additions and 19 deletions.
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 and reroutes with `data-sveltekit-preload-code="viewport"`
53 changes: 39 additions & 14 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -426,9 +426,15 @@ async function _preload_data(intent) {
return load_cache.promise;
}

/** @param {URL} url */
/**
* @param {URL} url
* @returns {Promise<void>}
*/
async function _preload_code(url) {
const route = routes.find((route) => route.exec(get_url_path(url)));
const rerouted = get_rerouted_url(url);
if (!rerouted) return;

const route = routes.find((route) => route.exec(get_url_path(rerouted)));

if (route) {
await Promise.all([...route.layouts, route.leaf].map((load) => load?.[1]()));
Expand Down Expand Up @@ -1189,16 +1195,11 @@ 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
* @param {URL} url
* @returns {URL | undefined}
*/
function get_navigation_intent(url, invalidating) {
if (!url) return;
if (is_external_url(url, base, app.hash)) return;

function get_rerouted_url(url) {
// reroute could alter the given URL, so we pass a copy
let rerouted;
try {
Expand All @@ -1225,9 +1226,26 @@ function get_navigation_intent(url, invalidating) {
}

// fall back to native navigation
return undefined;
return;
}

return rerouted;
}

/**
* 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, app.hash)) return;

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

const path = get_url_path(rerouted);

for (const route of routes) {
Expand Down Expand Up @@ -1942,13 +1960,20 @@ export function preloadCode(pathname) {
const url = new URL(pathname, current.url);

if (DEV) {
if (!pathname.startsWith('/')) {
throw new Error(
'argument passed to preloadCode must be a pathname (i.e. "/about" rather than "http://example.com/about"'
);
}

if (!pathname.startsWith(base)) {
throw new Error(
`pathnames passed to preloadCode must start with \`paths.base\` (i.e. "${base}${pathname}" rather than "${pathname}")`
`pathname passed to preloadCode must start with \`paths.base\` (i.e. "${base}${pathname}" rather than "${pathname}")`
);
}

if (!routes.find((route) => route.exec(get_url_path(url)))) {
const rerouted = get_rerouted_url(url);
if (!rerouted || !routes.find((route) => route.exec(get_url_path(rerouted)))) {
throw new Error(`'${pathname}' did not match any routes`);
}
}
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 @@ -824,6 +824,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')]);
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
35 changes: 34 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 @@ -601,7 +601,40 @@ 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 {
expect(requests.filter((req) => /\/_app\/immutable\/nodes\/.*?.js$/.test(req)).length).toBe(
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(
'argument passed to preloadCode must be a pathname (i.e. "/about" rather than "http://example.com/about"'
);
}
}
});

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 @@ -18,7 +18,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?: Parameters<Page['waitForNavigation']>[0]): Promise<void>;
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/test/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ 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((pathname) => preloadCode(pathname), pathname),

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

0 comments on commit 777c8ef

Please sign in to comment.