Skip to content

Commit

Permalink
Implement experimental pages/404.js for custom 404 page (#10329)
Browse files Browse the repository at this point in the history
* Implement experimental pages/404.js for custom 404 page

* Make sure to show error for getInitialProps in pages/404 in dev mode also

* Update routes-manifest tests for new value

* Make sure page404 is boolean in routes-manifest

* Rename variables for consistency

* Make sure to only use 404 page for 404 error
  • Loading branch information
ijjk committed Feb 1, 2020
1 parent 1674f2d commit db90ffe
Show file tree
Hide file tree
Showing 14 changed files with 340 additions and 11 deletions.
15 changes: 15 additions & 0 deletions errors/404-get-initial-props.md
@@ -0,0 +1,15 @@
# 404.js Cannot Have getInitialProps

#### Why This Error Occurred

In your `404.js` page you added `getInitialProps` or `getServerProps` which is not allowed as this prevents the page from being static and `404.js` is meant to provide more flexibility for a static 404 page. Having a static 404 page is the most ideal as this page can be served very often and if not static puts extra strain on your server and more invocations for serverless functions which increase the cost of hosting your site unnecessarily.

#### Possible Ways to Fix It

Remove `getInitialProps` from `404.js` and make sure no HOC's used in `404.js` attach `getInitialProps`.

If you want to fetch data for your `404.js` page move it to a client side fetch inside of `componentDidMount` or `useEffect(() => {}, [])`.

### Useful Links

- [Automatic Static Optimization](https://nextjs.org/docs/advanced-features/automatic-static-optimization)
28 changes: 24 additions & 4 deletions packages/next/build/index.ts
Expand Up @@ -16,7 +16,10 @@ import checkCustomRoutes, {
Rewrite,
Header,
} from '../lib/check-custom-routes'
import { PUBLIC_DIR_MIDDLEWARE_CONFLICT } from '../lib/constants'
import {
PUBLIC_DIR_MIDDLEWARE_CONFLICT,
PAGES_404_GET_INITIAL_PROPS_ERROR,
} from '../lib/constants'
import { findPagesDir } from '../lib/find-pages-dir'
import { recursiveDelete } from '../lib/recursive-delete'
import { recursiveReadDir } from '../lib/recursive-readdir'
Expand Down Expand Up @@ -202,6 +205,10 @@ 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')

if (hasPublicDir) {
try {
Expand Down Expand Up @@ -262,6 +269,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,
basePath: config.experimental.basePath,
redirects: redirects.map(r => buildCustomRoute(r, 'redirect')),
rewrites: rewrites.map(r => buildCustomRoute(r, 'rewrite')),
Expand Down Expand Up @@ -511,6 +519,17 @@ export default async function build(dir: string, conf = null): Promise<void> {
staticPages.add(page)
isStatic = true
}

if (hasPages404 && page === '/404') {
if (!result.isStatic) {
throw new Error(PAGES_404_GET_INITIAL_PROPS_ERROR)
}
// we need to ensure the 404 lambda is present since we use
// it when _app has getInitialProps
if (customAppGetInitialProps) {
staticPages.delete(page)
}
}
} catch (err) {
if (err.message !== 'INVALID_DEFAULT_EXPORT') throw err
invalidPages.add(page)
Expand Down Expand Up @@ -569,8 +588,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
// Only export the static 404 when there is no /_error present
const useStatic404 =
!customAppGetInitialProps &&
!hasCustomErrorPage &&
config.experimental.static404
((!hasCustomErrorPage && config.experimental.static404) || hasPages404)

if (invalidPages.size > 0) {
throw new Error(
Expand Down Expand Up @@ -640,7 +658,9 @@ export default async function build(dir: string, conf = null): Promise<void> {
})

if (useStatic404) {
defaultMap['/_errors/404'] = { page: '/_error' }
defaultMap['/_errors/404'] = {
page: hasPages404 ? '/404' : '/_error',
}
}

return defaultMap
Expand Down
2 changes: 2 additions & 0 deletions packages/next/lib/constants.ts
Expand Up @@ -29,3 +29,5 @@ export const SSG_GET_INITIAL_PROPS_CONFLICT = `You can not use getInitialProps w
export const SERVER_PROPS_GET_INIT_PROPS_CONFLICT = `You can not use getInitialProps with unstable_getServerProps. Please remove one or the other`

export const SERVER_PROPS_SSG_CONFLICT = `You can not use unstable_getStaticProps with unstable_getServerProps. To use SSG, please remove your unstable_getServerProps`

export const PAGES_404_GET_INITIAL_PROPS_ERROR = `\`pages/404\` can not have getInitialProps/getServerProps, https://err.sh/zeit/next.js/404-get-initial-props`
1 change: 1 addition & 0 deletions packages/next/next-server/server/config.ts
Expand Up @@ -53,6 +53,7 @@ const defaultConfig: { [key: string]: any } = {
workerThreads: false,
basePath: '',
static404: true,
pages404: false,
},
future: {
excludeDefaultMomentLocales: false,
Expand Down
40 changes: 33 additions & 7 deletions packages/next/next-server/server/next-server.ts
Expand Up @@ -101,6 +101,7 @@ export default class Server {
documentMiddlewareEnabled: boolean
hasCssMode: boolean
dev?: boolean
pages404?: boolean
}
private compression?: Middleware
private onErrorMiddleware?: ({ err }: { err: Error }) => Promise<void>
Expand Down Expand Up @@ -148,6 +149,7 @@ export default class Server {
staticMarkup,
buildId: this.buildId,
generateEtags,
pages404: this.nextConfig.experimental.pages404,
}

// Only the `publicRuntimeConfig` key is exposed to the client side
Expand Down Expand Up @@ -857,6 +859,11 @@ export default class Server {
result: LoadComponentsReturnType,
opts: any
): Promise<string | null> {
// we need to ensure the status code if /404 is visited directly
if (this.nextConfig.experimental.pages404 && pathname === '/404') {
res.statusCode = 404
}

// handle static page
if (typeof result.Component === 'string') {
return result.Component
Expand Down Expand Up @@ -1115,13 +1122,32 @@ export default class Server {
) {
let result: null | LoadComponentsReturnType = 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 (this.nextConfig.experimental.static404 && res.statusCode === 404) {
try {
result = await this.findPageComponents('/_errors/404')
} catch (err) {
if (err.code !== 'ENOENT') {
throw err
if (is404) {
if (static404) {
try {
result = await this.findPageComponents('/_errors/404')
} catch (err) {
if (err.code !== 'ENOENT') {
throw err
}
}
}

// use 404 if /_errors/404 isn't available which occurs
// during development and when _app has getInitialProps
if (!result && pages404) {
try {
result = await this.findPageComponents('/404')
using404Page = true
} catch (err) {
if (err.code !== 'ENOENT') {
throw err
}
}
}
}
Expand All @@ -1135,7 +1161,7 @@ export default class Server {
html = await this.renderToHTMLWithComponents(
req,
res,
'/_error',
using404Page ? '/404' : '/_error',
query,
result,
{
Expand Down
7 changes: 7 additions & 0 deletions packages/next/next-server/server/render.tsx
Expand Up @@ -28,6 +28,7 @@ import {
SSG_GET_INITIAL_PROPS_CONFLICT,
SERVER_PROPS_GET_INIT_PROPS_CONFLICT,
SERVER_PROPS_SSG_CONFLICT,
PAGES_404_GET_INITIAL_PROPS_ERROR,
} from '../../lib/constants'
import { AMP_RENDER_TARGET } from '../lib/constants'
import { LoadComponentsReturnType, ManifestItem } from './load-components'
Expand Down Expand Up @@ -135,6 +136,7 @@ type RenderOpts = LoadComponentsReturnType & {
documentMiddlewareEnabled?: boolean
isDataReq?: boolean
params?: ParsedUrlQuery
pages404?: boolean
}

function renderDocument(
Expand Down Expand Up @@ -263,6 +265,7 @@ export async function renderToHTML(
unstable_getServerProps,
isDataReq,
params,
pages404,
} = renderOpts

const callMiddleware = async (method: string, args: any[], props = false) => {
Expand Down Expand Up @@ -363,6 +366,10 @@ export async function renderToHTML(
req.url = pathname
renderOpts.nextExport = true
}

if (pages404 && pathname === '/404' && !isAutoExport) {
throw new Error(PAGES_404_GET_INITIAL_PROPS_ERROR)
}
}
if (isAutoExport) renderOpts.autoExport = true
if (isSpr) renderOpts.nextExport = false
Expand Down
10 changes: 10 additions & 0 deletions packages/next/server/next-dev-server.ts
Expand Up @@ -432,6 +432,16 @@ 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
}
}
}

res.statusCode = 404
return this.renderErrorToHTML(null, req, res, pathname, query)
}
Expand Down
5 changes: 5 additions & 0 deletions test/integration/404-page/next.config.js
@@ -0,0 +1,5 @@
module.exports = {
experimental: {
pages404: true,
},
}
2 changes: 2 additions & 0 deletions test/integration/404-page/pages/404.js
@@ -0,0 +1,2 @@
const page = () => 'custom 404 page'
export default page
5 changes: 5 additions & 0 deletions test/integration/404-page/pages/err.js
@@ -0,0 +1,5 @@
const page = () => 'custom 404 page'
page.getInitialProps = () => {
throw new Error('oops')
}
export default page
1 change: 1 addition & 0 deletions test/integration/404-page/pages/index.js
@@ -0,0 +1 @@
export default () => 'hello from index'

0 comments on commit db90ffe

Please sign in to comment.