Clean up landed experimental flags (#10593)

* Clean up landed experimental flags * Remove check for experimental flags from build too * Remove /_errors/404 in favor of /404 * Remove unneeded check for pathname * Update test paths * Fix test * Update test * Remove test for disabled config * Set pages404 always to true Co-authored-by: Joe Haddad <timer150@gmail.com> Co-authored-by: JJ Kasper <jj@jjsweb.site>
vercel · Feb 19, 2020 · 9a0d82b · 9a0d82b
1 parent 1c64c1e
commit 9a0d82b
Show file tree

Hide file tree

Showing 10 changed files with 32 additions and 74 deletions.
diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts
@@ -221,10 +221,9 @@ export default async function build(dir: string, conf = null): Promise<void> {
   const hasCustomErrorPage = mappedPages['/_error'].startsWith(
     'private-next-pages'
   )
-  const hasPages404 =
-    config.experimental.pages404 &&
-    mappedPages['/404'] &&
-    mappedPages['/404'].startsWith('private-next-pages')
+  const hasPages404 = Boolean(
+    mappedPages['/404'] && mappedPages['/404'].startsWith('private-next-pages')
+  )
 
   if (hasPublicDir) {
     try {
@@ -285,7 +284,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
   const routesManifestPath = path.join(distDir, ROUTES_MANIFEST)
   const routesManifest: any = {
     version: 1,
-    pages404: !!hasPages404,
+    pages404: true,
     basePath: config.experimental.basePath,
     redirects: redirects.map(r => buildCustomRoute(r, 'redirect')),
     rewrites: rewrites.map(r => buildCustomRoute(r, 'rewrite')),
@@ -606,8 +605,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
   // Since custom _app.js can wrap the 404 page we have to opt-out of static optimization if it has getInitialProps
   // Only export the static 404 when there is no /_error present
   const useStatic404 =
-    !customAppGetInitialProps &&
-    ((!hasCustomErrorPage && config.experimental.static404) || hasPages404)
+    !customAppGetInitialProps && (!hasCustomErrorPage || hasPages404)
 
   if (invalidPages.size > 0) {
     throw new Error(
@@ -681,7 +679,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
         })
 
         if (useStatic404) {
-          defaultMap['/_errors/404'] = {
+          defaultMap['/404'] = {
             page: hasPages404 ? '/404' : '/_error',
           }
         }
@@ -690,6 +688,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
       },
       exportTrailingSlash: false,
     }
+
     await exportApp(dir, exportOptions, exportConfig)
 
     // remove server bundles that were exported
@@ -726,8 +725,9 @@ export default async function build(dir: string, conf = null): Promise<void> {
       await fsMove(orig, dest)
     }
 
-    if (useStatic404) {
-      await moveExportedPage('/_errors/404', '/_errors/404', false, 'html')
+    // Only move /404 to /404 when there is no custom 404 as in that case we don't know about the 404 page
+    if (!hasPages404 && useStatic404) {
+      await moveExportedPage('/404', '/404', false, 'html')
     }
 
     for (const page of combinedPages) {

diff --git a/packages/next/next-server/server/config.ts b/packages/next/next-server/server/config.ts
@@ -52,8 +52,6 @@ const defaultConfig: { [key: string]: any } = {
     reactMode: 'legacy',
     workerThreads: false,
     basePath: '',
-    static404: true,
-    pages404: true,
   },
   future: {
     excludeDefaultMomentLocales: false,

diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts
@@ -114,7 +114,6 @@ export default class Server {
     documentMiddlewareEnabled: boolean
     hasCssMode: boolean
     dev?: boolean
-    pages404?: boolean
   }
   private compression?: Middleware
   private onErrorMiddleware?: ({ err }: { err: Error }) => Promise<void>
@@ -162,7 +161,6 @@ export default class Server {
       staticMarkup,
       buildId: this.buildId,
       generateEtags,
-      pages404: this.nextConfig.experimental.pages404,
     }
 
     // Only the `publicRuntimeConfig` key is exposed to the client side
@@ -866,7 +864,7 @@ export default class Server {
     opts: any
   ): Promise<string | null> {
     // we need to ensure the status code if /404 is visited directly
-    if (this.nextConfig.experimental.pages404 && pathname === '/404') {
+    if (pathname === '/404') {
       res.statusCode = 404
     }
 
@@ -1171,22 +1169,13 @@ export default class Server {
   ) {
     let result: null | FindComponentsResult = null
 
-    const { static404, pages404 } = this.nextConfig.experimental
     const is404 = res.statusCode === 404
     let using404Page = false
 
     // use static 404 page if available and is 404 response
     if (is404) {
-      if (static404) {
-        result = await this.findPageComponents('/_errors/404')
-      }
-
-      // use 404 if /_errors/404 isn't available which occurs
-      // during development and when _app has getInitialProps
-      if (!result && pages404) {
-        result = await this.findPageComponents('/404')
-        using404Page = result !== null
-      }
+      result = await this.findPageComponents('/404')
+      using404Page = result !== null
     }
 
     if (!result) {
@@ -1220,11 +1209,8 @@ export default class Server {
   ): Promise<void> {
     const url: any = req.url
     const { pathname, query } = parsedUrl ? parsedUrl : parseUrl(url, true)
-    if (!pathname) {
-      throw new Error('pathname is undefined')
-    }
     res.statusCode = 404
-    return this.renderError(null, req, res, pathname, query)
+    return this.renderError(null, req, res, pathname!, query)
   }
 
   public async serveStatic(

diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx
@@ -145,7 +145,6 @@ type RenderOpts = LoadComponentsReturnType & {
   documentMiddlewareEnabled?: boolean
   isDataReq?: boolean
   params?: ParsedUrlQuery
-  pages404?: boolean
   previewProps: __ApiPreviewProps
 }
 
@@ -278,7 +277,6 @@ export async function renderToHTML(
     unstable_getServerProps,
     isDataReq,
     params,
-    pages404,
     previewProps,
   } = renderOpts
 
@@ -391,7 +389,7 @@ export async function renderToHTML(
       renderOpts.nextExport = true
     }
 
-    if (pages404 && pathname === '/404' && !isAutoExport) {
+    if (pathname === '/404' && !isAutoExport) {
       throw new Error(PAGES_404_GET_INITIAL_PROPS_ERROR)
     }
   }

diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts
@@ -463,13 +463,11 @@ export default class DevServer extends Server {
       })
     } catch (err) {
       if (err.code === 'ENOENT') {
-        if (this.nextConfig.experimental.pages404) {
-          try {
-            await this.hotReloader!.ensurePage('/404')
-          } catch (err) {
-            if (err.code !== 'ENOENT') {
-              throw err
-            }
+        try {
+          await this.hotReloader!.ensurePage('/404')
+        } catch (err) {
+          if (err.code !== 'ENOENT') {
+            throw err
           }
         }
 

diff --git a/test/integration/404-page/next.config.js b/test/integration/404-page/next.config.js
@@ -1,5 +1 @@
-module.exports = {
-  experimental: {
-    pages404: true,
-  },
-}
+module.exports = {}
diff --git a/test/integration/404-page/test/index.test.js b/test/integration/404-page/test/index.test.js
@@ -49,7 +49,7 @@ const runTests = (mode = 'server') => {
   })
 
   if (mode !== 'dev') {
-    it('should output _errors/404.html during build', async () => {
+    it('should output 404.html during build', async () => {
       expect(
         await fs.exists(
           join(
@@ -58,17 +58,17 @@ const runTests = (mode = 'server') => {
             mode === 'serverless'
               ? 'serverless/pages'
               : `server/static/${buildId}/pages`,
-            '_errors/404.html'
+            '404.html'
           )
         )
       ).toBe(true)
     })
 
-    it('should add _errors/404 to pages-manifest correctly', async () => {
+    it('should add /404 to pages-manifest correctly', async () => {
       const manifest = await fs.readJSON(
         join(appDir, '.next', mode, 'pages-manifest.json')
       )
-      expect('/_errors/404' in manifest).toBe(true)
+      expect('/404' in manifest).toBe(true)
     })
   }
 }
@@ -104,10 +104,7 @@ describe('404 Page Support', () => {
         nextConfig,
         `
         module.exports = {
-          target: 'serverless',
-          experimental: {
-            pages404: true
-          }
+          target: 'serverless'
         }
       `
       )
@@ -212,7 +209,7 @@ describe('404 Page Support', () => {
     it('should not output static 404 if _app has getInitialProps', async () => {
       expect(
         await fs.exists(
-          join(appDir, '.next/server/static', buildId, 'pages/_errors/404.html')
+          join(appDir, '.next/server/static', buildId, 'pages/404.html')
         )
       ).toBe(false)
     })

diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js
@@ -344,7 +344,7 @@ const runTests = (isDev = false) => {
 
       expect(manifest).toEqual({
         version: 1,
-        pages404: false,
+        pages404: true,
         basePath: '',
         redirects: [
           {

diff --git a/test/integration/dynamic-routing/test/index.test.js b/test/integration/dynamic-routing/test/index.test.js
@@ -487,7 +487,7 @@ function runTests(dev) {
 
       expect(manifest).toEqual({
         version: 1,
-        pages404: false,
+        pages404: true,
         basePath: '',
         headers: [],
         rewrites: [],

diff --git a/test/integration/static-404/test/index.test.js b/test/integration/static-404/test/index.test.js
@@ -13,10 +13,7 @@ import {
 jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2
 const appDir = join(__dirname, '..')
 const nextConfig = join(appDir, 'next.config.js')
-const static404 = join(
-  appDir,
-  '.next/server/static/test-id/pages/_errors/404.html'
-)
+const static404 = join(appDir, '.next/server/static/test-id/pages/404.html')
 const appPage = join(appDir, 'pages/_app.js')
 const errorPage = join(appDir, 'pages/_error.js')
 const buildId = `generateBuildId: () => 'test-id'`
@@ -31,17 +28,6 @@ describe('Static 404 page', () => {
   })
   beforeEach(() => fs.remove(join(appDir, '.next/server')))
 
-  describe('With config disabled', () => {
-    it('should not have exported static 404 page', async () => {
-      await fs.writeFile(
-        nextConfig,
-        `module.exports = { ${buildId}, experimental: { static404: false } }`
-      )
-      await nextBuild(appDir)
-      expect(await fs.exists(static404)).toBe(false)
-    })
-  })
-
   describe('With config enabled', () => {
     beforeEach(() =>
       fs.writeFile(nextConfig, `module.exports = { ${buildId} }`)
@@ -62,8 +48,7 @@ describe('Static 404 page', () => {
         nextConfig,
         `
         module.exports = {
-          target: 'experimental-serverless-trace',
-          experimental: { static404: true }
+          target: 'experimental-serverless-trace'
         }
       `
       )
@@ -74,7 +59,7 @@ describe('Static 404 page', () => {
       await killApp(app)
       expect(html).toContain('This page could not be found')
       expect(
-        await fs.exists(join(appDir, '.next/serverless/pages/_errors/404.html'))
+        await fs.exists(join(appDir, '.next/serverless/pages/404.html'))
       ).toBe(true)
     })