diff --git a/.changeset/quick-error-preloads.md b/.changeset/quick-error-preloads.md new file mode 100644 index 000000000000..f2d48b2216d0 --- /dev/null +++ b/.changeset/quick-error-preloads.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: preload error page modules required for client rendering diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js index 3b50240b9515..9933d5bfd372 100644 --- a/packages/kit/src/runtime/server/page/index.js +++ b/packages/kit/src/runtime/server/page/index.js @@ -148,7 +148,8 @@ export async function render_page(event, state, page, nodes, resolve_opts) { event, state, resolve_opts, - data_serializer: server_data_serializer(event, state) + data_serializer: server_data_serializer(event, state), + ...(csr ? await load_error_components(false, nodes.data, page) : {}) }); } @@ -295,7 +296,7 @@ export async function render_page(event, state, page, nodes, resolve_opts) { }, status, error, - error_components: await load_error_components(ssr, error_branch, page), + ...(await load_error_components(ssr, error_branch, page)), branch: error_branch, fetched, data_serializer @@ -343,7 +344,7 @@ export async function render_page(event, state, page, nodes, resolve_opts) { action_result, fetched, data_serializer: !ssr ? server_data_serializer(event, state) : data_serializer, - error_components: await load_error_components(ssr, branch, page) + ...(await load_error_components(ssr, branch, page)) }); } catch (e) { // a remote function could have thrown a redirect during render @@ -364,13 +365,20 @@ export async function render_page(event, state, page, nodes, resolve_opts) { /** * @param {boolean} ssr - * @param {Array} branch + * @param {Array} branch * @param {PageNodeIndexes} page */ -function load_error_components(ssr, branch, page) { - if (!ssr) return undefined; - - return build_error_chain(branch, page.errors, (idx) => - manifest.nodes[idx]?.().then((e) => e.component?.()) - ); +async function load_error_components(ssr, branch, page) { + /** @type {SSRNode[]} */ + const error_nodes = []; + const error_components = await build_error_chain(branch, page.errors, async (idx) => { + const node = await manifest.nodes[idx]?.(); + if (!node) return undefined; + + error_nodes.push(node); + // Without SSR, only load the metadata: the component may use browser-only globals. + return ssr ? node.component?.() : undefined; + }); + + return { error_components: ssr ? error_components : undefined, error_nodes }; } diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js index 0a8042eb03fa..8798e8a0ce02 100644 --- a/packages/kit/src/runtime/server/page/render.js +++ b/packages/kit/src/runtime/server/page/render.js @@ -53,7 +53,8 @@ import * as w from '../../../messages/server-warnings.js'; * resolve_opts: import('types').RequiredResolveOptions; * action_result?: import('types').ServerActionResult; * data_serializer: import('./types.js').ServerDataSerializer; - * error_components?: Array + * error_components?: Array; + * error_nodes?: import('types').SSRNode[] * }} opts */ export async function render_response({ @@ -67,7 +68,8 @@ export async function render_response({ resolve_opts, action_result, data_serializer, - error_components + error_components, + error_nodes = [] }) { if (state.prerendering || state.prerender_default === true) { if (options.csp.mode === 'nonce') { @@ -257,7 +259,6 @@ export async function render_response({ } for (const { node } of branch) { - for (const url of node.imports) modulepreloads.add(url); for (const url of node.stylesheets) stylesheets.add(url); for (const font of node.fonts) fonts.set(font.file, font); @@ -367,6 +368,16 @@ export async function render_response({ } if (!client.inline) { + // The client awaits the root error page and the route's error chain before rendering. + // Only preload their JS; unused error-page styles must not affect a successful page. + const preload_nodes = new Map(branch.map(({ node }) => [node.index, node])); + for (const node of error_nodes) preload_nodes.set(node.index, node); + if (!preload_nodes.has(1)) preload_nodes.set(1, await manifest.nodes[1]()); + + for (const node of preload_nodes.values()) { + for (const url of node.imports) modulepreloads.add(url); + } + for (const dep of modulepreloads) { const path = prefixed(dep); diff --git a/packages/kit/test/apps/basics/src/routes/no-ssr/error-preload/+error.svelte b/packages/kit/test/apps/basics/src/routes/no-ssr/error-preload/+error.svelte new file mode 100644 index 000000000000..d3150e6f94a9 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/no-ssr/error-preload/+error.svelte @@ -0,0 +1,5 @@ + + +

{title}

diff --git a/packages/kit/test/apps/basics/src/routes/no-ssr/error-preload/+page.svelte b/packages/kit/test/apps/basics/src/routes/no-ssr/error-preload/+page.svelte new file mode 100644 index 000000000000..bb09d48a1d3d --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/no-ssr/error-preload/+page.svelte @@ -0,0 +1 @@ +

No SSR with a browser-only error page

diff --git a/packages/kit/test/apps/basics/test/playwright/client.test.js b/packages/kit/test/apps/basics/test/playwright/client.test.js index 09b135b4a88b..e3a362903c4f 100644 --- a/packages/kit/test/apps/basics/test/playwright/client.test.js +++ b/packages/kit/test/apps/basics/test/playwright/client.test.js @@ -19,6 +19,43 @@ test.describe('a11y', () => { }); }); +test.describe('Hydration preloads', () => { + for (const path of [ + '/', + '/errors/nested-error-page', + '/errors/nested-error-page/nope', + '/non-existent-route', + '/no-ssr', + '/no-ssr/error-preload', + '/prerendering/env/prerendered' + ]) { + test(`preloads error modules before hydration on ${path}`, async ({ page }) => { + test.skip(!!process.env.DEV, 'modulepreloads are emitted in production builds'); + + /** @type {Set} */ + const modules = new Set(); + page.on('request', (request) => { + if (/\/_app\/immutable\/(nodes|chunks)\/.*\.js$/.test(request.url())) { + modules.add(request.url()); + } + }); + + const response = await page.goto(path); + const html = await /** @type {Response} */ (response).text(); + // Read the original response, not the DOM: Vite inserts preload links during startup. + const preloads = [...html.matchAll(//g)].map( + ([, href]) => new URL(href, page.url()).href + ); + + expect([...modules].some((url) => /\/nodes\/1\./.test(url))).toBe(true); + for (const url of modules) { + expect(preloads).toContain(url); + } + expect(new Set(preloads).size).toBe(preloads.length); + }); + } +}); + test.describe('Caching', () => { test('caches __data.json requests with invalidated search param', async ({ page, app }) => { await page.goto('/'); diff --git a/packages/kit/test/apps/basics/test/playwright/server.test.js b/packages/kit/test/apps/basics/test/playwright/server.test.js index 585a1c496b1c..f793a66264e4 100644 --- a/packages/kit/test/apps/basics/test/playwright/server.test.js +++ b/packages/kit/test/apps/basics/test/playwright/server.test.js @@ -48,6 +48,34 @@ test.describe('Endpoints', () => { }); test.describe('Errors', () => { + test('does not include unused error-page styles in successful SSR responses', async ({ + request + }) => { + test.skip(!!process.env.DEV, 'production builds emit stylesheet links'); + + /** @param {import('@playwright/test').APIResponse} response */ + const stylesheets = async (response) => + [...(await response.text()).matchAll(/]*>/g)].map( + ([, href]) => new URL(href, response.url()).href + ); + + // These successful pages share their layouts and have no page-local CSS. Only the + // second has a nested +error.svelte with styles, which must not be linked until used. + const baseline = await request.get('/errors/load-client'); + const success = await request.get('/errors/nested-error-page'); + expect(baseline.status()).toBe(200); + expect(success.status()).toBe(200); + const layout_styles = await stylesheets(baseline); + expect(layout_styles.length).toBeGreaterThan(0); + expect(await stylesheets(success)).toEqual(layout_styles); + + const failure = await request.get('/errors/nested-error-page/nope'); + expect(failure.status()).toBe(500); + const error_styles = await stylesheets(failure); + for (const href of layout_styles) expect(error_styles).toContain(href); + expect(error_styles).toHaveLength(layout_styles.length + 1); + }); + test('returns 400 when accessing a malformed URI', async ({ page }) => { const response = await page.goto('/%c0%ae%c0%ae/etc/passwd'); if (process.env.DEV) {