Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested error components #901

Merged
merged 24 commits into from
Apr 12, 2021
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/kit/src/core/build/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,8 @@ async function build_server(
type: 'page',
pattern: ${route.pattern},
params: ${params},
parts: [${route.parts.map(file => s(file)).join(', ')}]
a: [${route.a.map(file => file && s(file)).join(', ')}],
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved
b: [${route.b.map(file => file && s(file)).join(', ')}]
}`;
} else {
const params = get_params(route.params);
Expand Down
29 changes: 11 additions & 18 deletions packages/kit/src/core/create_app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ function generate_client_manifest(manifest_data, base) {
.join(',\n\t\t\t\t')}
]`.replace(/^\t/gm, '');

/** @param {string[]} parts */
const get_indices = (parts) =>
`[${parts.map((part) => (part ? `c[${component_indexes[part]}]` : '')).join(', ')}]`;

const routes = `[
${manifest_data.routes
.map((route) => {
Expand All @@ -81,15 +85,10 @@ function generate_client_manifest(manifest_data, base) {
.join(', ') +
'})';

const tuple = [
route.pattern,
`[${route.parts.map((part) => `components[${component_indexes[part]}]`).join(', ')}]`,
params
]
.filter(Boolean)
.join(', ');
const tuple = [route.pattern, get_indices(route.a), get_indices(route.b)];
if (params) tuple.push(params);

return `// ${route.parts[route.parts.length - 1]}\n\t\t[${tuple}]`;
return `// ${route.a[route.a.length - 1]}\n\t\t[${tuple.join(', ')}]`;
} else {
return `// ${route.file}\n\t\t[${route.pattern}]`;
}
Expand All @@ -98,13 +97,13 @@ function generate_client_manifest(manifest_data, base) {
]`.replace(/^\t/gm, '');

return trim(`
const components = ${components};
const c = ${components};

const d = decodeURIComponent;

export const routes = ${routes};

