From e05328e1cc52d5b09b4021ab0bf03e297b81a18a Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 16 Aug 2022 10:18:02 +0200 Subject: [PATCH 1/4] [feat] better clarify invalid named layout references Closes #5903 --- .changeset/slow-spiders-agree.md | 5 +++ documentation/docs/04-advanced-routing.md | 2 + .../core/sync/create_manifest_data/index.js | 38 ++++++++++--------- 3 files changed, 28 insertions(+), 17 deletions(-) create mode 100644 .changeset/slow-spiders-agree.md diff --git a/.changeset/slow-spiders-agree.md b/.changeset/slow-spiders-agree.md new file mode 100644 index 000000000000..48d86996609f --- /dev/null +++ b/.changeset/slow-spiders-agree.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +Provide helpful error message on invalid named layout reference diff --git a/documentation/docs/04-advanced-routing.md b/documentation/docs/04-advanced-routing.md index 4541b15f834f..995f1fd18422 100644 --- a/documentation/docs/04-advanced-routing.md +++ b/documentation/docs/04-advanced-routing.md @@ -144,6 +144,8 @@ Some parts of your app might need something other than the default layout. For t

I am inside +layout-foo

``` +> Only the Svelte file should reference the named layout, not its accompanying `+page.js` for example + Named layouts are very powerful, but it can take a minute to get your head round them. Don't worry if this doesn't make sense all at once. #### Scoping diff --git a/packages/kit/src/core/sync/create_manifest_data/index.js b/packages/kit/src/core/sync/create_manifest_data/index.js index 9234a5e3f439..3cba3936d5b4 100644 --- a/packages/kit/src/core/sync/create_manifest_data/index.js +++ b/packages/kit/src/core/sync/create_manifest_data/index.js @@ -67,14 +67,7 @@ export default function create_manifest_data({ if (file[0] !== '+') return; // not a route file - const item = analyze(file, config.extensions, config.kit.moduleExtensions); - - if (!item) { - throw new Error( - `Files and directories prefixed with + are reserved (saw ${project_relative})` - ); - } - + const item = analyze(project_relative, file, config.extensions, config.kit.moduleExtensions); const id = segments.join('/'); if (/\]\[/.test(id)) { @@ -275,19 +268,22 @@ export default function create_manifest_data({ } /** + * @param {string} project_relative * @param {string} file * @param {string[]} component_extensions * @param {string[]} module_extensions - * @returns {import('./types').RouteFile | null} + * @returns {import('./types').RouteFile} */ -function analyze(file, component_extensions, module_extensions) { +function analyze(project_relative, file, component_extensions, module_extensions) { const component_extension = component_extensions.find((ext) => file.endsWith(ext)); if (component_extension) { const name = file.slice(0, -component_extension.length); const pattern = /^\+(?:(page(?:@([a-zA-Z0-9_-]+))?)|(layout(?:-([a-zA-Z0-9_-]+))?(?:@([a-zA-Z0-9_-]+))?)|(error))$/; const match = pattern.exec(name); - if (!match) return null; + if (!match) { + throw new Error(`Files prefixed with + are reserved (saw ${project_relative})`); + } return { kind: 'component', @@ -302,21 +298,29 @@ function analyze(file, component_extensions, module_extensions) { const module_extension = module_extensions.find((ext) => file.endsWith(ext)); if (module_extension) { const name = file.slice(0, -module_extension.length); - const pattern = /^\+(?:(server)|(page(\.server)?)|(layout(?:-([a-zA-Z0-9_-]+))?(\.server)?))$/; + const pattern = + /^\+(?:(server)|(page(?:@([a-zA-Z0-9_-]+))?(\.server)?)|(layout(?:-([a-zA-Z0-9_-]+))?(?:@([a-zA-Z0-9_-]+))?(\.server)?))$/; const match = pattern.exec(name); - if (!match) return null; + if (!match) { + throw new Error(`Files prefixed with + are reserved (saw ${project_relative})`); + } else if (match[3] || match[7]) { + throw new Error( + // prettier-ignore + `Only Svelte files can reference named layouts. Remove '@${match[3] || match[7]}' from ${file} (at ${project_relative})` + ); + } - const kind = !!(match[1] || match[3] || match[6]) ? 'server' : 'shared'; + const kind = !!(match[1] || match[4] || match[8]) ? 'server' : 'shared'; return { kind, is_page: !!match[2], - is_layout: !!match[4], - declares_layout: match[5] + is_layout: !!match[5], + declares_layout: match[6] }; } - return null; + throw new Error(`Files and directories prefixed with + are reserved (saw ${project_relative})`); } /** From e958aeca4caae4d83b1e2b9cd4015e34d247c947 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 16 Aug 2022 10:24:49 +0200 Subject: [PATCH 2/4] test --- .../kit/src/core/sync/create_manifest_data/index.spec.js | 7 +++++++ .../invalid-named-layout-reference/+layout-named.svelte | 0 .../samples/invalid-named-layout-reference/+page@named.js | 0 .../invalid-named-layout-reference/+page@named.svelte | 0 4 files changed, 7 insertions(+) create mode 100644 packages/kit/src/core/sync/create_manifest_data/test/samples/invalid-named-layout-reference/+layout-named.svelte create mode 100644 packages/kit/src/core/sync/create_manifest_data/test/samples/invalid-named-layout-reference/+page@named.js create mode 100644 packages/kit/src/core/sync/create_manifest_data/test/samples/invalid-named-layout-reference/+page@named.svelte diff --git a/packages/kit/src/core/sync/create_manifest_data/index.spec.js b/packages/kit/src/core/sync/create_manifest_data/index.spec.js index a841e923d4a1..f60559ec4d15 100644 --- a/packages/kit/src/core/sync/create_manifest_data/index.spec.js +++ b/packages/kit/src/core/sync/create_manifest_data/index.spec.js @@ -566,6 +566,13 @@ test('errors on layout named default', () => { ); }); +test('errors on invalid named layout reference', () => { + assert.throws( + () => create('samples/invalid-named-layout-reference'), + /Only Svelte files can reference named layouts. Remove '@named' from \+page@named.js \(at samples\/invalid-named-layout-reference\/\+page@named.js\)/ + ); +}); + test('errors on duplicate layout definition', () => { assert.throws( () => create('samples/duplicate-layout'), diff --git a/packages/kit/src/core/sync/create_manifest_data/test/samples/invalid-named-layout-reference/+layout-named.svelte b/packages/kit/src/core/sync/create_manifest_data/test/samples/invalid-named-layout-reference/+layout-named.svelte new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/kit/src/core/sync/create_manifest_data/test/samples/invalid-named-layout-reference/+page@named.js b/packages/kit/src/core/sync/create_manifest_data/test/samples/invalid-named-layout-reference/+page@named.js new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/kit/src/core/sync/create_manifest_data/test/samples/invalid-named-layout-reference/+page@named.svelte b/packages/kit/src/core/sync/create_manifest_data/test/samples/invalid-named-layout-reference/+page@named.svelte new file mode 100644 index 000000000000..e69de29bb2d1 From 74f006e85abd7c954be50288fc11fde7bc2230ac Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 16 Aug 2022 13:03:31 +0200 Subject: [PATCH 3/4] Update documentation/docs/04-advanced-routing.md Co-authored-by: Ignatius Bagus --- documentation/docs/04-advanced-routing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/docs/04-advanced-routing.md b/documentation/docs/04-advanced-routing.md index 995f1fd18422..1e6681aced20 100644 --- a/documentation/docs/04-advanced-routing.md +++ b/documentation/docs/04-advanced-routing.md @@ -144,7 +144,7 @@ Some parts of your app might need something other than the default layout. For t

I am inside +layout-foo

``` -> Only the Svelte file should reference the named layout, not its accompanying `+page.js` for example +> Named layout should only be referenced from `+page.svelte` files Named layouts are very powerful, but it can take a minute to get your head round them. Don't worry if this doesn't make sense all at once. From 190d404b4b8e2ed5f9a48c7eac2df746a62d665d Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 16 Aug 2022 14:53:55 +0200 Subject: [PATCH 4/4] Update documentation/docs/04-advanced-routing.md --- documentation/docs/04-advanced-routing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/docs/04-advanced-routing.md b/documentation/docs/04-advanced-routing.md index 1e6681aced20..8c82a8be0ec2 100644 --- a/documentation/docs/04-advanced-routing.md +++ b/documentation/docs/04-advanced-routing.md @@ -144,7 +144,7 @@ Some parts of your app might need something other than the default layout. For t

I am inside +layout-foo

``` -> Named layout should only be referenced from `+page.svelte` files +> Named layout should only be referenced from Svelte files Named layouts are very powerful, but it can take a minute to get your head round them. Don't worry if this doesn't make sense all at once.