From 3d94ae0a7d3034c86f0a33904c8625f6ff92a9b8 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 1 Oct 2018 16:31:47 +0200 Subject: [PATCH] Drop prepare requirement from production server (#5351) As prepare is only needed to boot up the hot reloader + exportPathMap routes in development, it's not longer a requirement in the production server. --- packages/next/server/next-dev-server.js | 62 ++++++++++--------- packages/next/server/next-server.js | 30 +++------ packages/next/server/router.js | 25 ++++---- .../integration/production/test/index.test.js | 15 +++++ 4 files changed, 69 insertions(+), 63 deletions(-) diff --git a/packages/next/server/next-dev-server.js b/packages/next/server/next-dev-server.js index bcf84cadc7..34b0184a19 100644 --- a/packages/next/server/next-dev-server.js +++ b/packages/next/server/next-dev-server.js @@ -1,6 +1,7 @@ import Server from './next-server' import { join } from 'path' import HotReloader from './hot-reloader' +import {route} from './router' import {PHASE_DEVELOPMENT_SERVER} from '../lib/constants' export default class DevServer extends Server { @@ -19,8 +20,37 @@ export default class DevServer extends Server { return 'development' } + async addExportPathMapRoutes () { + // Makes `next export` exportPathMap work in development mode. + // So that the user doesn't have to define a custom server reading the exportPathMap + if (this.nextConfig.exportPathMap) { + console.log('Defining routes from exportPathMap') + const exportPathMap = await this.nextConfig.exportPathMap({}, {dev: true, dir: this.dir, outDir: null, distDir: this.distDir, buildId: this.buildId}) // In development we can't give a default path mapping + for (const path in exportPathMap) { + const {page, query = {}} = exportPathMap[path] + + // We use unshift so that we're sure the routes is defined before Next's default routes + this.router.add({ + match: route(path), + fn: async (req, res, params, parsedUrl) => { + const { query: urlQuery } = parsedUrl + + Object.keys(urlQuery) + .filter(key => query[key] === undefined) + .forEach(key => console.warn(`Url defines a query parameter '${key}' that is missing in exportPathMap`)) + + const mergedQuery = {...urlQuery, ...query} + + await this.render(req, res, page, mergedQuery, parsedUrl) + } + }) + } + } + } + async prepare () { await super.prepare() + await this.addExportPathMapRoutes() await this.hotReloader.start() } @@ -37,45 +67,19 @@ export default class DevServer extends Server { return super.run(req, res, parsedUrl) } - async generateRoutes () { - const routes = await super.generateRoutes() + generateRoutes () { + const routes = super.generateRoutes() // In development we expose all compiled files for react-error-overlay's line show feature // We use unshift so that we're sure the routes is defined before Next's default routes routes.unshift({ - path: '/_next/development/:path*', + match: route('/_next/development/:path*'), fn: async (req, res, params) => { const p = join(this.distDir, ...(params.path || [])) await this.serveStatic(req, res, p) } }) - // Makes `next export` exportPathMap work in development mode. - // So that the user doesn't have to define a custom server reading the exportPathMap - if (this.nextConfig.exportPathMap) { - console.log('Defining routes from exportPathMap') - const exportPathMap = await this.nextConfig.exportPathMap({}, {dev: true, dir: this.dir, outDir: null, distDir: this.distDir, buildId: this.buildId}) // In development we can't give a default path mapping - for (const path in exportPathMap) { - const {page, query = {}} = exportPathMap[path] - - // We use unshift so that we're sure the routes is defined before Next's default routes - routes.unshift({ - path, - fn: async (req, res, params, parsedUrl) => { - const { query: urlQuery } = parsedUrl - - Object.keys(urlQuery) - .filter(key => query[key] === undefined) - .forEach(key => console.warn(`Url defines a query parameter '${key}' that is missing in exportPathMap`)) - - const mergedQuery = {...urlQuery, ...query} - - await this.render(req, res, page, mergedQuery, parsedUrl) - } - }) - } - } - return routes } diff --git a/packages/next/server/next-server.js b/packages/next/server/next-server.js index 04de276abd..f3e8553687 100644 --- a/packages/next/server/next-server.js +++ b/packages/next/server/next-server.js @@ -9,7 +9,7 @@ import { sendHTML, serveStatic } from './render' -import Router from './router' +import Router, {route} from './router' import { isInternalUrl } from './utils' import loadConfig from './config' import {PHASE_PRODUCTION_SERVER, BLOCKED_PAGES, BUILD_ID_FILE, CLIENT_STATIC_FILES_PATH, CLIENT_STATIC_FILES_RUNTIME} from '../lib/constants' @@ -21,7 +21,6 @@ export default class Server { constructor ({ dir = '.', staticMarkup = false, quiet = false, conf = null } = {}) { this.dir = resolve(dir) this.quiet = quiet - this.router = new Router() const phase = this.currentPhase() this.nextConfig = loadConfig(phase, this.dir, conf) this.distDir = join(this.dir, this.nextConfig.distDir) @@ -50,6 +49,8 @@ export default class Server { publicRuntimeConfig }) + const routes = this.generateRoutes() + this.router = new Router(routes) this.setAssetPrefix(assetPrefix) } @@ -86,9 +87,8 @@ export default class Server { asset.setAssetPrefix(this.renderOpts.assetPrefix) } - async prepare () { - await this.defineRoutes() - } + // Backwards compatibility + async prepare () {} // Backwards compatibility async close () {} @@ -97,10 +97,10 @@ export default class Server { res.setHeader('Cache-Control', 'public, max-age=31536000, immutable') } - async generateRoutes () { + generateRoutes () { const routes = [ { - path: '/_next/static/:path*', + match: route('/_next/static/:path*'), fn: async (req, res, params) => { // The commons folder holds commonschunk files // The chunks folder holds dynamic entries @@ -113,7 +113,7 @@ export default class Server { } }, { - path: '/_next/:path*', + match: route('/_next/:path*'), // This path is needed because `render()` does a check for `/_next` and the calls the routing again fn: async (req, res, params, parsedUrl) => { await this.render404(req, res, parsedUrl) @@ -124,7 +124,7 @@ export default class Server { // (but it should support as many as params, seperated by '/') // Othewise this will lead to a pretty simple DOS attack. // See more: https://github.com/zeit/next.js/issues/2617 - path: '/static/:path*', + match: route('/static/:path*'), fn: async (req, res, params) => { const p = join(this.dir, 'static', ...(params.path || [])) await this.serveStatic(req, res, p) @@ -138,7 +138,7 @@ export default class Server { // Othewise this will lead to a pretty simple DOS attack. // See more: https://github.com/zeit/next.js/issues/2617 routes.push({ - path: '/:path*', + match: route('/:path*'), fn: async (req, res, params, parsedUrl) => { const { pathname, query } = parsedUrl await this.render(req, res, pathname, query, parsedUrl) @@ -149,16 +149,6 @@ export default class Server { return routes } - async defineRoutes () { - const routes = await this.generateRoutes() - - for (const method of ['GET', 'HEAD']) { - for (const route of routes) { - this.router.add(method, route.path, route.fn) - } - } - } - async run (req, res, parsedUrl) { const fn = this.router.match(req, res, parsedUrl) if (fn) { diff --git a/packages/next/server/router.js b/packages/next/server/router.js index 50efd70eb5..90bab9a5b8 100644 --- a/packages/next/server/router.js +++ b/packages/next/server/router.js @@ -1,29 +1,26 @@ import pathMatch from './lib/path-match' -const route = pathMatch() +export const route = pathMatch() export default class Router { - constructor () { - this.routes = new Map() + constructor (routes = []) { + this.routes = routes } - add (method, path, fn) { - const routes = this.routes.get(method) || new Set() - routes.add({ match: route(path), fn }) - this.routes.set(method, routes) + add (route) { + this.routes.unshift(route) } match (req, res, parsedUrl) { - const routes = this.routes.get(req.method) - if (!routes) return + if (req.method !== 'GET' && req.method !== 'HEAD') { + return + } const { pathname } = parsedUrl - for (const r of routes) { - const params = r.match(pathname) + for (const route of this.routes) { + const params = route.match(pathname) if (params) { - return async () => { - return r.fn(req, res, params, parsedUrl) - } + return () => route.fn(req, res, params, parsedUrl) } } } diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js index 1b78a0d0b9..e1c31c12ce 100644 --- a/test/integration/production/test/index.test.js +++ b/test/integration/production/test/index.test.js @@ -61,6 +61,21 @@ describe('Production Usage', () => { expect(res.status).toBe(404) }) + it('should render 501 if the HTTP method is not GET or HEAD', async () => { + const url = `http://localhost:${appPort}/_next/abcdef` + const methods = ['POST', 'PUT', 'DELETE'] + for (const method of methods) { + const res = await fetch(url, {method}) + expect(res.status).toBe(501) + } + }) + + it('should set Content-Length header', async () => { + const url = `http://localhost:${appPort}` + const res = await fetch(url) + expect(res.headers.get('Content-Length')).toBeDefined() + }) + it('should set Cache-Control header', async () => { const buildId = readFileSync(join(__dirname, '../.next/BUILD_ID'), 'utf8') const buildManifest = require(join('../.next', BUILD_MANIFEST)) -- GitLab