export const fallback = [components[0](), components[1]()];
export const fallback = [c[0](), c[1]()];
`);
}

Expand All @@ -117,7 +116,7 @@ function generate_app(manifest_data, base) {

const max_depth = Math.max(
...manifest_data.routes.map((route) =>
route.type === 'page' ? route.parts.filter(Boolean).length : 0
route.type === 'page' ? route.a.filter(Boolean).length : 0
)
);

Expand All @@ -132,9 +131,7 @@ function generate_app(manifest_data, base) {

while (l--) {
pyramid = `
<svelte:component this={components[${l}]} {...(props_${l} || {})}${
l === 1 ? ' {status} {error}' : '' // TODO this is awkward
}>
<svelte:component this={components[${l}]} {...(props_${l} || {})}>
{#if components[${l + 1}]}
${pyramid.replace(/\n/g, '\n\t\t\t\t\t')}
{/if}
Expand All @@ -149,10 +146,6 @@ function generate_app(manifest_data, base) {
<script>
import { setContext, afterUpdate, onMount } from 'svelte';

// error handling
export let status = undefined;
export let error = undefined;

// stores
export let stores;
export let page;
Expand Down
40 changes: 29 additions & 11 deletions packages/kit/src/core/create_manifest_data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@ export default function create_manifest_data({ config, output, cwd = process.cwd
* @param {string} dir
* @param {Part[][]} parent_segments
* @param {string[]} parent_params
* @param {string[]} stack
* @param {string[]} layout_stack
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved
* @param {string[]} error_stack
*/
function walk(dir, parent_segments, parent_params, stack) {
function walk(dir, parent_segments, parent_params, layout_stack, error_stack) {
/** @type {Item[]} */
const items = fs
.readdirSync(dir)
Expand Down Expand Up @@ -138,31 +139,48 @@ export default function create_manifest_data({ config, output, cwd = process.cwd
params.push(...item.parts.filter((p) => p.dynamic).map((p) => p.content));

if (item.is_dir) {
const component = find_layout('$layout', item.file);
const layout = find_layout('$layout', item.file);
const error = find_layout('$error', item.file);

if (component) components.push(component);
if (layout) components.push(layout);
if (error) components.push(error);

walk(
path.join(dir, item.basename),
segments,
params,
component ? stack.concat(component) : stack
layout_stack.concat(layout),
error_stack.concat(error)
);
} else if (item.is_page) {
components.push(item.file);

const parts =
item.is_index && stack[stack.length - 1] === null
? stack.slice(0, -1).concat(item.file)
: stack.concat(item.file);
const a = layout_stack.concat(item.file);
const b = error_stack;

const pattern = get_pattern(segments, true);

let i = a.length;
while (i--) {
if (!b[i] && !a[i]) {
b.splice(i, 1);
a.splice(i, 1);
}
}

i = b.length;
while (i--) {
if (b[i]) break;
}

b.splice(i + 1);

routes.push({
type: 'page',
pattern,
params,
parts
a,
b
});
} else {
const pattern = get_pattern(segments, !item.route_suffix);
Expand All @@ -184,7 +202,7 @@ export default function create_manifest_data({ config, output, cwd = process.cwd

components.push(layout, error);

walk(config.kit.files.routes, [], [], [layout]);
walk(config.kit.files.routes, [], [], [layout], [error]);

const assets_dir = config.kit.files.assets;

Expand Down
57 changes: 46 additions & 11 deletions packages/kit/src/core/create_manifest_data/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,16 @@ test('creates routes', () => {
type: 'page',
pattern: /^\/$/,
params: [],
parts: [layout, index]
a: [layout, index],
b: [error]
},

{
type: 'page',
pattern: /^\/about\/?$/,
params: [],
parts: [layout, about]
a: [layout, about],
b: [error]
},

{
Expand All @@ -68,7 +70,8 @@ test('creates routes', () => {
type: 'page',
pattern: /^\/blog\/?$/,
params: [],
parts: [layout, blog]
a: [layout, blog],
b: [error]
},

{
Expand All @@ -82,7 +85,8 @@ test('creates routes', () => {
type: 'page',
pattern: /^\/blog\/([^/]+?)\/?$/,
params: ['slug'],
parts: [layout, blog_$slug]
a: [layout, blog_$slug],
b: [error]
}
]);
});
Expand All @@ -103,14 +107,16 @@ test('creates routes with layout', () => {
type: 'page',
pattern: /^\/$/,
params: [],
parts: [layout, index]
a: [layout, index],
b: [error]
},

{
type: 'page',
pattern: /^\/foo\/?$/,
params: [],
parts: [layout, foo_$layout, foo]
a: [layout, foo_$layout, foo],
b: [error]
}
]);
});
Expand Down Expand Up @@ -146,7 +152,7 @@ test('sorts routes correctly', () => {
const { routes } = create('samples/sorting');

assert.equal(
routes.map((p) => (p.type === 'page' ? p.parts : p.file)),
routes.map((p) => (p.type === 'page' ? p.a : p.file)),
[
[layout, 'samples/sorting/index.svelte'],
[layout, 'samples/sorting/about.svelte'],
Expand Down Expand Up @@ -254,14 +260,16 @@ test('works with custom extensions', () => {
type: 'page',
pattern: /^\/$/,
params: [],
parts: [layout, index]
a: [layout, index],
b: [error]
},

{
type: 'page',
pattern: /^\/about\/?$/,
params: [],
parts: [layout, about]
a: [layout, about],
b: [error]
},

{
Expand All @@ -275,7 +283,8 @@ test('works with custom extensions', () => {
type: 'page',
pattern: /^\/blog\/?$/,
params: [],
parts: [layout, blog]
a: [layout, blog],
b: [error]
},

{
Expand All @@ -289,7 +298,8 @@ test('works with custom extensions', () => {
type: 'page',
pattern: /^\/blog\/([^/]+?)\/?$/,
params: ['slug'],
parts: [layout, blog_$slug]
a: [layout, blog_$slug],
b: [error]
}
]);
});
Expand All @@ -311,4 +321,29 @@ test('lists static assets', () => {
]);
});

test('includes nested error components', () => {
const { routes } = create('samples/nested-errors');

assert.equal(routes, [
{
type: 'page',
pattern: /^\/foo\/bar\/baz\/?$/,
params: [],
a: [
layout,
'samples/nested-errors/foo/$layout.svelte',
undefined,
'samples/nested-errors/foo/bar/baz/$layout.svelte',
'samples/nested-errors/foo/bar/baz/index.svelte'
],
b: [
error,
undefined,
'samples/nested-errors/foo/bar/$error.svelte',
'samples/nested-errors/foo/bar/baz/$error.svelte'
]
}
]);
});

test.run();
5 changes: 2 additions & 3 deletions packages/kit/src/core/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,14 +263,12 @@ class Watcher extends EventEmitter {
handle: hooks.handle || ((request, render) => render(request))
},
only_render_prerenderable_pages: false,
// get_component_path: (id) => `/${id}?import`,
get_stack: (error) => {
this.vite.ssrFixStacktrace(error);
return error.stack;
},
get_static_file: (file) =>
fs.readFileSync(path.join(this.config.kit.files.assets, file)),
// get_amp_css: (url) => '', // TODO: implement this
ssr: this.config.kit.ssr,
router: this.config.kit.router,
hydrate: this.config.kit.hydrate
Expand Down Expand Up @@ -316,7 +314,8 @@ class Watcher extends EventEmitter {
type: 'page',
pattern: route.pattern,
params: get_params(route.params),
parts: route.parts
a: route.a,
b: route.b
};
}

Expand Down
Loading