From f324095c4150a56e3074bc78a0fdc579e45bb1b3 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 15 Jan 2020 14:05:40 -0600 Subject: [PATCH 1/4] Add initial support for static 404 page --- packages/next/build/index.ts | 20 +++- packages/next/next-server/server/config.ts | 1 + .../next/next-server/server/next-server.ts | 18 ++- test/integration/static-404/pages/index.js | 1 + .../integration/static-404/test/index.test.js | 105 ++++++++++++++++++ 5 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 test/integration/static-404/pages/index.js create mode 100644 test/integration/static-404/test/index.test.js diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 1018cf0f30bd223..43034ce85079b06 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -197,6 +197,10 @@ export default async function build(dir: string, conf = null): Promise { const pageKeys = Object.keys(mappedPages) const dynamicRoutes = pageKeys.filter(page => isDynamicRoute(page)) const conflictingPublicFiles: string[] = [] + const errorPageRegex = new RegExp( + `^(/|\\\\)_error\\.(?:${config.pageExtensions.join('|')})$` + ) + const hasCustomErrorPage = pagePaths.some(page => page.match(errorPageRegex)) if (hasPublicDir) { try { @@ -521,6 +525,11 @@ export default async function build(dir: string, conf = null): Promise { ) staticCheckWorkers.end() + const useStatic404 = + !customAppGetInitialProps && + !hasCustomErrorPage && + config.experimental.static404 + if (invalidPages.size > 0) { throw new Error( `Build optimization failed: found page${ @@ -551,7 +560,7 @@ export default async function build(dir: string, conf = null): Promise { const finalPrerenderRoutes: { [route: string]: SprRoute } = {} const tbdPrerenderRoutes: string[] = [] - if (staticPages.size > 0 || sprPages.size > 0) { + if (staticPages.size > 0 || sprPages.size > 0 || useStatic404) { const combinedPages = [...staticPages, ...sprPages] const exportApp = require('../export').default const exportOptions = { @@ -588,6 +597,11 @@ export default async function build(dir: string, conf = null): Promise { defaultMap[route] = { page } }) }) + + if (useStatic404) { + defaultMap['/_errors/404'] = { page: '/_error' } + } + return defaultMap }, exportTrailingSlash: false, @@ -628,6 +642,10 @@ export default async function build(dir: string, conf = null): Promise { await fsMove(orig, dest) } + if (useStatic404) { + await moveExportedPage('/_errors/404', '/_errors/404', false, 'html') + } + for (const page of combinedPages) { const isSpr = sprPages.has(page) const isDynamic = isDynamicRoute(page) diff --git a/packages/next/next-server/server/config.ts b/packages/next/next-server/server/config.ts index 83669d5f879ed49..819e60cfdbb159e 100644 --- a/packages/next/next-server/server/config.ts +++ b/packages/next/next-server/server/config.ts @@ -51,6 +51,7 @@ const defaultConfig: { [key: string]: any } = { reactMode: 'legacy', workerThreads: false, basePath: '', + static404: false, }, future: { excludeDefaultMomentLocales: false, diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 9a7ccf14802352c..a8c250f7ca542d1 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -1049,7 +1049,23 @@ export default class Server { _pathname: string, query: ParsedUrlQuery = {} ) { - const result = await this.findPageComponents('/_error', query) + let result: null | LoadComponentsReturnType = null + + // use static 404 page if available and is 404 response + if (this.nextConfig.experimental.static404 && res.statusCode === 404) { + try { + result = await this.findPageComponents('/_errors/404') + } catch (err) { + if (err.code !== 'ENOENT') { + throw err + } + } + } + + if (!result) { + result = await this.findPageComponents('/_error', query) + } + let html try { html = await this.renderToHTMLWithComponents( diff --git a/test/integration/static-404/pages/index.js b/test/integration/static-404/pages/index.js new file mode 100644 index 000000000000000..0957a987fc2f227 --- /dev/null +++ b/test/integration/static-404/pages/index.js @@ -0,0 +1 @@ +export default () => 'hi' diff --git a/test/integration/static-404/test/index.test.js b/test/integration/static-404/test/index.test.js new file mode 100644 index 000000000000000..5e466ce9c808079 --- /dev/null +++ b/test/integration/static-404/test/index.test.js @@ -0,0 +1,105 @@ +/* eslint-env jest */ +/* global jasmine */ +import fs from 'fs-extra' +import { join } from 'path' +import { + renderViaHTTP, + findPort, + nextBuild, + nextStart, + killApp, +} from 'next-test-utils' + +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 appPage = join(appDir, 'pages/_app.js') +const errorPage = join(appDir, 'pages/_error.js') +const buildId = `generateBuildId: () => 'test-id'` +const experimentalConfig = `experimental: { static404: true }` +let app +let appPort + +describe('Static 404 page', () => { + afterEach(async () => { + await fs.remove(appPage) + await fs.remove(errorPage) + await fs.remove(nextConfig) + }) + 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} }`) + await nextBuild(appDir) + expect(await fs.exists(static404)).toBe(false) + }) + }) + + describe('With config enabled', () => { + beforeEach(() => + fs.writeFile( + nextConfig, + `module.exports = { ${buildId}, ${experimentalConfig} }` + ) + ) + + it('should export 404 page without custom _error', async () => { + await nextBuild(appDir) + appPort = await findPort() + app = await nextStart(appDir, appPort) + const html = await renderViaHTTP(appPort, '/non-existent') + await killApp(app) + expect(html).toContain('This page could not be found') + expect(await fs.exists(static404)).toBe(true) + }) + + it('should export 404 page without custom _error (serverless)', async () => { + await fs.writeFile( + nextConfig, + ` + module.exports = { + target: 'experimental-serverless-trace', + experimental: { static404: true } + } + ` + ) + await nextBuild(appDir) + appPort = await findPort() + app = await nextStart(appDir, appPort) + const html = await renderViaHTTP(appPort, '/non-existent') + await killApp(app) + expect(html).toContain('This page could not be found') + expect( + await fs.exists(join(appDir, '.next/serverless/pages/_errors/404.html')) + ).toBe(true) + }) + + it('should not export 404 page with custom _error', async () => { + await fs.writeFile(errorPage, `export { default } from 'next/error'`) + await nextBuild(appDir) + await fs.remove(errorPage) + expect(await fs.exists(static404)).toBe(false) + }) + + it('should not export 404 page with getInitialProps in _app', async () => { + await fs.writeFile( + appPage, + ` + const Page = ({ Component, pageProps }) => { + return + } + Page.getInitialProps = () => ({ hello: 'world', pageProps: {} }) + export default Page + ` + ) + await nextBuild(appDir) + await fs.remove(appPage) + expect(await fs.exists(static404)).toBe(false) + }) + }) +}) From e6243e98b932e018236b4faca152de50a3775de2 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 16 Jan 2020 11:07:57 -0600 Subject: [PATCH 2/4] Apply suggestions from code review Co-Authored-By: Tim Neutkens --- packages/next/next-server/server/next-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index a8c250f7ca542d1..80e162e6bfa8ae6 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -1052,7 +1052,7 @@ export default class Server { let result: null | LoadComponentsReturnType = null // use static 404 page if available and is 404 response - if (this.nextConfig.experimental.static404 && res.statusCode === 404) { + if (this.nextConfig.experimental.static404 && err === null) { try { result = await this.findPageComponents('/_errors/404') } catch (err) { From 946aa27bdd540ce2bf70b0681559042ad7a5c7b8 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 16 Jan 2020 11:08:35 -0600 Subject: [PATCH 3/4] Simplify custom error page check --- packages/next/build/index.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 526bb0dc8680a06..ba82febcc5c84b2 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -197,10 +197,9 @@ export default async function build(dir: string, conf = null): Promise { const pageKeys = Object.keys(mappedPages) const dynamicRoutes = pageKeys.filter(page => isDynamicRoute(page)) const conflictingPublicFiles: string[] = [] - const errorPageRegex = new RegExp( - `^(/|\\\\)_error\\.(?:${config.pageExtensions.join('|')})$` + const hasCustomErrorPage = mappedPages['/_error'].startsWith( + 'private-next-pages' ) - const hasCustomErrorPage = pagePaths.some(page => page.match(errorPageRegex)) if (hasPublicDir) { try { From f81d2a65d08ac1fbd1899645d9292220824815af Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 20 Jan 2020 15:07:55 +0100 Subject: [PATCH 4/4] Add comment explaining reason for custom app check --- packages/next/build/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index ba82febcc5c84b2..00d34f8cd25d8d5 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -524,6 +524,8 @@ export default async function build(dir: string, conf = null): Promise { ) staticCheckWorkers.end() + // 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 &&