prevent unnecessary promise chains, and hoist handler function

lib/index.js

@@ -23,9 +23,11 @@ function connect_dev() {
 			heartbeat: 10 * 1000
 		}),
 
-		async (req, res, next) => {
-			asset_cache = await watcher.ready;
-			next();
+		(req, res, next) => {
+			watcher.ready.then(cache => {
+				asset_cache = cache;
+				next();
+			});
 		},
 
 		set_req_pathname,
@@ -125,81 +127,88 @@ function get_asset_handler(opts) {
 	};
 }
 
-function get_route_handler(fn) {
-	return async function handle_route(req, res, next) {
-		const url = req.pathname;
+const resolved = Promise.resolve();
 
-		const { client, server } = fn();
+function get_route_handler(fn) {
+	function handle_route(route, req, res, next, { client, server }) {
+		req.params = route.exec(req.pathname);
+
+		const mod = require(server.entry)[route.id];
+
+		if (route.type === 'page') {
+			// preload main.js and current route
+			// TODO detect other stuff we can preload? images, CSS, fonts?
+			res.set('Link', `<${client.main_file}>;rel="preload";as="script", <${client.routes[route.id]}>;rel="preload";as="script"`);
+
+			const data = { params: req.params, query: req.query };
+
+			if (mod.preload) {
+				const promise = Promise.resolve(mod.preload(req)).then(preloaded => {
+					Object.assign(data, preloaded);
+					return mod.render(data);
+				});
+
+				return templates.stream(res, 200, {
+					main: client.main_file,
+					html: promise.then(rendered => rendered.html),
+					head: promise.then(({ head }) => `<noscript id='sapper-head-start'></noscript>${head}<noscript id='sapper-head-end'></noscript>`),
+					styles: promise.then(({ css }) => (css && css.code ? `<style>${css.code}</style>` : ''))
+				});
+			} else {
+				const { html, head, css } = mod.render(data);
+
+				const page = templates.render(200, {
+					main: client.main_file,
+					html,
+					head: `<noscript id='sapper-head-start'></noscript>${head}<noscript id='sapper-head-end'></noscript>`,
+					styles: (css && css.code ? `<style>${css.code}</style>` : '')
+				});
+
+				res.end(page);
+			}
+		}
+
+		else {
+			const method = req.method.toLowerCase();
+			// 'delete' cannot be exported from a module because it is a keyword,
+			// so check for 'del' instead
+			const method_export = method === 'delete' ? 'del' : method;
+			const handler = mod[method_export];
+			if (handler) {
+				handler(req, res, next);
+			} else {
+				// no matching handler for method — 404
+				next();
+			}
+		}
+	}
+
+	return function find_route(req, res, next) {
+		const url = req.pathname;
 
 		// whatever happens, we're going to serve some HTML
 		res.set({
 			'Content-Type': 'text/html'
 		});
 
-		try {
-			for (const route of route_manager.routes) {
-				if (route.test(url)) {
-					req.params = route.exec(url);
-
-					const mod = require(server.entry)[route.id];
-
-					if (route.type === 'page') {
-						// preload main.js and current route
-						// TODO detect other stuff we can preload? images, CSS, fonts?
-						res.set('Link', `<${client.main_file}>;rel="preload";as="script", <${client.routes[route.id]}>;rel="preload";as="script"`);
-
-						const data = { params: req.params, query: req.query };
-
-						if (mod.preload) {
-							const promise = Promise.resolve(mod.preload(req)).then(preloaded => {
-								Object.assign(data, preloaded);
-								return mod.render(data);
-							});
-
-							await templates.stream(res, 200, {
-								main: client.main_file,
-								html: promise.then(rendered => rendered.html),
-								head: promise.then(({ head }) => `<noscript id='sapper-head-start'></noscript>${head}<noscript id='sapper-head-end'></noscript>`),
-								styles: promise.then(({ css }) => (css && css.code ? `<style>${css.code}</style>` : ''))
-							});
-						} else {
-							const { html, head, css } = mod.render(data);
-
-							const page = templates.render(200, {
-								main: client.main_file,
-								html,
-								head: `<noscript id='sapper-head-start'></noscript>${head}<noscript id='sapper-head-end'></noscript>`,
-								styles: (css && css.code ? `<style>${css.code}</style>` : '')
-							});
-
-							res.end(page);
-						}
-
-						return;
-					}
-
-					const method = req.method.toLowerCase();
-					// 'delete' cannot be exported from a module because it is a keyword,
-					// so check for 'del' instead
-					const method_export = method === 'delete' ? 'del' : method;
-					const handler = mod[method_export];
-					if (handler) {
-						handler(req, res, next);
-						return;
-					}
+		resolved
+			.then(() => {
+				for (const route of route_manager.routes) {
+					if (route.test(url)) return handle_route(route, req, res, next, fn());
 				}
-			}
 
-			next();
-		} catch(err) {
-			res.status(500);
-			res.end(templates.render(500, {
-				title: (err && err.name) || 'Internal server error',
-				url,
-				error: escape_html(err && (err.details || err.message || err) || 'Unknown error'),
-				stack: err && err.stack.split('\n').slice(1).join('\n')
-			}));
-		}
+				// no matching route — 404
+				next();
+			})
+			.catch(err => {
+				res.status(500);
+				res.end(templates.render(500, {
+					title: (err && err.name) || 'Internal server error',
+					url,
+					error: escape_html(err && (err.details || err.message || err) || 'Unknown error'),
+					stack: err && err.stack.split('\n').slice(1).join('\n')
+				}));
+			});
 	};
 }
 

lib/templates.js

@@ -31,10 +31,14 @@ function create_templates() {
 						return key in data ? data[key] : '';
 					});
 				},
-				stream: async (res, data) => {
+				stream: (res, data) => {
 					let i = 0;
 
-					do {
+					function stream_inner() {
+						if (i >= template.length) {
+							return;
+						}
+
 						const start = template.indexOf('%sapper', i);
 
 						if (start === -1) {
@@ -53,9 +57,14 @@ function create_templates() {
 						const match = /sapper\.(\w+)/.exec(tag);
 						if (!match || !(match[1] in data)) throw new Error(`Bad template`); // TODO ditto
 
-						res.write(await data[match[1]]);
-						i = end + 1;
-					} while (i < template.length);
+						return Promise.resolve(data[match[1]]).then(datamatch => {
+							res.write(datamatch);
+							i = end + 1;
+							return stream_inner();
+						});
+					}
+
+					return Promise.resolve().then(stream_inner);
 				}
 			};
 		})

test/common/test.js

@@ -62,52 +62,60 @@ function run(env) {
 			});
 		}
 
