-> v0.7.6

don't serve error page for unhandled server route errors

CHANGELOG.md

@@ -1,5 +1,14 @@
 # sapper changelog
 
+## 0.7.6
+
+* Prevent client-side navigation to server route ([#145](https://github.com/sveltejs/sapper/issues/145))
+* Don't serve error page for server route errors ([#138](https://github.com/sveltejs/sapper/issues/138))
+
+## 0.7.5
+
+* Allow dynamic parameters inside route parts ([#139](https://github.com/sveltejs/sapper/issues/139))
+
 ## 0.7.4
 
 * Force `NODE_ENV='production'` when running `build` or `export` ([#141](https://github.com/sveltejs/sapper/issues/141))

package.json

@@ -1,6 +1,6 @@
 {
   "name": "sapper",
-  "version": "0.7.4",
+  "version": "0.7.6",
   "description": "Military-grade apps, engineered by Svelte",
   "main": "middleware.js",
   "bin": {

src/core/create_app.ts

@@ -22,19 +22,22 @@ function generate_client(routes: Route[], src: string, dev: boolean, dev_port?:
 		// This file is generated by Sapper — do not edit it!
 		export const routes = [
 			${routes
-				.filter(route => route.type === 'page')
 				.map(route => {
+					if (route.type !== 'page') {
+						return `{ pattern: ${route.pattern}, ignore: true }`;
+					}
+
 					const file = posixify(`../../routes/${route.file}`);
 
 					if (route.id === '_4xx' || route.id === '_5xx') {
 						return `{ error: '${route.id.slice(1)}', load: () => import(/* webpackChunkName: "${route.id}" */ '${file}') }`;
 					}
 
-					const params = route.dynamic.length === 0
+					const params = route.params.length === 0
 						? '{}'
-						: `{ ${route.dynamic.map((part, i) => `${part}: match[${i + 1}]`).join(', ')} }`;
+						: `{ ${route.params.map((part, i) => `${part}: match[${i + 1}]`).join(', ')} }`;
 
-					return `{ pattern: ${route.pattern}, params: ${route.dynamic.length > 0 ? `match` : `()`} => (${params}), load: () => import(/* webpackChunkName: "${route.id}" */ '${file}') }`;
+					return `{ pattern: ${route.pattern}, params: ${route.params.length > 0 ? `match` : `()`} => (${params}), load: () => import(/* webpackChunkName: "${route.id}" */ '${file}') }`;
 				})
 				.join(',\n\t')}
 		];`.replace(/^\t\t/gm, '').trim();
@@ -77,11 +80,11 @@ function generate_server(routes: Route[], src: string) {
 						return `{ error: '${route.id.slice(1)}', module: ${route.id} }`;
 					}
 
-					const params = route.dynamic.length === 0
+					const params = route.params.length === 0
 						? '{}'
-						: `{ ${route.dynamic.map((part, i) => `${part}: match[${i + 1}]`).join(', ')} }`;
+						: `{ ${route.params.map((part, i) => `${part}: match[${i + 1}]`).join(', ')} }`;
 
-					return `{ id: '${route.id}', type: '${route.type}', pattern: ${route.pattern}, params: ${route.dynamic.length > 0 ? `match` : `()`} => (${params}), module: ${route.id} }`;
+					return `{ id: '${route.id}', type: '${route.type}', pattern: ${route.pattern}, params: ${route.params.length > 0 ? `match` : `()`} => (${params}), module: ${route.id} }`;
 				})
 				.join(',\n\t')
 			}

src/core/create_routes.ts

@@ -10,17 +10,27 @@ export default function create_routes({ src, files = glob.sync('**/*.+(html|js|m
 		.map((file: string) => {
 			if (/(^|\/|\\)_/.test(file)) return;
 
-			const parts = file.replace(/\.(html|js|mjs)$/, '').split('/'); // glob output is always posix-style
+			if (/]\[/.test(file)) {
+				throw new Error(`Invalid route ${file} — parameters must be separated`);
+			}
+
+			const base = file.replace(/\.[^/.]+$/, '');
+			const parts = base.split('/'); // glob output is always posix-style
 			if (parts[parts.length - 1] === 'index') parts.pop();
 
 			const id = (
 				parts.join('_').replace(/[[\]]/g, '$').replace(/^\d/, '_$&').replace(/[^a-zA-Z0-9_$]/g, '_')
 			 ) || '_';
 
-			const dynamic = parts
-				.filter(part => part[0] === '[')
-				.map(part => part.slice(1, -1));
+			const params: string[] = [];
+			const param_pattern = /\[([^\]]+)\]/g;
+			let match;
+			while (match = param_pattern.exec(base)) {
+				params.push(match[1]);
+			}
 
+			// TODO can we do all this with sub-parts? or does
+			// nesting make that impossible?
 			let pattern_string = '';
 			let i = parts.length;
 			let nested = true;
@@ -29,7 +39,8 @@ export default function create_routes({ src, files = glob.sync('**/*.+(html|js|m
 				const dynamic = part[0] === '[';
 
 				if (dynamic) {
-					pattern_string = nested ? `(?:\\/([^/]+)${pattern_string})?` : `\\/([^/]+)${pattern_string}`;
+					const matcher = part.replace(param_pattern, `([^\/]+?)`);
+					pattern_string = nested ? `(?:\\/${matcher}${pattern_string})?` : `\\/${matcher}${pattern_string}`;
 				} else {
 					nested = false;
 					pattern_string = `\\/${part}${pattern_string}`;
@@ -44,12 +55,12 @@ export default function create_routes({ src, files = glob.sync('**/*.+(html|js|m
 				const match = pattern.exec(url);
 				if (!match) return;
 
-				const params: Record<string, string> = {};
-				dynamic.forEach((param, i) => {
-					params[param] = match[i + 1];
+				const result: Record<string, string> = {};
+				params.forEach((param, i) => {
+					result[param] = match[i + 1];
 				});
 
-				return params;
+				return result;
 			};
 
 			return {
@@ -60,7 +71,7 @@ export default function create_routes({ src, files = glob.sync('**/*.+(html|js|m
 				test,
 				exec,
 				parts,
-				dynamic
+				params
 			};
 		})
 		.filter(Boolean)
@@ -79,17 +90,40 @@ export default function create_routes({ src, files = glob.sync('**/*.+(html|js|m
 				if (!a_part) return -1;
 				if (!b_part) return 1;
 
-				const a_is_dynamic = a_part[0] === '[';
-				const b_is_dynamic = b_part[0] === '[';
+				const a_sub_parts = get_sub_parts(a_part);
+				const b_sub_parts = get_sub_parts(b_part);
 
-				if (a_is_dynamic === b_is_dynamic) {
-					if (!a_is_dynamic && a_part !== b_part) same = false;
-					continue;
+				for (let i = 0; true; i += 1) {
+					const a_sub_part = a_sub_parts[i];
+					const b_sub_part = b_sub_parts[i];
+
+					if (!a_sub_part && !b_sub_part) break;
+
+					if (!a_sub_part) return 1; // note this is reversed from above — match [foo].json before [foo]
+					if (!b_sub_part) return -1;
+
+					if (a_sub_part.dynamic !== b_sub_part.dynamic) {
+						return a_sub_part.dynamic ? 1 : -1;
+					}
+
+					if (!a_sub_part.dynamic && a_sub_part.content !== b_sub_part.content) {
+						return b_sub_part.content.length - a_sub_part.content.length;
+					}
 				}
-
-				return a_is_dynamic ? 1 : -1;
 			}
 		});
 
 	return routes;
+}
+
+function get_sub_parts(part: string) {
+	return part.split(/[\[\]]/)
+		.map((content, i) => {
+			if (!content) return null;
+			return {
+				content,
+				dynamic: i % 2 === 1
+			};
+		})
+		.filter(Boolean);
 }

src/interfaces.ts

@@ -6,7 +6,7 @@ export type Route = {
 	test: (url: string) => boolean;
 	exec: (url: string) => Record<string, string>;
 	parts: string[];
-	dynamic: string[];
+	params: string[];
 };
 
 export type Template = {

src/middleware/index.ts

@@ -234,9 +234,21 @@ function get_route_handler(chunks: Record<string, string>, routes: RouteObject[]
 					};
 				}
 
-				handler(req, res, () => {
-					handle_not_found(req, res, 404, 'Not found');
-				});
+				const handle_error = (err?: Error) => {
+					if (err) {
+						console.error(err.stack);
+						res.statusCode = 500;
+						res.end(err.message);
+					} else {
+						handle_not_found(req, res, 404, 'Not found');
+					}
+				};
+
+				try {
+					handler(req, res, handle_error);
+				} catch (err) {
+					handle_error(err);
+				}
 			} else {
 				// no matching handler for method — 404
 				handle_not_found(req, res, 404, 'Not found');
@@ -294,7 +306,7 @@ function get_route_handler(chunks: Record<string, string>, routes: RouteObject[]
 		}));
 	}
 
-	return function find_route(req: Req, res: ServerResponse, next: () => void) {
+	return function find_route(req: Req, res: ServerResponse) {
 		const url = req.pathname;
 
 		try {

src/runtime/index.ts

@@ -26,6 +26,8 @@ function select_route(url: URL): Target {
 	for (const route of routes) {
 		const match = route.pattern.exec(url.pathname);
 		if (match) {
+			if (route.ignore) return null;
+
 			const params = route.params(match);
 
 			const query: Record<string, string | true> = {};

src/runtime/interfaces.ts

@@ -14,7 +14,8 @@ export interface Component {
 export type Route = {
 	pattern: RegExp;
 	params: (match: RegExpExecArray) => Record<string, string>;
-	load: () => Promise<{ default: ComponentConstructor }>
+	load: () => Promise<{ default: ComponentConstructor }>;
+	ignore?: boolean;
 };
 
 export type ScrollPosition = {

test/app/routes/blog.json.js

@@ -1,4 +1,4 @@
-import posts from './_posts.js';
+import posts from './blog/_posts.js';
 
 const contents = JSON.stringify(posts.map(post => {
 	return {

test/app/routes/blog/[slug].html

@@ -63,7 +63,7 @@
 				return this.error(500, 'something went wrong');
 			}
 
-			return fetch(`/api/blog/${slug}`).then(r => {
+			return fetch(`/blog/${slug}.json`).then(r => {
 				if (r.status === 200) {
 					return r.json().then(post => ({ post }));
 					this.error(r.status, '')

test/app/routes/blog/_posts.js

@@ -70,7 +70,7 @@ const posts = [
 			<ul>
 				<li>It's powered by <a href='https://svelte.technology'>Svelte</a> instead of React, so it's faster and your apps are smaller</li>
 				<li>Instead of route masking, we encode route parameters in filenames. For example, the page you're looking at right now is <code>routes/blog/[slug].html</code></li>
-				<li>As well as pages (Svelte components, which render on server or client), you can create <em>server routes</em> in your <code>routes</code> directory. These are just <code>.js</code> files that export functions corresponding to HTTP methods, and receive Express <code>request</code> and <code>response</code> objects as arguments. This makes it very easy to, for example, add a JSON API such as the one powering this very page (look in <code>routes/api/blog</code>)</li>
+				<li>As well as pages (Svelte components, which render on server or client), you can create <em>server routes</em> in your <code>routes</code> directory. These are just <code>.js</code> files that export functions corresponding to HTTP methods, and receive Express <code>request</code> and <code>response</code> objects as arguments. This makes it very easy to, for example, add a JSON API such as the one <a href='/blog/how-is-sapper-different-from-next.json'>powering this very page</a></li>
 				<li>Links are just <code>&lt;a&gt;</code> elements, rather than framework-specific <code>&lt;Link&gt;</code> components. That means, for example, that <a href='/blog/how-can-i-get-involved'>this link right here</a>, despite being inside a blob of HTML, works with the router as you'd expect.</li>
 			</ul>
 		`

test/app/routes/blog/index.html

@@ -32,7 +32,7 @@
 		},
 
 		preload({ params, query }) {
-			return fetch(`/api/blog/contents`).then(r => r.json()).then(posts => {
+			return fetch(`/blog.json`).then(r => r.json()).then(posts => {
 				return { posts };
 			});
 		}

test/app/routes/throw-an-error.js

@@ -0,0 +1,3 @@
+export function get() {
+	throw new Error('nope');
+}

test/common/test.js

@@ -12,6 +12,10 @@ run('development');
 Nightmare.action('page', {
 	title(done) {
 		this.evaluate_now(() => document.querySelector('h1').textContent, done);
+	},
+
+	text(done) {
+		this.evaluate_now(() => document.body.textContent, done);
 	}
 });
 
@@ -193,7 +197,7 @@ function run(env) {
 						});
 					})
 					.then(requests => {
-						assert.ok(!!requests.find(r => r.url === '/api/blog/why-the-name'));
+						assert.ok(!!requests.find(r => r.url === '/blog/why-the-name.json'));
 					});
 			});
 
@@ -219,7 +223,7 @@ function run(env) {
 						});
 					})
 					.then(mouseover_requests => {
-						assert.ok(mouseover_requests.findIndex(r => r.url === '/api/blog/what-is-sapper') !== -1);
+						assert.ok(mouseover_requests.findIndex(r => r.url === '/blog/what-is-sapper.json') !== -1);
 
 						return capture(() => {
 							return nightmare
@@ -228,7 +232,7 @@ function run(env) {
 						});
 					})
 					.then(click_requests => {
-						assert.ok(click_requests.findIndex(r => r.url === '/api/blog/what-is-sapper') === -1);
+						assert.ok(click_requests.findIndex(r => r.url === '/blog/what-is-sapper.json') === -1);
 					});
 			});
 
@@ -376,6 +380,25 @@ function run(env) {
 						assert.equal(title, 'Internal server error');
 					});
 			});
