Remove fallthrough (#4330)

* remove fallthrough * changeset * remove fallthrough documentation * tweak docs * simplify * simplify * simplify a tiny bit * lint * oops
sveltejs · Mar 16, 2022 · 1f0d822 · 1f0d822
1 parent fac2844
commit 1f0d822
Show file tree

Hide file tree

Showing 33 changed files with 74 additions and 447 deletions.
diff --git a/.changeset/nasty-lemons-provide.md b/.changeset/nasty-lemons-provide.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+[breaking] remove fallthrough routes
diff --git a/documentation/docs/01-routing.md b/documentation/docs/01-routing.md
@@ -323,11 +323,10 @@ A route can have multiple dynamic parameters, for example `src/routes/[category]
 It's possible for multiple routes to match a given path. For example each of these routes would match `/foo-abc`:
 
 ```bash
+src/routes/[...catchall].svelte
 src/routes/[a].js
 src/routes/[b].svelte
-src/routes/[c].svelte
-src/routes/[...catchall].svelte
-src/routes/foo-[bar].svelte
+src/routes/foo-[c].svelte
 ```
 
 SvelteKit needs to know which route is being requested. To do so, it sorts them according to the following rules...
@@ -340,48 +339,8 @@ SvelteKit needs to know which route is being requested. To do so, it sorts them
 ...resulting in this ordering, meaning that `/foo-abc` will invoke `src/routes/foo-[bar].svelte` rather than a less specific route:
 
 ```bash
-src/routes/foo-[bar].svelte
+src/routes/foo-[c].svelte
 src/routes/[a].js
 src/routes/[b].svelte
-src/routes/[c].svelte
 src/routes/[...catchall].svelte
 ```
-
-#### Fallthrough routes
-
-In rare cases, the ordering above might not be what you want for a given path. For example, perhaps `/foo-abc` should resolve to `src/routes/foo-[bar].svelte`, but `/foo-def` should resolve to `src/routes/[b].svelte`.
-
-Higher priority routes can _fall through_ to lower priority routes by returning `{ fallthrough: true }`, either from `load` (for pages) or a request handler (for endpoints):
-
-```svelte
-/// file: src/routes/foo-[bar].svelte
-<script context="module">
-	export function load({ params }) {
-		if (params.bar === 'def') {
-			return { fallthrough: true };
-		}
-
-		// ...
-	}
-</script>
-```
-
-```js
-/// file: src/routes/[a].js
-
-// @filename: [a].d.ts
-import type { RequestHandler as GenericRequestHandler } from '@sveltejs/kit';
-export type RequestHandler<Body = any> = GenericRequestHandler<{ a: string }, Body>;
-
-// @filename: index.js
-// @errors: 2366
-// ---cut---
-/** @type {import('./[a]').RequestHandler} */
-export function get({ params }) {
-	if (params.a === 'foo-def') {
-		return { fallthrough: true };
-	}
-
-	// ...
-}
-```
diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js
@@ -177,7 +177,7 @@ export function create_client({ target, session, base, trailing_slash }) {
 
 		const intent = get_navigation_intent(url);
 
-		load_cache.promise = get_navigation_result(intent, false);
+		load_cache.promise = load_route(intent, false);
 		load_cache.id = intent.id;
 
 		return load_cache.promise;
@@ -191,7 +191,7 @@ export function create_client({ target, session, base, trailing_slash }) {
 	 */
 	async function update(intent, redirect_chain, no_cache, opts) {
 		const current_token = (token = {});
-		let navigation_result = await get_navigation_result(intent, no_cache);
+		let navigation_result = await load_route(intent, no_cache);
 
 		if (!navigation_result && intent.url.pathname === location.pathname) {
 			// this could happen in SPA fallback mode if the user navigated to
@@ -341,36 +341,6 @@ export function create_client({ target, session, base, trailing_slash }) {
 		}
 	}
 
-	/**
-	 * @param {import('./types').NavigationIntent} intent
-	 * @param {boolean} no_cache
-	 */
-	async function get_navigation_result(intent, no_cache) {
-		if (load_cache.id === intent.id && load_cache.promise) {
-			return load_cache.promise;
-		}
-
-		for (let i = 0; i < intent.routes.length; i += 1) {
-			const route = intent.routes[i];
-
-			// load code for subsequent routes immediately, if they are as
-			// likely to match the current path/query as the current one
-			let j = i + 1;
-			while (j < intent.routes.length) {
-				const next = intent.routes[j];
-				if (next[0].toString() === route[0].toString()) {
-					next[1].forEach((loader) => loader());
-					j += 1;
-				} else {
-					break;
-				}
-			}
-
-			const result = await load_route(route, intent, no_cache);
-			if (result) return result;
-		}
-	}
-
 	/**
 	 *
 	 * @param {{
@@ -557,11 +527,16 @@ export function create_client({ target, session, base, trailing_slash }) {
 	}
 
 	/**
-	 * @param {import('types').CSRRoute} route
 	 * @param {import('./types').NavigationIntent} intent
 	 * @param {boolean} no_cache
 	 */
-	async function load_route(route, { id, url, path, routes }, no_cache) {
+	async function load_route({ id, url, path, route }, no_cache) {
+		if (!route) return;
+
+		if (load_cache.id === id && load_cache.promise) {
+			return load_cache.promise;
+		}
+
 		if (!no_cache) {
 			const cached = cache.get(id);
 			if (cached) return cached;
@@ -625,7 +600,7 @@ export function create_client({ target, session, base, trailing_slash }) {
 							`${url.pathname}${url.pathname.endsWith('/') ? '' : '/'}__data.json${url.search}`,
 							{
 								headers: {
-									'x-sveltekit-load': /** @type {string} */ (shadow_key)
+									'x-sveltekit-load': 'true'
 								}
 							}
 						);
@@ -641,15 +616,7 @@ export function create_client({ target, session, base, trailing_slash }) {
 								};
 							}
 
-							if (res.status === 204) {
-								if (route !== routes[routes.length - 1]) {
-									// fallthrough
-									return;
-								}
-								props = {};
-							} else {
-								props = await res.json();
-							}
+							props = res.status === 204 ? {} : await res.json();
 						} else {
 							status = res.status;
 							error = new Error('Failed to load data');
@@ -672,9 +639,12 @@ export function create_client({ target, session, base, trailing_slash }) {
 						}
 
 						if (node.loaded) {
+							// TODO remove for 1.0
+							// @ts-expect-error
 							if (node.loaded.fallthrough) {
-								return;
+								throw new Error('fallthrough is no longer supported');
 							}
+
 							if (node.loaded.error) {
 								status = node.loaded.status;
 								error = node.loaded.error;
@@ -811,6 +781,7 @@ export function create_client({ target, session, base, trailing_slash }) {
 
 	/** @param {URL} url */
 	function owns(url) {
+		// TODO now that we've got rid of fallthrough, check against routes immediately
 		return url.origin === location.origin && url.pathname.startsWith(base);
 	}
 
@@ -821,7 +792,7 @@ export function create_client({ target, session, base, trailing_slash }) {
 		/** @type {import('./types').NavigationIntent} */
 		const intent = {
 			id: url.pathname + url.search,
-			routes: routes.filter(([pattern]) => pattern.test(path)),
+			route: routes.find(([pattern]) => pattern.test(path)),
 			url,
 			path
 		};
@@ -860,14 +831,14 @@ export function create_client({ target, session, base, trailing_slash }) {
 			return;
 		}
 
-		if (!owns(url)) {
+		const pathname = normalize_path(url.pathname, trailing_slash);
+		const normalized = new URL(url.origin + pathname + url.search + url.hash);
+
+		if (!owns(normalized)) {
 			await native_navigation(url);
 		}
 
-		const pathname = normalize_path(url.pathname, trailing_slash);
-		url = new URL(url.origin + pathname + url.search + url.hash);
-
-		const intent = get_navigation_intent(url);
+		const intent = get_navigation_intent(normalized);
 
 		update_scroll_positions(current_history_index);
 
@@ -896,7 +867,7 @@ export function create_client({ target, session, base, trailing_slash }) {
 		if (navigating_token !== current_navigating_token) return;
 
 		if (!navigating) {
-			const navigation = { from, to: url };
+			const navigation = { from, to: normalized };
 			callbacks.after_navigate.forEach((fn) => fn(navigation));
 
 			stores.navigating.set(null);

diff --git a/packages/kit/src/runtime/client/types.d.ts b/packages/kit/src/runtime/client/types.d.ts
@@ -38,9 +38,9 @@ export type NavigationIntent = {
 	 */
 	path: string;
 	/**
-	 * The routes that could satisfy this navigation intent
+	 * The route that matches `path`
 	 */
-	routes: CSRRoute[];
+	route: CSRRoute | undefined; // TODO i'm pretty sure we can make this required, and simplify some stuff
 	/**
 	 * The destination URL
 	 */

diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js
@@ -59,8 +59,10 @@ export async function render_endpoint(event, mod) {
 		return error(`${preface}: expected an object, got ${typeof response}`);
 	}
 
+	// TODO remove for 1.0
+	// @ts-expect-error
 	if (response.fallthrough) {
-		return;
+		throw new Error('fallthrough is no longer supported');
 	}
 
 	const { status = 200, body = {} } = response;

diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js
@@ -165,17 +165,7 @@ export async function respond(request, options, state) {
 					event.url = new URL(event.url.origin + normalized + event.url.search);
 				}
 
-				// `key` will be set if this request came from a client-side navigation
-				// to a page with a matching endpoint
-				const key = request.headers.get('x-sveltekit-load');
-
 				for (const route of options.manifest._.routes) {
-					if (key) {
-						// client is requesting data for a specific endpoint
-						if (route.type !== 'page') continue;
-						if (route.key !== key) continue;
-					}
-
 					const match = route.pattern.exec(decoded);
 					if (!match) continue;
 
@@ -188,7 +178,7 @@ export async function respond(request, options, state) {
 						response = await render_endpoint(event, await route.shadow());
 
 						// loading data for a client-side transition is a special case
-						if (key) {
+						if (request.headers.has('x-sveltekit-load')) {
 							if (response) {
 								// since redirects are opaque to the browser, we need to repackage
 								// 3xx responses as 200s with a custom header
@@ -205,12 +195,8 @@ export async function respond(request, options, state) {
 									}
 								}
 							} else {
-								// fallthrough
 								response = new Response(undefined, {
-									status: 204,
-									headers: {
-										'content-type': 'application/json'
-									}
+									status: 204
 								});
 							}
 						}
@@ -257,6 +243,8 @@ export async function respond(request, options, state) {
 
 						return response;
 					}
+
+					break;
 				}
 
 				// if this request came direct from the user, rather than

diff --git a/packages/kit/src/runtime/server/page/load_node.js b/packages/kit/src/runtime/server/page/load_node.js
@@ -21,7 +21,7 @@ import { coalesce_to_error } from '../../../utils/error.js';
  *   status?: number;
  *   error?: Error;
  * }} opts
- * @returns {Promise<import('./types').Loaded | undefined>} undefined for fallthrough
+ * @returns {Promise<import('./types').Loaded>}
  */
 export async function load_node({
 	event,
@@ -50,7 +50,7 @@ export async function load_node({
 	 */
 	let set_cookie_headers = [];
 
-	/** @type {import('types').Either<import('types').Fallthrough, import('types').LoadOutput>} */
+	/** @type {import('types').LoadOutput} */
 	let loaded;
 
 	/** @type {import('types').ShadowData} */
@@ -63,8 +63,6 @@ export async function load_node({
 		  )
 		: {};
 
-	if (shadow.fallthrough) return;
-
 	if (shadow.cookies) {
 		set_cookie_headers.push(...shadow.cookies);
 	}
@@ -325,8 +323,15 @@ export async function load_node({
 		loaded = await module.load.call(null, load_input);
 
 		if (!loaded) {
+			// TODO do we still want to enforce this now that there's no fallthrough?
 			throw new Error(`load function must return a value${options.dev ? ` (${node.entry})` : ''}`);
 		}
+
+		// TODO remove for 1.0
+		// @ts-expect-error
+		if (loaded.fallthrough) {
+			throw new Error('fallthrough is no longer supported');
+		}
 	} else if (shadow.body) {
 		loaded = {
 			props: shadow.body
@@ -335,10 +340,6 @@ export async function load_node({
 		loaded = {};
 	}
 
-	if (loaded.fallthrough && !is_error) {
-		return;
-	}
-
 	// generate __data.json files when prerendering
 	if (shadow.body && state.prerender) {
 		const pathname = `${event.url.pathname.replace(/\/$/, '')}/__data.json`;
@@ -401,7 +402,11 @@ async function load_shadow_data(route, event, options, prerender) {
 		if (!is_get) {
 			const result = await handler(event);
 
-			if (result.fallthrough) return result;
+			// TODO remove for 1.0
+			// @ts-expect-error
+			if (result.fallthrough) {
+				throw new Error('fallthrough is no longer supported');
+			}
 
 			const { status, headers, body } = validate_shadow_output(result);
 			data.status = status;
@@ -426,7 +431,11 @@ async function load_shadow_data(route, event, options, prerender) {
 		if (get) {
 			const result = await get(event);
 
-			if (result.fallthrough) return result;
+			// TODO remove for 1.0
+			// @ts-expect-error
+			if (result.fallthrough) {
+				throw new Error('fallthrough is no longer supported');
+			}
 
 			const { status, headers, body } = validate_shadow_output(result);
 			add_cookies(/** @type {string[]} */ (data.cookies), headers);

diff --git a/packages/kit/test/apps/basics/src/routes/endpoint-output/empty.js b/packages/kit/test/apps/basics/src/routes/endpoint-output/empty.js
@@ -2,8 +2,3 @@
 export function get() {
 	return {};
 }
-
-/** @type {import('@sveltejs/kit').RequestHandler} */
-export function del() {
-	return { fallthrough: true };
-}