-		before(async () => {
+		before(() => {
 			process.chdir(path.resolve(__dirname, '../app'));
 
 			process.env.NODE_ENV = env;
 
+			let exec_promise = Promise.resolve();
+			let sapper;
+
 			if (env === 'production') {
 				const cli = path.resolve(__dirname, '../../cli/index.js');
-				await exec(`${cli} build`);
+				exec_promise = exec(`${cli} build`);
 			}
 
-			const resolved = require.resolve('../..');
-			delete require.cache[resolved];
-			const sapper = require(resolved);
+			return exec_promise.then(() => {
+				const resolved = require.resolve('../..');
+				delete require.cache[resolved];
+				sapper = require(resolved);
 
-			PORT = await getPort();
-			base = `http://localhost:${PORT}`;
+				return getPort();
+			}).then(port => {
+				PORT = port;
+				base = `http://localhost:${PORT}`;
 
-			global.fetch = (url, opts) => {
-				if (url[0] === '/') url = `${base}${url}`;
-				return fetch(url, opts);
-			};
+				global.fetch = (url, opts) => {
+					if (url[0] === '/') url = `${base}${url}`;
+					return fetch(url, opts);
+				};
 
-			let captured;
-			capture = async fn => {
-				const result = captured = [];
-				await fn();
-				captured = null;
-				return result;
-			};
+				let captured;
+				capture = fn => {
+					const result = captured = [];
+					return fn().then(() => {
+						captured = null;
+						return result;
+					});
+				};
 
-			const app = express();
+				const app = express();
 
-			app.use(serve('assets'));
+				app.use(serve('assets'));
 
-			app.use((req, res, next) => {
-				if (captured) captured.push(req);
-				next();
-			});
+				app.use((req, res, next) => {
+					if (captured) captured.push(req);
+					next();
+				});
 
-			middleware = sapper();
-			app.use(middleware);
+				middleware = sapper();
+				app.use(middleware);
 
-			return new Promise((fulfil, reject) => {
-				server = app.listen(PORT, err => {
-					if (err) reject(err);
-					else fulfil();
+				return new Promise((fulfil, reject) => {
+					server = app.listen(PORT, err => {
+						if (err) reject(err);
+						else fulfil();
+					});
 				});
 			});
 		});
@@ -137,157 +145,170 @@ function run(env) {
 				});
 			});
 
-			afterEach(async () => {
-				await nightmare.end();
+			afterEach(() => {
+				return nightmare.end();
 			});
 
-			it('serves /', async () => {
-				const title = await nightmare
+			it('serves /', () => {
+				return nightmare
 					.goto(base)
-					.evaluate(() => document.querySelector('h1').textContent);
-
-				assert.equal(title, 'Great success!');
+					.evaluate(() => document.querySelector('h1').textContent)
+					.then(title => {
+						assert.equal(title, 'Great success!');
+					});
 			});
 
-			it('serves static route', async () => {
-				const title = await nightmare
+			it('serves static route', () => {
+				return nightmare
 					.goto(`${base}/about`)
-					.evaluate(() => document.querySelector('h1').textContent);
-
-				assert.equal(title, 'About this site');
+					.evaluate(() => document.querySelector('h1').textContent)
+					.then(title => {
+						assert.equal(title, 'About this site');
+					});
 			});
 
-			it('serves dynamic route', async () => {
-				const title = await nightmare
+			it('serves dynamic route', () => {
+				return nightmare
 					.goto(`${base}/blog/what-is-sapper`)
-					.evaluate(() => document.querySelector('h1').textContent);
-
-				assert.equal(title, 'What is Sapper?');
+					.evaluate(() => document.querySelector('h1').textContent)
+					.then(title => {
+						assert.equal(title, 'What is Sapper?');
+					});
 			});
 
-			it('navigates to a new page without reloading', async () => {
-				await nightmare.goto(base).wait(() => window.READY).wait(200);
+			it('navigates to a new page without reloading', () => {
+				let requests;
+				return nightmare
+					.goto(base).wait(() => window.READY).wait(200)
+					.then(() => {
+						return capture(() => {
+							return nightmare.click('a[href="/about"]');
+						});
+					})
+					.then(reqs => {
+						requests = reqs;
 
-				const requests = await capture(async () => {
-					await nightmare.click('a[href="/about"]');
-				});
+						return nightmare.path();
+					})
+					.then(path => {
+						assert.equal(path, '/about');
 
-				assert.equal(
-					await nightmare.path(),
-					'/about'
-				);
+						return nightmare.evaluate(() => document.title);
+					})
+					.then(title => {
+						assert.equal(title, 'About');
 
-				assert.equal(
-					await nightmare.evaluate(() => document.title),
-					'About'
-				);
-
-				assert.deepEqual(requests.map(r => r.url), []);
+						assert.deepEqual(requests.map(r => r.url), []);
+					});
 			});
 
-			it('navigates programmatically', async () => {
-				await nightmare
+			it('navigates programmatically', () => {
+				return nightmare
 					.goto(`${base}/about`)
 					.wait(() => window.READY)
 					.click('.goto')
 					.wait(() => window.location.pathname === '/blog/what-is-sapper')
-					.wait(100);
-
-				assert.equal(
-					await nightmare.evaluate(() => document.title),
-					'What is Sapper?'
-				);
+					.wait(100)
+					.then(() => nightmare.evaluate(() => document.title))
+					.then(title => {
+						assert.equal(title, 'What is Sapper?');
+					});
 			});
 
-			it('prefetches programmatically', async () => {
-				await nightmare
+			it('prefetches programmatically', () => {
+				return nightmare
 					.goto(`${base}/about`)
-					.wait(() => window.READY);
-
-				const requests = await capture(async () => {
-					return await nightmare
-						.click('.prefetch')
-						.wait(100);
-				});
-
-				assert.ok(!!requests.find(r => r.url === '/api/blog/why-the-name'));
+					.wait(() => window.READY)
+					.then(() => {
+						return capture(() => {
+							return nightmare
+								.click('.prefetch')
+								.wait(100);
+						});
+					})
+					.then(requests => {
+						assert.ok(!!requests.find(r => r.url === '/api/blog/why-the-name'));
+					});
 			});
 
-			it('scrolls to active deeplink', async () => {
-				const scrollY = await nightmare
+			it('scrolls to active deeplink', () => {
+				return nightmare
 					.goto(`${base}/blog/a-very-long-post#four`)
 					.wait(() => window.READY)
 					.wait(100)
-					.evaluate(() => window.scrollY);
-
-				assert.ok(scrollY > 0, scrollY);
+					.evaluate(() => window.scrollY)
+					.then(scrollY => {
+						assert.ok(scrollY > 0, scrollY);
+					});
 			});
 
-			it('reuses prefetch promise', async () => {
-				await nightmare
+			it('reuses prefetch promise', () => {
+				return nightmare
 					.goto(`${base}/blog`)
 					.wait(() => window.READY)
-					.wait(200);
+					.wait(200)
+					.then(() => {
+						return capture(() => {
+							return nightmare
+								.mouseover('[href="/blog/what-is-sapper"]')
+								.wait(200);
+						});
+					})
+					.then(mouseover_requests => {
+						assert.deepEqual(mouseover_requests.map(r => r.url), [
+							'/api/blog/what-is-sapper'
+						]);
 
-				const mouseover_requests = (await capture(async () => {
-					await nightmare
-						.mouseover('[href="/blog/what-is-sapper"]')
-						.wait(200);
-				})).map(r => r.url);
-
-				assert.deepEqual(mouseover_requests, [
-					'/api/blog/what-is-sapper'
-				]);
-
-				const click_requests = (await capture(async () => {
-					await nightmare
-						.click('[href="/blog/what-is-sapper"]')
-						.wait(200);
-				})).map(r => r.url);
-
-				assert.deepEqual(click_requests, []);
+						return capture(() => {
+							return nightmare
+								.click('[href="/blog/what-is-sapper"]')
+								.wait(200);
+						});
+					})
+					.then(click_requests => {
+						assert.deepEqual(click_requests.map(r => r.url), []);
+					});
 			});
 
-			it('cancels navigation if subsequent navigation occurs during preload', async () => {
-				await nightmare
+			it('cancels navigation if subsequent navigation occurs during preload', () => {
+				return nightmare
 					.goto(base)
 					.wait(() => window.READY)
 					.click('a[href="/slow-preload"]')
 					.wait(100)
 					.click('a[href="/about"]')
-					.wait(100);
+					.wait(100)
+					.then(() => nightmare.path())
+					.then(path => {
+						assert.equal(path, '/about');
 
-				assert.equal(
-					await nightmare.path(),
-					'/about'
-				);
+						return nightmare.evaluate(() => document.querySelector('h1').textContent);
+					})
+					.then(header_text => {
+						assert.equal(header_text, 'About this site');
 
-				assert.equal(
-					await nightmare.evaluate(() => document.querySelector('h1').textContent),
-					'About this site'
-				);
+						return nightmare.evaluate(() => window.fulfil({})).wait(100);
+					})
+					.then(() => nightmare.path())
+					.then(path => {
+						assert.equal(path, '/about');
 
-				await nightmare
-					.evaluate(() => window.fulfil({}))
-					.wait(100);
+						return nightmare.evaluate(() => document.querySelector('h1').textContent);
+					})
+					.then(header_text => {
+						assert.equal(header_text, 'About this site');
 
-				assert.equal(
-					await nightmare.path(),
-					'/about'
-				);
-
-				assert.equal(
-					await nightmare.evaluate(() => document.querySelector('h1').textContent),
-					'About this site'
-				);
+						return nightmare.evaluate(() => window.fulfil({})).wait(100);
+					});
 			});
 
-			it('passes entire request object to preload', async () => {
-				const html = await nightmare
+			it('passes entire request object to preload', () => {
+				return nightmare
 					.goto(`${base}/show-url`)
-					.evaluate(() => document.querySelector('p').innerHTML);
-
-				assert.equal(html, `URL is /show-url`);
+					.evaluate(() => document.querySelector('p').innerHTML)
+					.then(html => {
+						assert.equal(html, `URL is /show-url`);
+					});
 			});
 
 			it('calls a delete handler', async () => {
@@ -305,18 +326,18 @@ function run(env) {
 		});
 
 		describe('headers', () => {
-			it('sets Content-Type and Link...preload headers', async () => {
-				const { headers } = await get('/');
+			it('sets Content-Type and Link...preload headers', () => {
+				return get('/').then(({ headers }) => {
+					assert.equal(
+						headers['Content-Type'],
+						'text/html'
+					);
 
-				assert.equal(
-					headers['Content-Type'],
-					'text/html'
-				);
-
-				assert.ok(
-					/<\/client\/main.\w+\.js>;rel="preload";as="script", <\/client\/_.\d+.\w+.js>;rel="preload";as="script"/.test(headers['Link']),
-					headers['Link']
-				);
+					assert.ok(
+						/<\/client\/main.\w+\.js>;rel="preload";as="script", <\/client\/_.\d+.\w+.js>;rel="preload";as="script"/.test(headers['Link']),
+						headers['Link']
+					);
+				});
 			});
 		});
 	});