Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement experimental pages/404.js for custom 404 page #10329

Merged
merged 13 commits into from Feb 1, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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'] = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this new change can't we just make this /404 and update @now/next?

Suggested change
defaultMap['/_errors/404'] = {
defaultMap['/404'] = {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until pages404 is no longer experimental we still have the potential conflict for pages/404.js. After it's no longer experimental we can rename this from /_errors/404 to /404 if desired

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay!

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'