diff --git a/.changeset/tasty-geese-hug.md b/.changeset/tasty-geese-hug.md new file mode 100644 index 000000000000..5fd7eee93c49 --- /dev/null +++ b/.changeset/tasty-geese-hug.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +[fix] do not call server and update `$page.data` unnecessarily diff --git a/packages/kit/src/core/sync/write_client_manifest.js b/packages/kit/src/core/sync/write_client_manifest.js index 25456c6c68df..ff200ae13fab 100644 --- a/packages/kit/src/core/sync/write_client_manifest.js +++ b/packages/kit/src/core/sync/write_client_manifest.js @@ -48,23 +48,17 @@ export function write_client_manifest(manifest_data, output) { .map((route) => { if (route.page) { const errors = route.page.errors.slice(1).map((n) => n ?? ''); - const layouts = route.page.layouts.slice(1).map((n) => n ?? ''); + const layouts = route.page.layouts.slice(1).map((n) => { + if (n == undefined) { + return ''; + } + return get_node_id(manifest_data.nodes, n); + }); while (layouts.at(-1) === '') layouts.pop(); while (errors.at(-1) === '') errors.pop(); - /** @type {import('types').PageNode | null} */ - let current_node = route.leaf; - - let uses_server_data = false; - while (current_node && !uses_server_data) { - uses_server_data = !!current_node?.server; - current_node = current_node?.parent ?? null; - } - - // encode whether or not the route uses the server data - // using the ones' complement, to save space - const array = [`${uses_server_data ? '~' : ''}${route.page.leaf}`]; + const array = [get_node_id(manifest_data.nodes, route.page.leaf)]; // only include non-root layout/error nodes if they exist if (layouts.length > 0 || errors.length > 0) array.push(`[${layouts.join(',')}]`); @@ -77,6 +71,7 @@ export function write_client_manifest(manifest_data, output) { .join(',\n\t\t')} }`.replace(/^\t/gm, ''); + // String representation of __GENERATED__/client-manifest.js write_if_changed( `${output}/client-manifest.js`, trim(` @@ -90,3 +85,13 @@ export function write_client_manifest(manifest_data, output) { `) ); } + +/** + * Encode whether or not the route uses the server data + * using the ones' complement, to save space + * @param {import('types').PageNode[]} nodes + * @param {number} id + */ +function get_node_id(nodes, id) { + return `${nodes[id].server ? '~' : ''}${id}`; +} diff --git a/packages/kit/src/runtime/client/ambient.d.ts b/packages/kit/src/runtime/client/ambient.d.ts index 74164d55c347..ac822de8ac03 100644 --- a/packages/kit/src/runtime/client/ambient.d.ts +++ b/packages/kit/src/runtime/client/ambient.d.ts @@ -8,9 +8,9 @@ declare module '__GENERATED__/client-manifest.js' { /** * A map of `[routeId: string]: [leaf, layouts, errors]` tuples, which - * is parsed into an array of routes on startup. The numbers refer to the - * indices in `nodes`. The route layout and error nodes are not referenced, - * they are always number 0 and 1 and always apply. + * is parsed into an array of routes on startup. The numbers refer to the indices in `nodes`. + * If the number is negative, it means it does use a server load function and the complement is the node index. + * The route layout and error nodes are not referenced, they are always number 0 and 1 and always apply. */ export const dictionary: Record; diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index a3a06e72ffa0..6849a433f03a 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -424,21 +424,37 @@ export function create_client({ target, base, trailing_slash }) { }; let data = {}; - let data_changed = false; + let data_changed = !page; for (let i = 0; i < filtered.length; i += 1) { - data = { ...data, ...filtered[i].data }; + const node = filtered[i]; + data = { ...data, ...node.data }; + // Only set props if the node actually updated. This prevents needless rerenders. - if (data_changed || !current.branch.some((node) => node === filtered[i])) { + if (data_changed || !current.branch.some((previous) => previous === node)) { result.props[`data_${i}`] = data; - data_changed = true; + data_changed = data_changed || Object.keys(node.data ?? {}).length > 0; } } + if (!data_changed) { + // If nothing was added, and the object entries are the same length, this means + // that nothing was removed either and therefore the data is the same as the previous one. + // This would be more readable with a separate boolean but that would cost us some bytes. + data_changed = Object.keys(page.data).length !== Object.keys(data).length; + } const page_changed = !current.url || url.href !== current.url.href || current.error !== error || data_changed; if (page_changed) { - result.props.page = { error, params, routeId, status, url, data }; + result.props.page = { + error, + params, + routeId, + status, + url, + // The whole page store is updated, but this way the object reference stays the same + data: data_changed ? data : page.data + }; // TODO remove this for 1.0 /** @@ -675,14 +691,13 @@ export function create_client({ target, base, trailing_slash }) { params: Object.keys(params).filter((key) => current.params[key] !== params[key]) }; + const loaders = [...layouts, leaf]; + // preload modules to avoid waterfall, but handle rejections // so they don't get reported to Sentry et al (we don't need // to act on the failures at this point) - [...errors, ...layouts, leaf].forEach((loader) => loader?.().catch(() => {})); - - const loaders = [...layouts, leaf]; - - // To avoid waterfalls when someone awaits a parent, compute as much as possible here already + errors.forEach((loader) => loader?.().catch(() => {})); + loaders.forEach((loader) => loader?.[1]().catch(() => {})); /** @type {import('types').ServerData | null} */ let server_data = null; @@ -690,15 +705,15 @@ export function create_client({ target, base, trailing_slash }) { const invalid_server_nodes = loaders.reduce((acc, loader, i) => { const previous = current.branch[i]; const invalid = - loader && - (previous?.loader !== loader || + !!loader?.[0] && + (previous?.loader !== loader[1] || has_changed(changed, acc.some(Boolean), previous.server?.uses)); acc.push(invalid); return acc; }, /** @type {boolean[]} */ ([])); - if (route.uses_server_data && invalid_server_nodes.some(Boolean)) { + if (invalid_server_nodes.some(Boolean)) { try { const res = await native_fetch( `${url.pathname}${url.pathname.endsWith('/') ? '' : '/'}__data.json${url.search}`, @@ -741,7 +756,7 @@ export function create_client({ target, base, trailing_slash }) { // re-use data from previous load if it's still valid const valid = can_reuse_server_data && - loader === previous?.loader && + loader[1] === previous?.loader && !has_changed(changed, parent_changed, previous.shared?.uses); if (valid) return previous; @@ -757,7 +772,7 @@ export function create_client({ target, base, trailing_slash }) { } return load_node({ - loader, + loader: loader[1], url, params, routeId: route.id, @@ -801,11 +816,10 @@ export function create_client({ target, base, trailing_slash }) { let j = i; while (!branch[j]) j -= 1; - try { error_loaded = { - node: await errors[i](), - loader: errors[i], + node: await /** @type {import('types').CSRPageNodeLoader } */ (errors[i])(), + loader: /** @type {import('types').CSRPageNodeLoader } */ (errors[i]), data: {}, server: null, shared: null @@ -1087,7 +1101,7 @@ export function create_client({ target, base, trailing_slash }) { : routes; const promises = matching.map((r) => { - return Promise.all([...r.layouts, r.leaf].map((load) => load?.())); + return Promise.all([...r.layouts, r.leaf].map((load) => load?.[1]())); }); await Promise.all(promises); diff --git a/packages/kit/src/runtime/client/parse.js b/packages/kit/src/runtime/client/parse.js index ec22a49b852e..16e0c65ff84d 100644 --- a/packages/kit/src/runtime/client/parse.js +++ b/packages/kit/src/runtime/client/parse.js @@ -10,11 +10,6 @@ export function parse(nodes, dictionary, matchers) { return Object.entries(dictionary).map(([id, [leaf, layouts, errors]]) => { const { pattern, names, types } = parse_route_id(id); - // whether or not the route uses the server data is - // encoded using the ones' complement, to save space - const uses_server_data = leaf < 0; - if (uses_server_data) leaf = ~leaf; - const route = { id, /** @param {string} path */ @@ -23,9 +18,8 @@ export function parse(nodes, dictionary, matchers) { if (match) return exec(match, names, types, matchers); }, errors: [1, ...(errors || [])].map((n) => nodes[n]), - layouts: [0, ...(layouts || [])].map((n) => nodes[n]), - leaf: nodes[leaf], - uses_server_data + layouts: [0, ...(layouts || [])].map(create_loader), + leaf: create_loader(leaf) }; // bit of a hack, but ensures that layout/error node lists are the same @@ -38,4 +32,16 @@ export function parse(nodes, dictionary, matchers) { return route; }); + + /** + * @param {number} id + * @returns {[boolean, import('types').CSRPageNodeLoader]} + */ + function create_loader(id) { + // whether or not the route uses the server data is + // encoded using the ones' complement, to save space + const uses_server_data = id < 0; + if (uses_server_data) id = ~id; + return [uses_server_data, nodes[id]]; + } } diff --git a/packages/kit/test/apps/basics/src/routes/+layout.js b/packages/kit/test/apps/basics/src/routes/+layout.js index 0716988f42f4..b0527af4f7b9 100644 --- a/packages/kit/test/apps/basics/src/routes/+layout.js +++ b/packages/kit/test/apps/basics/src/routes/+layout.js @@ -1,12 +1,6 @@ -import { error } from '@sveltejs/kit'; - /** @type {import('@sveltejs/kit').Load} */ -export async function load({ fetch, url }) { - if (url.pathname.startsWith('/errors/error-in-layout')) { - const res = await fetch('/errors/error-in-layout/non-existent'); - throw error(res.status); - } - +export async function load() { + // Do NOT make this load function depend on something which would cause it to rerun return { foo: { bar: 'Custom layout' diff --git a/packages/kit/test/apps/basics/src/routes/+layout.server.js b/packages/kit/test/apps/basics/src/routes/+layout.server.js index a57ef430c4ff..bacb24019ecc 100644 --- a/packages/kit/test/apps/basics/src/routes/+layout.server.js +++ b/packages/kit/test/apps/basics/src/routes/+layout.server.js @@ -1,4 +1,5 @@ export async function load() { + // Do NOT make this load function depend on something which would cause it to rerun return { rootlayout: 'rootlayout' }; diff --git a/packages/kit/test/apps/basics/src/routes/errors/+layout.js b/packages/kit/test/apps/basics/src/routes/errors/+layout.js new file mode 100644 index 000000000000..db7cc564dfbc --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/+layout.js @@ -0,0 +1,9 @@ +import { error } from '@sveltejs/kit'; + +/** @type {import('@sveltejs/kit').Load} */ +export async function load({ fetch, url }) { + if (url.pathname.startsWith('/errors/error-in-layout')) { + const res = await fetch('/errors/error-in-layout/non-existent'); + throw error(res.status); + } +} diff --git a/packages/kit/test/apps/basics/src/routes/errors/+layout.svelte b/packages/kit/test/apps/basics/src/routes/errors/+layout.svelte new file mode 100644 index 000000000000..0edcae4bf71e --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/errors/+layout.svelte @@ -0,0 +1 @@ +; diff --git a/packages/kit/test/apps/basics/src/routes/store/data/unchanged/+layout.svelte b/packages/kit/test/apps/basics/src/routes/store/data/unchanged/+layout.svelte new file mode 100644 index 000000000000..effe3150ad94 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/store/data/unchanged/+layout.svelte @@ -0,0 +1,14 @@ + + +

$page.data was updated {count} time(s)

+a +b diff --git a/packages/kit/test/apps/basics/src/routes/store/data/unchanged/a/+page.svelte b/packages/kit/test/apps/basics/src/routes/store/data/unchanged/a/+page.svelte new file mode 100644 index 000000000000..98aa6d082810 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/store/data/unchanged/a/+page.svelte @@ -0,0 +1 @@ +Page A \ No newline at end of file diff --git a/packages/kit/test/apps/basics/src/routes/store/data/unchanged/b/+page.svelte b/packages/kit/test/apps/basics/src/routes/store/data/unchanged/b/+page.svelte new file mode 100644 index 000000000000..331b9e8386f5 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/store/data/unchanged/b/+page.svelte @@ -0,0 +1 @@ +Page B \ No newline at end of file diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index a49bdca0af90..03da4032df27 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -598,11 +598,19 @@ test('Can use browser-only global on client-only page', async ({ page, read_erro expect(read_errors('/no-ssr/browser-only-global')).toBe(undefined); }); -test('can use $app/stores from anywhere on client', async ({ page }) => { - await page.goto('/store/client-access'); - await expect(page.locator('h1')).toHaveText('undefined'); - await page.click('button'); - await expect(page.locator('h1')).toHaveText('/store/client-access'); +test.describe('$app/stores', () => { + test('can use $app/stores from anywhere on client', async ({ page }) => { + await page.goto('/store/client-access'); + await expect(page.locator('h1')).toHaveText('undefined'); + await page.click('button'); + await expect(page.locator('h1')).toHaveText('/store/client-access'); + }); + + test('$page.data does not update if data is unchanged', async ({ page, app }) => { + await page.goto('/store/data/unchanged/a'); + await app.goto('/store/data/unchanged/b'); + await expect(page.locator('p')).toHaveText('$page.data was updated 0 time(s)'); + }); }); test.describe.serial('Invalidation', () => { diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts index 3fc11b75283b..2543bc14c35f 100644 --- a/packages/kit/types/internal.d.ts +++ b/packages/kit/types/internal.d.ts @@ -71,13 +71,16 @@ export interface CSRPageNode { export type CSRPageNodeLoader = () => Promise; +/** + * Definition of a client side route. + * The boolean in the tuples indicates whether the route has a server load. + */ export type CSRRoute = { id: string; exec: (path: string) => undefined | Record; - errors: CSRPageNodeLoader[]; - layouts: CSRPageNodeLoader[]; - leaf: CSRPageNodeLoader; - uses_server_data: boolean; + errors: Array; + layouts: Array<[boolean, CSRPageNodeLoader] | undefined>; + leaf: [boolean, CSRPageNodeLoader]; }; export type GetParams = (match: RegExpExecArray) => Record;