Skip to content

Commit

Permalink
Add support for static 404 when _error does not have custom GIP (#11062)
Browse files Browse the repository at this point in the history
* Add support for static 404 when _error does not have custom GIP

* Update tests
  • Loading branch information
ijjk committed Mar 14, 2020
1 parent e6b4cdd commit 71f9719
Show file tree
Hide file tree
Showing 9 changed files with 195 additions and 25 deletions.
30 changes: 23 additions & 7 deletions packages/next/build/index.ts
Expand Up @@ -64,7 +64,7 @@ import createSpinner from './spinner'
import {
collectPages,
getPageSizeInKb,
hasCustomAppGetInitialProps,
hasCustomGetInitialProps,
isPageStatic,
PageInfo,
printCustomRoutes,
Expand Down Expand Up @@ -229,6 +229,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
const hasPages404 = Boolean(
mappedPages['/404'] && mappedPages['/404'].startsWith('private-next-pages')
)
let hasNonStaticErrorPage: boolean

if (hasPublicDir) {
try {
Expand Down Expand Up @@ -456,6 +457,24 @@ export default async function build(dir: string, conf = null): Promise<void> {
staticCheckWorkers.getStdout().pipe(process.stdout)
staticCheckWorkers.getStderr().pipe(process.stderr)

const runtimeEnvConfig = {
publicRuntimeConfig: config.publicRuntimeConfig,
serverRuntimeConfig: config.serverRuntimeConfig,
}

hasNonStaticErrorPage =
hasCustomErrorPage &&
(await hasCustomGetInitialProps(
path.join(
distDir,
...(isLikeServerless
? ['serverless', 'pages']
: ['server', 'static', buildId, 'pages']),
'_error.js'
),
runtimeEnvConfig
))

const analysisBegin = process.hrtime()
await Promise.all(
pageKeys.map(async page => {
Expand Down Expand Up @@ -485,14 +504,10 @@ export default async function build(dir: string, conf = null): Promise<void> {

pagesManifest[page] = bundleRelative.replace(/\\/g, '/')

const runtimeEnvConfig = {
publicRuntimeConfig: config.publicRuntimeConfig,
serverRuntimeConfig: config.serverRuntimeConfig,
}
const nonReservedPage = !page.match(/^\/(_app|_error|_document|api)/)

if (nonReservedPage && customAppGetInitialProps === undefined) {
customAppGetInitialProps = hasCustomAppGetInitialProps(
customAppGetInitialProps = hasCustomGetInitialProps(
isLikeServerless
? serverBundle
: path.join(
Expand Down Expand Up @@ -618,7 +633,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 || hasPages404)
!customAppGetInitialProps && (!hasNonStaticErrorPage || hasPages404)

if (invalidPages.size > 0) {
throw new Error(
Expand Down Expand Up @@ -907,6 +922,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
distPath: distDir,
buildId: buildId,
pagesDir,
useStatic404,
pageExtensions: config.pageExtensions,
buildManifest,
isModern: config.experimental.modern,
Expand Down
18 changes: 14 additions & 4 deletions packages/next/build/utils.ts
Expand Up @@ -57,13 +57,15 @@ export async function printTreeView(
pageExtensions,
buildManifest,
isModern,
useStatic404,
}: {
distPath: string
buildId: string
pagesDir: string
pageExtensions: string[]
buildManifest: BuildManifestShape
isModern: boolean
useStatic404: boolean
}
) {
const getPrettySize = (_size: number): string => {
Expand All @@ -87,6 +89,14 @@ export async function printTreeView(
const hasCustomApp = await findPageFile(pagesDir, '/_app', pageExtensions)
const hasCustomError = await findPageFile(pagesDir, '/_error', pageExtensions)

if (useStatic404) {
pageInfos.set('/404', {
...(pageInfos.get('/404') || pageInfos.get('/_error')),
static: true,
} as any)
list = [...list, '/404']
}

const pageList = list
.slice()
.filter(
Expand Down Expand Up @@ -720,14 +730,14 @@ export async function isPageStatic(
}
}

export function hasCustomAppGetInitialProps(
_appBundle: string,
export function hasCustomGetInitialProps(
bundle: string,
runtimeEnvConfig: any
): boolean {
require('../next-server/lib/runtime-config').setConfig(runtimeEnvConfig)
let mod = require(_appBundle)
let mod = require(bundle)

if (_appBundle.endsWith('_app.js')) {
if (bundle.endsWith('_app.js') || bundle.endsWith('_error.js')) {
mod = mod.default || mod
} else {
// since we don't output _app in serverless mode get it from a page
Expand Down
19 changes: 11 additions & 8 deletions packages/next/pages/_error.tsx
Expand Up @@ -14,20 +14,23 @@ export type ErrorProps = {
title?: string
}

function _getInitialProps({
res,
err,
}: NextPageContext): Promise<ErrorProps> | ErrorProps {
const statusCode =
res && res.statusCode ? res.statusCode : err ? err.statusCode! : 404
return { statusCode }
}

/**
* `Error` component used for handling errors.
*/
export default class Error<P = {}> extends React.Component<P & ErrorProps> {
static displayName = 'ErrorPage'

static getInitialProps({
res,
err,
}: NextPageContext): Promise<ErrorProps> | ErrorProps {
const statusCode =
res && res.statusCode ? res.statusCode : err ? err.statusCode! : 404
return { statusCode }
}
static getInitialProps = _getInitialProps
static origGetInitialProps = _getInitialProps

render() {
const { statusCode } = this.props
Expand Down
File renamed without changes.
5 changes: 5 additions & 0 deletions test/integration/404-page-custom-error/pages/err.js
@@ -0,0 +1,5 @@
const page = () => 'err page'
page.getInitialProps = () => {
throw new Error('oops')
}
export default page
1 change: 1 addition & 0 deletions test/integration/404-page-custom-error/pages/index.js
@@ -0,0 +1 @@
export default () => 'hello from index'
127 changes: 127 additions & 0 deletions test/integration/404-page-custom-error/test/index.test.js
@@ -0,0 +1,127 @@
/* eslint-env jest */
/* global jasmine */
import fs from 'fs-extra'
import { join } from 'path'
import {
killApp,
findPort,
launchApp,
nextStart,
nextBuild,
renderViaHTTP,
fetchViaHTTP,
} from 'next-test-utils'

jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2

const appDir = join(__dirname, '../')
const nextConfig = join(appDir, 'next.config.js')

let appPort
let buildId
let app

const runTests = mode => {
const isDev = mode === 'dev'

it('should respond to 404 correctly', async () => {
const res = await fetchViaHTTP(appPort, '/404')
expect(res.status).toBe(404)
expect(await res.text()).toContain('This page could not be found')
})

it('should render error correctly', async () => {
const text = await renderViaHTTP(appPort, '/err')
expect(text).toContain(isDev ? 'oops' : 'Internal Server Error')
})

it('should render index page normal', async () => {
const html = await renderViaHTTP(appPort, '/')
expect(html).toContain('hello from index')
})

if (!isDev) {
it('should set pages404 in routes-manifest correctly', async () => {
const data = await fs.readJSON(join(appDir, '.next/routes-manifest.json'))
expect(data.pages404).toBe(true)
})

it('should have output 404.html', async () => {
expect(
await fs
.access(
join(
appDir,
'.next',
...(mode === 'server'
? ['server', 'static', buildId, 'pages']
: ['serverless', 'pages']),
'404.html'
)
)
.then(() => true)
.catch(() => false)
)
})
}
}

describe('Default 404 Page with custom _error', () => {
describe('server mode', () => {
afterAll(() => killApp(app))

it('should build successfully', async () => {
const { code } = await nextBuild(appDir, [], {
stderr: true,
stdout: true,
})

expect(code).toBe(0)

appPort = await findPort()

app = await nextStart(appDir, appPort)
buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8')
})

runTests('server')
})

describe('serverless mode', () => {
afterAll(async () => {
await fs.remove(nextConfig)
await killApp(app)
})

it('should build successfully', async () => {
await fs.writeFile(
nextConfig,
`
module.exports = { target: 'experimental-serverless-trace' }
`
)
const { code } = await nextBuild(appDir, [], {
stderr: true,
stdout: true,
})

expect(code).toBe(0)

appPort = await findPort()
app = await nextStart(appDir, appPort)
buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8')
})

runTests('serverless')
})

describe('dev mode', () => {
beforeAll(async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort)
})
afterAll(() => killApp(app))

runTests('dev')
})
})
4 changes: 0 additions & 4 deletions test/integration/static-404/next.config.js

This file was deleted.

16 changes: 14 additions & 2 deletions test/integration/static-404/test/index.test.js
Expand Up @@ -63,8 +63,20 @@ describe('Static 404 page', () => {
).toBe(true)
})

it('should not export 404 page with custom _error', async () => {
await fs.writeFile(errorPage, `export { default } from 'next/error'`)
it('should not export 404 page with custom _error GIP', async () => {
await fs.writeFile(
errorPage,
`
import Error from 'next/error'
export default class MyError extends Error {
static getInitialProps() {
return {
statusCode: 404
}
}
}
`
)
await nextBuild(appDir)
await fs.remove(errorPage)
expect(await fs.exists(static404)).toBe(false)
Expand Down

0 comments on commit 71f9719

Please sign in to comment.