Skip to content

Commit

Permalink
[fix] correctly do fallthrough in simple case (#2072)
Browse files Browse the repository at this point in the history
  • Loading branch information
benmccann authored Aug 2, 2021
1 parent 4d2144e commit ee73a26
Show file tree
Hide file tree
Showing 15 changed files with 83 additions and 47 deletions.
5 changes: 5 additions & 0 deletions .changeset/wet-beds-thank.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[fix] correctly do fallthrough in simple case
6 changes: 3 additions & 3 deletions packages/kit/src/runtime/server/endpoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ function is_string(s) {
/**
* @param {import('types/hooks').ServerRequest} request
* @param {import('types/internal').SSREndpoint} route
* @returns {Promise<import('types/hooks').ServerResponse>}
* @returns {Promise<import('types/hooks').ServerResponse | undefined>}
*/
export default async function render_route(request, route) {
export async function render_endpoint(request, route) {
const mod = await route.load();

/** @type {import('types/endpoint').RequestHandler} */
const handler = mod[request.method.toLowerCase().replace('delete', 'del')]; // 'delete' is a reserved word

if (!handler) {
return error('no handler');
return;
}

const match = route.pattern.exec(request.path);
Expand Down
15 changes: 12 additions & 3 deletions packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import render_page from './page/index.js';
import { render_endpoint } from './endpoint.js';
import { render_page } from './page/index.js';
import { render_response } from './page/render.js';
import render_endpoint from './endpoint.js';
import { respond_with_error } from './page/respond_with_error.js';
import { parse_body } from './parse_body/index.js';
import { coalesce_to_error, lowercase_keys } from './utils.js';
import { hash } from '../hash.js';
Expand Down Expand Up @@ -84,7 +85,15 @@ export async function respond(incoming, options, state = {}) {
}
}

return await render_page(request, null, options, state);
const $session = await options.hooks.getSession(request);
return await respond_with_error({
request,
options,
state,
$session,
status: 404,
error: new Error(`Not found: ${request.path}`)
});
}
});
} catch (/** @type {unknown} */ err) {
Expand Down
38 changes: 14 additions & 24 deletions packages/kit/src/runtime/server/page/index.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import { respond } from './respond.js';
import { respond_with_error } from './respond_with_error.js';

/**
* @param {import('types/hooks').ServerRequest} request
* @param {import('types/internal').SSRPage | null} route
* @param {import('types/internal').SSRPage} route
* @param {import('types/internal').SSRRenderOptions} options
* @param {import('types/internal').SSRRenderState} state
* @returns {Promise<import('types/hooks').ServerResponse>}
* @returns {Promise<import('types/hooks').ServerResponse | undefined>}
*/
export default async function render_page(request, route, options, state) {
export async function render_page(request, route, options, state) {
if (state.initiator === route) {
// infinite request cycle detected
return {
Expand All @@ -20,19 +19,19 @@ export default async function render_page(request, route, options, state) {

const $session = await options.hooks.getSession(request);

if (route) {
const response = await respond({
request,
options,
state,
$session,
route
});
const response = await respond({
request,
options,
state,
$session,
route
});

if (response) {
return response;
}
if (response) {
return response;
}

if (state.fetched) {
// we came here because of a bad request in a `load` function.
// rather than render the error page — which could lead to an
// infinite loop, if the `load` belonged to the root layout,
Expand All @@ -42,14 +41,5 @@ export default async function render_page(request, route, options, state) {
headers: {},
body: `Bad request in load function: failed to fetch ${state.fetched}`
};
} else {
return await respond_with_error({
request,
options,
state,
$session,
status: 404,
error: new Error(`Not found: ${request.path}`)
});
}
}
22 changes: 15 additions & 7 deletions packages/kit/test/apps/basics/src/routes/routing/_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,15 +202,23 @@ export default function (test) {
}
);

test('falls through', '/routing/fallthrough/borax', async ({ page, clicknav }) => {
assert.equal(await page.textContent('h1'), 'borax is a mineral');
test('fallthrough', '/routing/fallthrough-simple/invalid', async ({ page, clicknav }) => {
assert.equal(await page.textContent('h1'), 'Page');
});

await clicknav('[href="/routing/fallthrough/camel"]');
assert.equal(await page.textContent('h1'), 'camel is an animal');
test(
'dynamic fallthrough of pages and endpoints',
'/routing/fallthrough-advanced/borax',
async ({ page, clicknav }) => {
assert.equal(await page.textContent('h1'), 'borax is a mineral');

await clicknav('[href="/routing/fallthrough/potato"]');
assert.equal(await page.textContent('h1'), '404');
});
await clicknav('[href="/routing/fallthrough-advanced/camel"]');
assert.equal(await page.textContent('h1'), 'camel is an animal');

await clicknav('[href="/routing/fallthrough-advanced/potato"]');
assert.equal(await page.textContent('h1'), '404');
}
);

test(
'last parameter in a segment wins in cases of ambiguity',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<script context="module">
/** @type {import("@sveltejs/kit").Load} */
export async function load({ page, fetch }) {
const res = await fetch(`/routing/fallthrough/${page.params.animal}.json`);
const res = await fetch(`/routing/fallthrough-advanced/${page.params.animal}.json`);
if (res.ok) {
const { type } = await res.json();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<script context="module">
/** @type {import("@sveltejs/kit").Load} */
export async function load({ page, fetch }) {
const res = await fetch(`/routing/fallthrough/${page.params.mineral}.json`);
const res = await fetch(`/routing/fallthrough-advanced/${page.params.mineral}.json`);
if (res.ok) {
const { type } = await res.json();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<script context="module">
/** @type {import("@sveltejs/kit").Load} */
export async function load({ page, fetch }) {
const res = await fetch(`/routing/fallthrough/${page.params.vegetable}.json`);
const res = await fetch(`/routing/fallthrough-advanced/${page.params.vegetable}.json`);
if (res.ok) {
const { type } = await res.json();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<slot></slot>

<a href="/routing/fallthrough-advanced/aluminium">aluminium</a>
<a href="/routing/fallthrough-advanced/broccoli">broccoli</a>
<a href="/routing/fallthrough-advanced/camel">camel</a>

<a href="/routing/fallthrough-advanced/potato">potato</a>
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<script context="module">
/** @type {import("@sveltejs/kit").Load} */
export const load = ({ page }) => {
if (page.path === '/valid') {
return {
props: {}
};
} else {
return;
}
};
</script>

<h1>Legal</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<script context="module">
/** @type {import("@sveltejs/kit").Load} */
export const load = ({ page }) => {
return {
props: {}
};
};
</script>

<h1>Page</h1>

This file was deleted.

0 comments on commit ee73a26

Please sign in to comment.