+
+			it('does not attempt client-side navigation to server routes', () => {
+				return nightmare.goto(`${base}/blog/how-is-sapper-different-from-next`)
+					.init()
+					.click(`[href="/blog/how-is-sapper-different-from-next.json"]`)
+					.wait(200)
+					.page.text()
+					.then(text => {
+						JSON.parse(text);
+					});
+			});
+
+			it('does not serve error page for non-page errors', () => {
+				return nightmare.goto(`${base}/throw-an-error`)
+					.page.text()
+					.then(text => {
+						assert.equal(text, 'nope');
+					});
+			});
 		});
 
 		describe('headers', () => {
@@ -415,13 +438,13 @@ function run(env) {
 						'blog/what-is-sapper/index.html',
 						'blog/why-the-name/index.html',
 
-						'api/blog/contents',
-						'api/blog/a-very-long-post',
-						'api/blog/how-can-i-get-involved',
-						'api/blog/how-is-sapper-different-from-next',
-						'api/blog/how-to-use-sapper',
-						'api/blog/what-is-sapper',
-						'api/blog/why-the-name',
+						'blog.json',
+						'blog/a-very-long-post.json',
+						'blog/how-can-i-get-involved.json',
+						'blog/how-is-sapper-different-from-next.json',
+						'blog/how-to-use-sapper.json',
+						'blog/what-is-sapper.json',
+						'blog/why-the-name.json',
 
 						'favicon.png',
 						'global.css',

test/unit/create_routes.test.js

@@ -4,7 +4,7 @@ const { create_routes } = require('../../core.js');
 describe('create_routes', () => {
 	it('sorts routes correctly', () => {
 		const routes = create_routes({
-			files: ['index.html', 'about.html', '[wildcard].html', 'post/foo.html', 'post/[id].html', 'post/bar.html']
+			files: ['index.html', 'about.html', 'post/f[xx].html', '[wildcard].html', 'post/foo.html', 'post/[id].html', 'post/bar.html', 'post/[id].json.js']
 		});
 
 		assert.deepEqual(
@@ -14,6 +14,8 @@ describe('create_routes', () => {
 				'about.html',
 				'post/foo.html',
 				'post/bar.html',
+				'post/f[xx].html',
+				'post/[id].json.js',
 				'post/[id].html',
 				'[wildcard].html'
 			]
@@ -133,4 +135,33 @@ describe('create_routes', () => {
 			b: null
 		});
 	});
+
+	it('matches a dynamic part within a part', () => {
+		const route = create_routes({
+			files: ['things/[slug].json.js']
+		})[0];
+
+		assert.deepEqual(route.exec('/things/foo.json'), {
+			slug: 'foo'
+		});
+	});
+
+	it('matches multiple dynamic parts within a part', () => {
+		const route = create_routes({
+			files: ['things/[id]_[slug].json.js']
+		})[0];
+
+		assert.deepEqual(route.exec('/things/someid_someslug.json'), {
+			id: 'someid',
+			slug: 'someslug'
+		});
+	});
+
+	it('fails if dynamic params are not separated', () => {
+		assert.throws(() => {
+			create_routes({
+				files: ['[foo][bar].js']
+			});
+		}, /Invalid route \[foo\]\[bar\]\.js — parameters must be separated/);
+	});
 });