From 969d62bb62854f0effaf410cbdaac240a7eb5738 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Sun, 28 Aug 2022 14:31:23 +0200 Subject: [PATCH 1/4] [fix] encode if root layout has server load Fixes #6349 --- .changeset/short-comics-unite.md | 5 +++++ packages/kit/src/core/sync/write_client_manifest.js | 10 +++++++--- packages/kit/src/runtime/client/ambient.d.ts | 2 +- packages/kit/src/runtime/client/parse.js | 2 +- 4 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 .changeset/short-comics-unite.md diff --git a/.changeset/short-comics-unite.md b/.changeset/short-comics-unite.md new file mode 100644 index 000000000000..4459af1d8031 --- /dev/null +++ b/.changeset/short-comics-unite.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +[fix] encode if root layout has server load diff --git a/packages/kit/src/core/sync/write_client_manifest.js b/packages/kit/src/core/sync/write_client_manifest.js index ff200ae13fab..a1743fc2e5ad 100644 --- a/packages/kit/src/core/sync/write_client_manifest.js +++ b/packages/kit/src/core/sync/write_client_manifest.js @@ -47,8 +47,11 @@ export function write_client_manifest(manifest_data, output) { ${manifest_data.routes .map((route) => { if (route.page) { + // Skip the first error, it's always the root error page with number 1 const errors = route.page.errors.slice(1).map((n) => n ?? ''); - const layouts = route.page.layouts.slice(1).map((n) => { + // Do _not_ skip the first layout, it's always the root layout with number 0, + // but we don't know whether it has a server load function or not, which we need to encode + const layouts = route.page.layouts.map((n) => { if (n == undefined) { return ''; } @@ -60,8 +63,8 @@ export function write_client_manifest(manifest_data, output) { 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(',')}]`); + array.push(`[${layouts.join(',')}]`); + // only include non-root error nodes if they exist if (errors.length > 0) array.push(`[${errors.join(',')}]`); return `${s(route.id)}: [${array.join(',')}]`; @@ -93,5 +96,6 @@ export function write_client_manifest(manifest_data, output) { * @param {number} id */ function get_node_id(nodes, id) { + console.log(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 ac822de8ac03..932a9d12f535 100644 --- a/packages/kit/src/runtime/client/ambient.d.ts +++ b/packages/kit/src/runtime/client/ambient.d.ts @@ -12,7 +12,7 @@ declare module '__GENERATED__/client-manifest.js' { * 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; + export const dictionary: Record; export const matchers: Record; } diff --git a/packages/kit/src/runtime/client/parse.js b/packages/kit/src/runtime/client/parse.js index 16e0c65ff84d..b52a44fe822c 100644 --- a/packages/kit/src/runtime/client/parse.js +++ b/packages/kit/src/runtime/client/parse.js @@ -18,7 +18,7 @@ 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(create_loader), + layouts: layouts.map(create_loader), leaf: create_loader(leaf) }; From 4979aaf46d3c4e79f5ff69f6e9bf56affa9a88c8 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Sun, 28 Aug 2022 15:16:42 +0200 Subject: [PATCH 2/4] optimize client manifest size --- .../src/core/sync/write_client_manifest.js | 26 ++++++++++++------- packages/kit/src/runtime/client/ambient.d.ts | 8 +++++- packages/kit/src/runtime/client/client.js | 4 +-- packages/kit/src/runtime/client/parse.js | 21 ++++++++++++--- 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/packages/kit/src/core/sync/write_client_manifest.js b/packages/kit/src/core/sync/write_client_manifest.js index a1743fc2e5ad..b36430a7101a 100644 --- a/packages/kit/src/core/sync/write_client_manifest.js +++ b/packages/kit/src/core/sync/write_client_manifest.js @@ -43,15 +43,14 @@ export function write_client_manifest(manifest_data, output) { }) .join(',\n\t'); + const layouts_with_server_load = new Set(); + const dictionary = `{ ${manifest_data.routes .map((route) => { if (route.page) { - // Skip the first error, it's always the root error page with number 1 const errors = route.page.errors.slice(1).map((n) => n ?? ''); - // Do _not_ skip the first layout, it's always the root layout with number 0, - // but we don't know whether it has a server load function or not, which we need to encode - const layouts = route.page.layouts.map((n) => { + const layouts = route.page.layouts.slice(1).map((n) => { if (n == undefined) { return ''; } @@ -61,7 +60,17 @@ export function write_client_manifest(manifest_data, output) { while (layouts.at(-1) === '') layouts.pop(); while (errors.at(-1) === '') errors.pop(); - const array = [get_node_id(manifest_data.nodes, route.page.leaf)]; + // Encode whether or not the route uses server data + // using the ones' complement, to save space + const array = [`${route.leaf?.server ? '~' : ''}${route.page.leaf}`]; + // Encode whether or not the layout uses server data. + // It's a different method compared to pages because layouts + // are reused across pages, so we safe space by doing it this way. + route.page.layouts.forEach((layout) => { + if (layout != undefined && manifest_data.nodes[layout].server) { + layouts_with_server_load.add(layout); + } + }); array.push(`[${layouts.join(',')}]`); // only include non-root error nodes if they exist @@ -80,9 +89,9 @@ export function write_client_manifest(manifest_data, output) { trim(` export { matchers } from './client-matchers.js'; - export const nodes = [ - ${nodes} - ]; + export const nodes = [${nodes}]; + + export const server_loads = [${[...layouts_with_server_load].join(',')}]; export const dictionary = ${dictionary}; `) @@ -96,6 +105,5 @@ export function write_client_manifest(manifest_data, output) { * @param {number} id */ function get_node_id(nodes, id) { - console.log(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 932a9d12f535..4cf3c8f9aa40 100644 --- a/packages/kit/src/runtime/client/ambient.d.ts +++ b/packages/kit/src/runtime/client/ambient.d.ts @@ -6,10 +6,16 @@ declare module '__GENERATED__/client-manifest.js' { */ export const nodes: CSRPageNodeLoader[]; + /** + * A list of all layout node ids that have a server load function. + * Pages are not present because it's shorter to encode it on the leaf itself. + */ + export const server_loads: number[]; + /** * 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`. - * If the number is negative, it means it does use a server load function and the complement is the node index. + * If the leaf 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 6849a433f03a..4bf1994d710a 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -7,14 +7,14 @@ import { parse } from './parse.js'; import { error } from '../../exports/index.js'; import Root from '__GENERATED__/root.svelte'; -import { nodes, dictionary, matchers } from '__GENERATED__/client-manifest.js'; +import { nodes, server_loads, dictionary, matchers } from '__GENERATED__/client-manifest.js'; import { HttpError, Redirect } from '../control.js'; import { stores } from './singletons.js'; const SCROLL_KEY = 'sveltekit:scroll'; const INDEX_KEY = 'sveltekit:index'; -const routes = parse(nodes, dictionary, matchers); +const routes = parse(nodes, server_loads, dictionary, matchers); const default_layout_loader = nodes[0]; const default_error_loader = nodes[1]; diff --git a/packages/kit/src/runtime/client/parse.js b/packages/kit/src/runtime/client/parse.js index b52a44fe822c..6955e828b9f1 100644 --- a/packages/kit/src/runtime/client/parse.js +++ b/packages/kit/src/runtime/client/parse.js @@ -2,11 +2,14 @@ import { exec, parse_route_id } from '../../utils/routing.js'; /** * @param {import('types').CSRPageNodeLoader[]} nodes + * @param {number[]} server_loads * @param {typeof import('__GENERATED__/client-manifest.js').dictionary} dictionary * @param {Record boolean>} matchers * @returns {import('types').CSRRoute[]} */ -export function parse(nodes, dictionary, matchers) { +export function parse(nodes, server_loads, dictionary, matchers) { + const layouts_with_server_load = new Set(server_loads); + return Object.entries(dictionary).map(([id, [leaf, layouts, errors]]) => { const { pattern, names, types } = parse_route_id(id); @@ -18,8 +21,8 @@ export function parse(nodes, dictionary, matchers) { if (match) return exec(match, names, types, matchers); }, errors: [1, ...(errors || [])].map((n) => nodes[n]), - layouts: layouts.map(create_loader), - leaf: create_loader(leaf) + layouts: [0, ...(layouts || [])].map(create_layout_loader), + leaf: create_leaf_loader(leaf) }; // bit of a hack, but ensures that layout/error node lists are the same @@ -37,11 +40,21 @@ export function parse(nodes, dictionary, matchers) { * @param {number} id * @returns {[boolean, import('types').CSRPageNodeLoader]} */ - function create_loader(id) { + function create_leaf_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]]; } + + /** + * @param {number} id + * @returns {[boolean, import('types').CSRPageNodeLoader]} + */ + function create_layout_loader(id) { + // whether or not the layout uses the server data is + // encoded in the layouts array, to save space + return [layouts_with_server_load.has(id), nodes[id]]; + } } From 55d93c7266451b20213dd5fad6cd3fb25247df8f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 28 Aug 2022 14:57:38 -0400 Subject: [PATCH 3/4] fix --- .../kit/src/core/sync/write_client_manifest.js | 17 +---------------- packages/kit/src/runtime/client/parse.js | 2 +- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/packages/kit/src/core/sync/write_client_manifest.js b/packages/kit/src/core/sync/write_client_manifest.js index b36430a7101a..e7470c794121 100644 --- a/packages/kit/src/core/sync/write_client_manifest.js +++ b/packages/kit/src/core/sync/write_client_manifest.js @@ -50,12 +50,7 @@ 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) => { - if (n == undefined) { - return ''; - } - return get_node_id(manifest_data.nodes, n); - }); + const layouts = route.page.layouts.slice(1).map((n) => n ?? ''); while (layouts.at(-1) === '') layouts.pop(); while (errors.at(-1) === '') errors.pop(); @@ -97,13 +92,3 @@ 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/parse.js b/packages/kit/src/runtime/client/parse.js index 6955e828b9f1..4d1b9f05947e 100644 --- a/packages/kit/src/runtime/client/parse.js +++ b/packages/kit/src/runtime/client/parse.js @@ -55,6 +55,6 @@ export function parse(nodes, server_loads, dictionary, matchers) { function create_layout_loader(id) { // whether or not the layout uses the server data is // encoded in the layouts array, to save space - return [layouts_with_server_load.has(id), nodes[id]]; + return id === undefined ? id : [layouts_with_server_load.has(id), nodes[id]]; } } From fecdac71f6392c1428f458a7393f3495cc6414c9 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 29 Aug 2022 07:57:54 +0200 Subject: [PATCH 4/4] optimize manifest, types --- packages/kit/src/core/sync/write_client_manifest.js | 4 ++-- packages/kit/src/runtime/client/parse.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/kit/src/core/sync/write_client_manifest.js b/packages/kit/src/core/sync/write_client_manifest.js index e7470c794121..2fc49b937ce0 100644 --- a/packages/kit/src/core/sync/write_client_manifest.js +++ b/packages/kit/src/core/sync/write_client_manifest.js @@ -67,8 +67,8 @@ export function write_client_manifest(manifest_data, output) { } }); - array.push(`[${layouts.join(',')}]`); - // only include non-root error nodes if they exist + // only include non-root layout/error nodes if they exist + if (layouts.length > 0 || errors.length > 0) array.push(`[${layouts.join(',')}]`); if (errors.length > 0) array.push(`[${errors.join(',')}]`); return `${s(route.id)}: [${array.join(',')}]`; diff --git a/packages/kit/src/runtime/client/parse.js b/packages/kit/src/runtime/client/parse.js index 4d1b9f05947e..167cc36e06f7 100644 --- a/packages/kit/src/runtime/client/parse.js +++ b/packages/kit/src/runtime/client/parse.js @@ -49,8 +49,8 @@ export function parse(nodes, server_loads, dictionary, matchers) { } /** - * @param {number} id - * @returns {[boolean, import('types').CSRPageNodeLoader]} + * @param {number | undefined} id + * @returns {[boolean, import('types').CSRPageNodeLoader] | undefined} */ function create_layout_loader(id) { // whether or not the layout uses the server data is