Skip to content

Commit

Permalink
Nested error components (#901)
Browse files Browse the repository at this point in the history
* failing test for #626

* update create_manifest_data and unit tests

* update types

* get existing tests working

* WIP

* nested error pages working in SSR, pass error/status via load

* most tests passing

* make it possible to filter tests

* all tests passing except the new one

* refactor some stuff, fix types

* fix/simplify caching

* refactoring and stuff

* remove unused property

* bypass _load on initial hydration

* pass entire route to this._load

* fix caching

* it works

* lint/check

* document a and b

* explain what layout_stack and error_stack are

* slight touch-up

* document nested error pages

* changeset
  • Loading branch information
Rich Harris committed Apr 12, 2021
1 parent ca108a6 commit 1007f67
Show file tree
Hide file tree
Showing 29 changed files with 897 additions and 562 deletions.
5 changes: 5 additions & 0 deletions .changeset/rude-tomatoes-thank.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Allow non-root \$error.svelte components
29 changes: 22 additions & 7 deletions documentation/docs/02-layouts.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,32 @@ We can create a layout that only applies to pages below `/settings` (while inher
<slot></slot>
```


### Error pages

If your page fails to load (see [Loading](#loading)), SvelteKit will render an error page. You can customise this page by creating a file called `src/routes/$error.svelte`, which is a component that receives an `error` prop alongside a `status` code:
If a page fails to load (see [Loading](#loading)), SvelteKit will render an error page. You can customise this page by creating `$error.svelte` components alongside your layout and page components.

For example, if `src/routes/settings/notifications/index.svelte` failed to load, SvelteKit would render `src/routes/settings/notifications/$error.svelte` in the same layout, if it existed. If not, it would render `src/routes/settings/$error.svelte` in the parent layout, or `src/routes/$error.svelte` in the root layout.

> SvelteKit provides a default error page in case you don't supply `src/routes/$error.svelte`, but it's recommend that you bring your own.
If an error component has a [`load`](#loading) function, it will be called with `error` and `status` properties:

```html
<script context="module">
export function load({ error, status }) {
return {
props: {
title: `${status}: ${error.message}`
}
};
}
</script>

<script>
export let status;
export let error;
export let title;
</script>

<h1>{status}</h1>
<p>{error.message}</p>
```
<h1>{title}</h1>
```

> Server-side stack traces will be removed from `error` in production, to avoid exposing privileged information to users.
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(', ')}],
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 // accumulated $layout.svelte components
* @param {string[]} error_stack // accumulated $error.svelte components
*/
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();
Empty file.
Empty file.
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

0 comments on commit 1007f67

Please sign in to comment.