diff --git a/.changeset/good-roses-design.md b/.changeset/good-roses-design.md new file mode 100644 index 000000000000..b2055ea2c9a3 --- /dev/null +++ b/.changeset/good-roses-design.md @@ -0,0 +1,5 @@ +--- +"@sveltejs/kit": patch +--- + +fix: prevent navigation when `data-sveltekit-preload-data` fails to fetch due to network error diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index dd400677931f..69e3a61b44ab 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -172,7 +172,7 @@ const invalidated = []; */ const components = []; -/** @type {{id: string, promise: Promise} | null} */ +/** @type {{id: string, token: {}, promise: Promise} | null} */ let load_cache = null; /** @type {Array<(navigation: import('@sveltejs/kit').BeforeNavigate) => void>} */ @@ -219,6 +219,14 @@ let page; /** @type {{}} */ let token; +/** + * A set of tokens which are associated to current preloads. + * If a preload becomes a real navigation, it's removed from the set. + * If a preload token is in the set and the preload errors, the error + * handling logic (for example reloading) is skipped. + */ +const preload_tokens = new Set(); + /** @type {Promise | null} */ let pending_invalidate; @@ -375,16 +383,26 @@ async function _goto(url, options, redirect_count, nav_token) { /** @param {import('./types.js').NavigationIntent} intent */ async function _preload_data(intent) { - load_cache = { - id: intent.id, - promise: load_route(intent).then((result) => { - if (result.type === 'loaded' && result.state.error) { - // Don't cache errors, because they might be transient - load_cache = null; - } - return result; - }) - }; + // Reuse the existing pending preload if it's for the same navigation. + // Prevents an edge case where same preload is triggered multiple times, + // then a later one is becoming the real navigation and the preload tokens + // get out of sync. + if (intent.id !== load_cache?.id) { + const preload = {}; + preload_tokens.add(preload); + load_cache = { + id: intent.id, + token: preload, + promise: load_route({ ...intent, preload }).then((result) => { + preload_tokens.delete(preload); + if (result.type === 'loaded' && result.state.error) { + // Don't cache errors, because they might be transient + load_cache = null; + } + return result; + }) + }; + } return load_cache.promise; } @@ -803,11 +821,31 @@ function diff_search_params(old_url, new_url) { } /** - * @param {import('./types.js').NavigationIntent} intent + * @param {Omit & { error: App.Error }} opts + * @returns {import('./types.js').NavigationFinished} + */ +function preload_error({ error, url, route, params }) { + return { + type: 'loaded', + state: { + error, + url, + route, + params, + branch: [] + }, + props: { page, constructors: [] } + }; +} + +/** + * @param {import('./types.js').NavigationIntent & { preload?: {} }} intent * @returns {Promise} */ -async function load_route({ id, invalidating, url, params, route }) { +async function load_route({ id, invalidating, url, params, route, preload }) { if (load_cache?.id === id) { + // the preload becomes the real navigation + preload_tokens.delete(load_cache.token); return load_cache.promise; } @@ -855,9 +893,15 @@ async function load_route({ id, invalidating, url, params, route }) { try { server_data = await load_data(url, invalid_server_nodes); } catch (error) { + const handled_error = await handle_error(error, { url, params, route: { id } }); + + if (preload_tokens.has(preload)) { + return preload_error({ error: handled_error, url, params, route }); + } + return load_root_error_page({ status: get_status(error), - error: await handle_error(error, { url, params, route: { id: route.id } }), + error: handled_error, url, route }); @@ -940,6 +984,15 @@ async function load_route({ id, invalidating, url, params, route }) { }; } + if (preload_tokens.has(preload)) { + return preload_error({ + error: await handle_error(err, { params, url, route: { id: route.id } }), + url, + params, + route + }); + } + let status = get_status(err); /** @type {App.Error} */ let error; @@ -972,8 +1025,6 @@ async function load_route({ id, invalidating, url, params, route }) { route }); } else { - // if we get here, it's because the root `load` function failed, - // and we need to fall back to the server return await server_fallback(url, { id: route.id }, error, status); } } diff --git a/packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/+layout.server.js b/packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/+layout.server.js new file mode 100644 index 000000000000..9d06c5313fd0 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/+layout.server.js @@ -0,0 +1,5 @@ +export function load({ url }) { + return { + url: url.toString() + }; +} diff --git a/packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/+page.svelte b/packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/+page.svelte new file mode 100644 index 000000000000..15e990e00a5c --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/+page.svelte @@ -0,0 +1,8 @@ +target + +slow-navigation diff --git a/packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/slow-navigation/+page.server.js b/packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/slow-navigation/+page.server.js new file mode 100644 index 000000000000..e7f852a46324 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/slow-navigation/+page.server.js @@ -0,0 +1,5 @@ +export async function load() { + return new Promise((resolve) => { + setTimeout(resolve, 1000); + }); +} diff --git a/packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/slow-navigation/+page.svelte b/packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/slow-navigation/+page.svelte new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/target/+page.svelte b/packages/kit/test/apps/basics/src/routes/data-sveltekit/preload-data/offline/target/+page.svelte new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index 2ae2b2818b74..4df2ba593000 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -665,6 +665,78 @@ test.describe('data-sveltekit attributes', () => { expect(requests.length).toBe(0); }); + test('data-sveltekit-preload-data network failure does not trigger navigation', async ({ + page, + context, + browserName + }) => { + await page.goto('/data-sveltekit/preload-data/offline'); + + await context.setOffline(true); + + await page.locator('#one').dispatchEvent('mousemove'); + await Promise.all([ + page.waitForTimeout(100), // wait for preloading to start + page.waitForLoadState('networkidle') // wait for preloading to finish + ]); + + let offline_url = /\/data-sveltekit\/preload-data\/offline/; + if (browserName === 'chromium') { + // it's chrome-error://chromewebdata/ on ubuntu but not on windows + offline_url = /chrome-error:\/\/chromewebdata\/|\/data-sveltekit\/preload-data\/offline/; + } + expect(page).toHaveURL(offline_url); + }); + + test('data-sveltekit-preload-data error does not block user navigation', async ({ + page, + context, + browserName + }) => { + await page.goto('/data-sveltekit/preload-data/offline'); + + await context.setOffline(true); + + await page.locator('#one').dispatchEvent('mousemove'); + await Promise.all([ + page.waitForTimeout(100), // wait for preloading to start + page.waitForLoadState('networkidle') // wait for preloading to finish + ]); + + expect(page).toHaveURL('/data-sveltekit/preload-data/offline'); + + await page.locator('#one').dispatchEvent('click'); + await page.waitForTimeout(100); // wait for navigation to start + await page.waitForLoadState('networkidle'); + + let offline_url = /\/data-sveltekit\/preload-data\/offline/; + if (browserName === 'chromium') { + // it's chrome-error://chromewebdata/ on ubuntu but not on windows + offline_url = /chrome-error:\/\/chromewebdata\/|\/data-sveltekit\/preload-data\/offline/; + } + expect(page).toHaveURL(offline_url); + }); + + test('data-sveltekit-preload does not abort ongoing navigation', async ({ + page, + browserName + }) => { + await page.goto('/data-sveltekit/preload-data/offline'); + + await page.locator('#slow-navigation').dispatchEvent('click'); + await page.waitForTimeout(100); // wait for navigation to start + await page.locator('#slow-navigation').dispatchEvent('mousemove'); + await Promise.all([ + page.waitForTimeout(100), // wait for preloading to start + page.waitForLoadState('networkidle') // wait for preloading to finish + ]); + + expect(page).toHaveURL( + '/data-sveltekit/preload-data/offline/slow-navigation' || + (browserName === 'chromium' && 'chrome-error://chromewebdata/') + ); + }); + test('data-sveltekit-reload', async ({ baseURL, page, clicknav }) => { /** @type {string[]} */ const requests = [];