Merge branch 'canary' of github.com:zeit/next.js into add/reliable-page-loading

timneutkens · timneutkens · commit b1a299e84eb2 · 2018-09-11T12:54:18.000+02:00
diff --git a/build/babel/preset.js b/build/babel/preset.js
@@ -33,7 +33,7 @@ module.exports = (context, opts = {}) => ({
     [require('@babel/preset-env').default, {
       // In the test environment `modules` is often needed to be set to true, babel figures that out by itself using the `'auto'` option
       // In production/development this option is set to `false` so that webpack can handle import/export with tree-shaking
-      modules: isDevelopment && isProduction ? false : 'auto',
+      modules: isDevelopment || isProduction ? false : 'auto',
       ...opts['preset-env']
     }],
     [require('@babel/preset-react'), {
diff --git a/build/webpack.js b/build/webpack.js
@@ -113,7 +113,7 @@ export default async function getBaseWebpackConfig (dir: string, {dev = false, i
   const defaultLoaders = {
     babel: {
       loader: 'next-babel-loader',
-      options: {dev, isServer}
+      options: {dev, isServer, cwd: dir}
     },
     hotSelfAccept: {
       loader: 'hot-self-accept-loader',
diff --git a/examples/with-webpack-bundle-analyzer/next.config.js b/examples/with-webpack-bundle-analyzer/next.config.js
@@ -1,9 +1,10 @@
-const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer')
 const { ANALYZE } = process.env
 
 module.exports = {
   webpack: function (config, { isServer }) {
     if (ANALYZE) {
+      const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer')
+
       config.plugins.push(new BundleAnalyzerPlugin({
         analyzerMode: 'server',
         analyzerPort: isServer ? 8888 : 8889,
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "next",
-  "version": "7.0.0-canary.13",
+  "version": "7.0.0-canary.15",
   "description": "Minimalistic framework for server-rendered React applications",
   "main": "./dist/server/next.js",
   "license": "MIT",
@@ -75,7 +75,7 @@
     "ansi-html": "0.0.7",
     "autodll-webpack-plugin": "0.4.2",
     "babel-core": "7.0.0-bridge.0",
-    "babel-loader": "8.0.0",
+    "babel-loader": "8.0.2",
     "babel-plugin-react-require": "3.0.0",
     "babel-plugin-transform-react-remove-prop-types": "0.4.15",
     "case-sensitive-paths-webpack-plugin": "2.1.2",
@@ -87,7 +87,7 @@
     "fresh": "0.5.2",
     "friendly-errors-webpack-plugin": "1.7.0",
     "glob": "7.1.2",
-    "hoist-non-react-statics": "2.5.0",
+    "hoist-non-react-statics": "2.5.5",
     "htmlescape": "1.1.1",
     "http-errors": "1.6.2",
     "http-status": "1.0.1",
diff --git a/server/index.js b/server/index.js
@@ -150,36 +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`))
+
+          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.
diff --git a/test/integration/basic/test/rendering.js b/test/integration/basic/test/rendering.js
@@ -2,7 +2,7 @@
 
 import cheerio from 'cheerio'
 
-export default function ({ app }, suiteName, render, fetch) {
+export default function ({ app }, suiteName, render, fetch, appPort) {
   async function get$ (path, query) {
     const html = await render(path, query)
     return cheerio.load(html)
@@ -123,6 +123,24 @@ export default function ({ app }, suiteName, render, fetch) {
       expect(res.headers.get('Content-Type')).toMatch('text/html; charset=iso-8859-2')
     })
 
+    test('should render 404 for _next routes that do not exist', async () => {
+      const res = await fetch('/_next/abcdef')
+      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()
diff --git a/test/integration/filesystempublicroutes/next.config.js b/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'}
+    }
+  }
+}
diff --git a/test/integration/filesystempublicroutes/pages/exportpathmap-route.js b/test/integration/filesystempublicroutes/pages/exportpathmap-route.js
@@ -0,0 +1 @@
+export default () => <div>exportpathmap was here</div>
diff --git a/test/integration/filesystempublicroutes/pages/index.js b/test/integration/filesystempublicroutes/pages/index.js
@@ -0,0 +1 @@
+export default () => <div>Index</div>
diff --git a/test/integration/filesystempublicroutes/server.js b/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}`)
+  })
+})
diff --git a/test/integration/filesystempublicroutes/test/index.test.js b/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/)
+  })
+})
diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js
@@ -54,6 +54,12 @@ describe('Production Usage', () => {
       expect(res2.status).toBe(304)
     })
 
+    it('should render 404 for _next routes that do not exist', async () => {
+      const url = `http://localhost:${appPort}/_next/abcdef`
+      const res = await fetch(url)
+      expect(res.status).toBe(404)
+    })
+
     it('should set Cache-Control header', async () => {
       const buildId = readFileSync(join(__dirname, '../.next/BUILD_ID'), 'utf8')
       const buildManifest = require(join('../.next', BUILD_MANIFEST))
@@ -247,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)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+export default () => <div>exportpathmap was here</div>`