Make exportPathMap / _next routes work with useFileSystemPublicRoutes disabled (#5131)

* Add test for /_next/development route

* Make sure useFileSystemPublicRoute: false only disables filesystem routing

* Bring back comment

* Add useFileSystemPublicRoutes tests

server/index.js

@@ -150,41 +150,42 @@ export default class Server {
       }
     }
 
-    if (this.nextConfig.useFileSystemPublicRoutes) {
-      // 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.dev && 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]
-          routes[path] = 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)
-          }
-        }
+    // In development we expose all compiled files for react-error-overlay's line show feature
+    if (this.dev) {
+      routes['/_next/development/:path*'] = async (req, res, params) => {
+        const p = join(this.distDir, ...(params.path || []))
+        console.log('page', p)
+        await this.serveStatic(req, res, p)
       }
+    }
 
-      // In development we expose all compiled files for react-error-overlay's line show feature
-      if (this.dev) {
-        routes['/_next/development/:path*'] = async (req, res, params) => {
-          const p = join(this.distDir, ...(params.path || []))
-          await this.serveStatic(req, res, p)
-        }
-      }
+    // This path is needed because `render()` does a check for `/_next` and the calls the routing again
+    routes['/_next/:path*'] = async (req, res, params, parsedUrl) => {
+      await this.render404(req, res, parsedUrl)
+    }
+
+    // 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.dev && 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]
+        routes[path] = 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`))
 
-      // This path is needed because `render()` does a check for `/_next` and the calls the routing again
-      routes['/_next/:path*'] = async (req, res, params, parsedUrl) => {
-        await this.render404(req, res, parsedUrl)
+          const mergedQuery = {...urlQuery, ...query}
+
+          await this.render(req, res, page, mergedQuery, parsedUrl)
+        }
       }
+    }
 
+    if (this.nextConfig.useFileSystemPublicRoutes) {
       // It's very important keep this route's param optional.
       // (but it should support as many as params, seperated by '/')
       // Othewise this will lead to a pretty simple DOS attack.

test/integration/basic/test/rendering.js

@@ -128,6 +128,19 @@ export default function ({ app }, suiteName, render, fetch, appPort) {
       expect(res.status).toBe(404)
     })
 
+    test('should expose the compiled page file in development', async () => {
+      await fetch('/stateless') // make sure the stateless page is built
+      const clientSideJsRes = await fetch('/_next/development/static/development/pages/stateless.js')
+      expect(clientSideJsRes.status).toBe(200)
+      const clientSideJsBody = await clientSideJsRes.text()
+      expect(clientSideJsBody).toMatch(/My component!/)
+
+      const serverSideJsRes = await fetch('/_next/development/server/static/development/pages/stateless.js')
+      expect(serverSideJsRes.status).toBe(200)
+      const serverSideJsBody = await serverSideJsRes.text()
+      expect(serverSideJsBody).toMatch(/My component!/)
+    })
+
     test('allows to import .json files', async () => {
       const html = await render('/json')
       expect(html.includes('Zeit')).toBeTruthy()

test/integration/filesystempublicroutes/next.config.js

+module.exports = {
+  onDemandEntries: {
+    // Make sure entries are not getting disposed.
+    maxInactiveAge: 1000 * 60 * 60
+  },
+  useFileSystemPublicRoutes: false,
+  exportPathMap () {
+    return {
+      '/exportpathmap-route': {page: '/exportpathmap-route'}
+    }
+  }
+}

test/integration/filesystempublicroutes/pages/exportpathmap-route.js

+export default () => <div>exportpathmap was here</div>

test/integration/filesystempublicroutes/pages/index.js

+export default () => <div>Index</div>

test/integration/filesystempublicroutes/server.js

+const micro = require('micro')
+const next = require('next')
+
+const dev = process.env.NODE_ENV !== 'production'
+const dir = __dirname
+const port = process.env.PORT || 3000
+
+const app = next({ dev, dir })
+const handleNextRequests = app.getRequestHandler()
+
+app.prepare().then(() => {
+  const server = micro((req, res) => {
+    if (/setAssetPrefix/.test(req.url)) {
+      app.setAssetPrefix(`http://127.0.0.1:${port}`)
+    } else if (/setEmptyAssetPrefix/.test(req.url)) {
+      app.setAssetPrefix(null)
+    } else {
+      // This is to support multi-zones support in localhost
+      // and may be in staging deployments
+      app.setAssetPrefix('')
+    }
+
+    handleNextRequests(req, res)
+  })
+
+  server.listen(port, (err) => {
+    if (err) {
+      throw err
+    }
+
+    console.log(`> Ready on http://localhost:${port}`)
+  })
+})

test/integration/filesystempublicroutes/test/index.test.js

+/* global jasmine, describe, it, expect, beforeAll, afterAll */
+
+import { join } from 'path'
+import getPort from 'get-port'
+import {
+  fetchViaHTTP,
+  initNextServerScript,
+  killApp
+} from 'next-test-utils'
+import clone from 'clone'
+
+const appDir = join(__dirname, '../')
+let appPort
+let server
+jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2
+
+const context = {}
+
+const startServer = async (optEnv = {}) => {
+  const scriptPath = join(appDir, 'server.js')
+  context.appPort = appPort = await getPort()
+  const env = Object.assign(
+    {},
+    clone(process.env),
+    { PORT: `${appPort}` },
+    optEnv
+  )
+
+  server = await initNextServerScript(scriptPath, /Ready on/, env)
+}
+
+describe('FileSystemPublicRoutes', () => {
+  beforeAll(() => startServer())
+  afterAll(() => killApp(server))
+
+  const fetch = (p, q) => fetchViaHTTP(context.appPort, p, q)
+
+  it('should not route to the index page', async () => {
+    const res = await fetch('/')
+    expect(res.status).toBe(404)
+    const body = await res.text()
+    expect(body).toMatch(/404/)
+  })
+
+  it('should route to exportPathMap defined routes in development', async () => {
+    const res = await fetch('/exportpathmap-route')
+    expect(res.status).toBe(200)
+    const body = await res.text()
+    expect(body).toMatch(/exportpathmap was here/)
+  })
+
+  it('should still handle /_next routes', async () => {
+    await fetch('/exportpathmap-route') // make sure it's built
+    const res = await fetch('/_next/static/development/pages/exportpathmap-route.js')
+    expect(res.status).toBe(200)
+    const body = await res.text()
+    expect(body).toMatch(/exportpathmap was here/)
+  })
+})

test/integration/production/test/index.test.js

@@ -253,6 +253,20 @@ describe('Production Usage', () => {
     })
   })
 
+  it('should not expose the compiled page file in development', async () => {
+    const url = `http://localhost:${appPort}`
+    await fetch(`${url}/stateless`) // make sure the stateless page is built
+    const clientSideJsRes = await fetch(`${url}/_next/development/static/development/pages/stateless.js`)
+    expect(clientSideJsRes.status).toBe(404)
+    const clientSideJsBody = await clientSideJsRes.text()
+    expect(clientSideJsBody).toMatch(/404/)
+
+    const serverSideJsRes = await fetch(`${url}/_next/development/server/static/development/pages/stateless.js`)
+    expect(serverSideJsRes.status).toBe(404)
+    const serverSideJsBody = await serverSideJsRes.text()
+    expect(serverSideJsBody).toMatch(/404/)
+  })
+
   dynamicImportTests(context, (p, q) => renderViaHTTP(context.appPort, p, q))
 
   security(context)