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,7 +150,20 @@ export default class Server {
       }
     }
 
-    if (this.nextConfig.useFileSystemPublicRoutes) {
+    // 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)
+      }
+    }
+
+    // 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) {
@@ -172,19 +185,7 @@ export default class Server {
       }
     }
 
-      // In development we expose all compiled files for react-error-overlay's line show feature
+    if (this.nextConfig.useFileSystemPublicRoutes) {
-      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)
-      }
-
       // 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

@@ -0,0 +1,12 @@
+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

@@ -0,0 +1 @@
+export default () => <div>exportpathmap was here</div>

test/integration/filesystempublicroutes/pages/index.js

@@ -0,0 +1 @@
+export default () => <div>Index</div>

test/integration/filesystempublicroutes/server.js

@@ -0,0 +1,33 @@
+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

@@ -0,0 +1,59 @@
+/* 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)