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

@@ -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
   }
 

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) {

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)
       }
     }
   }

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))