Make page require faster (#3776)

* Make page require faster

* Add windows search/replace

* Use normalize instead of resolve

* Add remaining tests

* Use sep instead of /

* Add test files

* Make component require faster

* Add console.error

server/render.js

@@ -4,7 +4,7 @@ import { renderToString, renderToStaticMarkup } from 'react-dom/server'
 import send from 'send'
 import generateETag from 'etag'
 import fresh from 'fresh'
-import requireModule from './require'
+import requirePage from './require'
 import getConfig from './config'
 import { Router } from '../lib/router'
 import { loadGetInitialProps, isResSent } from '../lib/utils'
@@ -48,17 +48,16 @@ async function doRender (req, res, pathname, query, {
 } = {}) {
   page = page || pathname
 
-  await ensurePage(page, { dir, hotReloader })
+  if (hotReloader) { // In dev mode we use on demand entries to compile the page before rendering
+    await ensurePage(page, { dir, hotReloader })
+  }
 
   const dist = getConfig(dir).distDir
 
-  const pagePath = join(dir, dist, 'dist', 'bundles', 'pages', page)
   const documentPath = join(dir, dist, 'dist', 'bundles', 'pages', '_document')
 
-  let [Component, Document] = await Promise.all([
-    requireModule(pagePath),
-    requireModule(documentPath)
-  ])
+  let Component = requirePage(page, {dir, dist})
+  let Document = require(documentPath)
   Component = Component.default || Component
   Document = Document.default || Document
   const asPath = req.url
@@ -225,7 +224,6 @@ export function serveStatic (req, res, path) {
 }
 
 async function ensurePage (page, { dir, hotReloader }) {
-  if (!hotReloader) return
   if (page === '_error' || page === '_document') return
 
   await hotReloader.ensurePage(page)

server/require.js

@@ -1,6 +1,64 @@
-import resolve from './resolve'
+import {join, parse, normalize, sep} from 'path'
 
-export default async function requireModule (path) {
-  const f = await resolve(path)
-  return require(f)
+export function pageNotFoundError (page) {
+  const err = new Error(`Cannot find module for page: ${page}`)
+  err.code = 'ENOENT'
+  return err
+}
+
+export function normalizePagePath (page) {
+  // If the page is `/` we need to append `/index`, otherwise the returned directory root will be bundles instead of pages
+  if (page === '/') {
+    page = '/index'
+  }
+
+  // Resolve on anything that doesn't start with `/`
+  if (page[0] !== '/') {
+    page = `/${page}`
+  }
+
+  // Windows compatibility
+  if (sep !== '/') {
+    page = page.replace(/\//g, sep)
+  }
+
+  // Throw when using ../ etc in the pathname
+  const resolvedPage = normalize(page)
+  if (page !== resolvedPage) {
+    throw new Error('Requested and resolved page mismatch')
+  }
+
+  return page
+}
+
+export function getPagePath (page, {dir, dist}) {
+  const pageBundlesPath = join(dir, dist, 'dist', 'bundles', 'pages')
+
+  try {
+    page = normalizePagePath(page)
+  } catch (err) {
+    console.error(err)
+    throw pageNotFoundError(page)
+  }
+
+  const pagePath = join(pageBundlesPath, page) // Path to the page that is to be loaded
+
+  // Don't allow wandering outside of the bundles directory
+  const pathDir = parse(pagePath).dir
+  if (pathDir.indexOf(pageBundlesPath) !== 0) {
+    console.error('Resolved page path goes outside of bundles path')
+    throw pageNotFoundError(page)
+  }
+
+  return pagePath
+}
+
+export default function requirePage (page, {dir, dist}) {
+  const pagePath = getPagePath(page, {dir, dist})
+  try {
+    return require(pagePath)
+  } catch (err) {
+    console.error(err)
+    throw pageNotFoundError(page)
+  }
 }

test/isolated/_resolvedata/.gitignore

@@ -0,0 +1 @@
+!dist

test/isolated/_resolvedata/dist/bundles/pages/index.js

@@ -0,0 +1,3 @@
+module.exports = {
+  test: 'hello'
+}

test/isolated/_resolvedata/dist/bundles/pages/world.js

@@ -0,0 +1,3 @@
+module.exports = {
+  test: 'world'
+}

test/isolated/require-page.test.js

@@ -0,0 +1,82 @@
+/* global describe, it, expect */
+
+import { join, sep } from 'path'
+import requirePage, {getPagePath, normalizePagePath, pageNotFoundError} from '../../dist/server/require'
+
+const dir = '/path/to/some/project'
+const dist = '.next'
+
+const pathToBundles = join(dir, dist, 'dist', 'bundles', 'pages')
+
+describe('pageNotFoundError', () => {
+  it('Should throw error with ENOENT code', () => {
+    try {
+      pageNotFoundError('test')
+    } catch (err) {
+      expect(err.code).toBe('ENOENT')
+    }
+  })
+})
+
+describe('normalizePagePath', () => {
+  it('Should turn / into /index', () => {
+    expect(normalizePagePath('/')).toBe(`${sep}index`)
+  })
+
+  it('Should turn _error into /_error', () => {
+    expect(normalizePagePath('_error')).toBe(`${sep}_error`)
+  })
+
+  it('Should turn /abc into /abc', () => {
+    expect(normalizePagePath('/abc')).toBe(`${sep}abc`)
+  })
+
+  it('Should turn /abc/def into /abc/def', () => {
+    expect(normalizePagePath('/abc/def')).toBe(`${sep}abc${sep}def`)
+  })
+
+  it('Should throw on /../../test.js', () => {
+    expect(() => normalizePagePath('/../../test.js')).toThrow()
+  })
+})
+
+describe('getPagePath', () => {
+  it('Should append /index to the / page', () => {
+    const pagePath = getPagePath('/', {dir, dist})
+    expect(pagePath).toBe(join(pathToBundles, `${sep}index`))
+  })
+
+  it('Should prepend / when a page does not have it', () => {
+    const pagePath = getPagePath('_error', {dir, dist})
+    expect(pagePath).toBe(join(pathToBundles, `${sep}_error`))
+  })
+
+  it('Should throw with paths containing ../', () => {
+    expect(() => getPagePath('/../../package.json', {dir, dist})).toThrow()
+  })
+})
+
+describe('requirePage', () => {
+  it('Should require /index.js when using /', () => {
+    const page = requirePage('/', {dir: __dirname, dist: '_resolvedata'})
+    expect(page.test).toBe('hello')
+  })
+
+  it('Should require /index.js when using /index', () => {
+    const page = requirePage('/index', {dir: __dirname, dist: '_resolvedata'})
+    expect(page.test).toBe('hello')
+  })
+
+  it('Should require /world.js when using /world', () => {
+    const page = requirePage('/world', {dir: __dirname, dist: '_resolvedata'})
+    expect(page.test).toBe('world')
+  })
+
+  it('Should throw when using /../../test.js', () => {
+    expect(() => requirePage('/../../test.js', {dir: __dirname, dist: '_resolvedata'})).toThrow()
+  })
+
+  it('Should throw when using non existent pages like /non-existent.js', () => {
+    expect(() => requirePage('/non-existent.js', {dir: __dirname, dist: '_resolvedata'})).toThrow()
+  })
+})