Skip to content

Commit

Permalink
Update /500 page exporting when _error has custom GIP (vercel#22887)
Browse files Browse the repository at this point in the history 
This updates to not automatically export `/500` from `_error` if a custom `getInitialProps` is used since logic may be used inside of this method that causes the export to fail. Users can still opt-in to the static `/500` by adding a `pages/500.js` file. 

This also refactors checking `_app` for custom `getInitialProps` to outside of the static check loop to prevent a potential race condition where we could run this check multiple times un-necessarily.  

Fixes: vercel#22815
  • Loading branch information
@ijjk
ijjk authored Mar 9, 2021
1 parent 635a98e commit d951b23
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 50 deletions.
78 changes: 37 additions & 41 deletions packages/next/build/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -605,11 +605,43 @@ export default async function build(
async () =>
hasCustomErrorPage &&
(await hasCustomGetInitialProps(
getPagePath('/_error', distDir, isLikeServerless),
'/_error',
distDir,
isLikeServerless,
runtimeEnvConfig,
false
))
)
// we don't output _app in serverless mode so use _app export
// from _error instead
const appPageToCheck = isLikeServerless ? '/_error' : '/_app'

customAppGetInitialProps = await hasCustomGetInitialProps(
appPageToCheck,
distDir,
isLikeServerless,
runtimeEnvConfig,
true
)

namedExports = await getNamedExports(
appPageToCheck,
distDir,
isLikeServerless,
runtimeEnvConfig
)

if (customAppGetInitialProps) {
console.warn(
chalk.bold.yellow(`Warning: `) +
chalk.yellow(
`You have opted-out of Automatic Static Optimization due to \`getInitialProps\` in \`pages/_app\`. This does not opt-out pages with \`getStaticProps\``
)
)
console.warn(
'Read more: https://err.sh/next.js/opt-out-auto-static-optimization\n'
)
}

await Promise.all(
pageKeys.map(async (page) => {
Expand All @@ -633,41 +665,6 @@ export default async function build(
)

if (nonReservedPage) {
const serverBundle = getPagePath(
page,
distDir,
isLikeServerless
)

if (customAppGetInitialProps === undefined) {
customAppGetInitialProps = await hasCustomGetInitialProps(
isLikeServerless
? serverBundle
: getPagePath('/_app', distDir, isLikeServerless),
runtimeEnvConfig,
true
)

namedExports = getNamedExports(
isLikeServerless
? serverBundle
: getPagePath('/_app', distDir, isLikeServerless),
runtimeEnvConfig
)

if (customAppGetInitialProps) {
console.warn(
chalk.bold.yellow(`Warning: `) +
chalk.yellow(
`You have opted-out of Automatic Static Optimization due to \`getInitialProps\` in \`pages/_app\`. This does not opt-out pages with \`getStaticProps\``
)
)
console.warn(
'Read more: https://err.sh/next.js/opt-out-auto-static-optimization\n'
)
}
}

try {
let workerResult = await traceAsyncFn(
tracer.startSpan('is-page-static'),
Expand Down Expand Up @@ -917,6 +914,7 @@ export default async function build(
})

const hasPages500 = usedStaticStatusPages.includes('/500')
const useDefaultStatic500 = !hasPages500 && !hasNonStaticErrorPage

await traceAsyncFn(tracer.startSpan('static-generation'), async () => {
const combinedPages = [...staticPages, ...ssgPages]
Expand Down Expand Up @@ -994,9 +992,7 @@ export default async function build(
}
}

// ensure 500.html is always generated even if pages/500.html
// doesn't exist
if (!hasPages500) {
if (useDefaultStatic500) {
defaultMap['/500'] = {
page: '/_error',
}
Expand All @@ -1007,7 +1003,7 @@ export default async function build(
...staticPages,
...ssgPages,
...(useStatic404 ? ['/404'] : []),
...(!hasPages500 ? ['/500'] : []),
...(useDefaultStatic500 ? ['/500'] : []),
]) {
const isSsg = ssgPages.has(page)
const isDynamic = isDynamicRoute(page)
Expand Down Expand Up @@ -1175,7 +1171,7 @@ export default async function build(
await moveExportedPage('/_error', '/404', '/404', false, 'html')
}

if (!hasPages500) {
if (useDefaultStatic500) {
await moveExportedPage('/_error', '/500', '/500', false, 'html')
}

Expand Down
21 changes: 15 additions & 6 deletions packages/next/build/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -860,12 +860,16 @@ export async function isPageStatic(
}

export async function hasCustomGetInitialProps(
bundle: string,
page: string,
distDir: string,
isLikeServerless: boolean,
runtimeEnvConfig: any,
checkingApp: boolean
): Promise<boolean> {
require('../next-server/lib/runtime-config').setConfig(runtimeEnvConfig)
let mod = require(bundle)

const components = await loadComponents(distDir, page, isLikeServerless)
let mod = components.ComponentMod

if (checkingApp) {
mod = (await mod._app) || mod.default || mod
Expand All @@ -876,12 +880,17 @@ export async function hasCustomGetInitialProps(
return mod.getInitialProps !== mod.origGetInitialProps
}

export function getNamedExports(
bundle: string,
export async function getNamedExports(
page: string,
distDir: string,
isLikeServerless: boolean,
runtimeEnvConfig: any
): Array<string> {
): Promise<Array<string>> {
require('../next-server/lib/runtime-config').setConfig(runtimeEnvConfig)
return Object.keys(require(bundle))
const components = await loadComponents(distDir, page, isLikeServerless)
let mod = components.ComponentMod

return Object.keys(mod)
}

export function detectConflictingPaths(
Expand Down
48 changes: 45 additions & 3 deletions test/integration/500-page/test/index.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const runTests = (mode = 'server') => {
describe('500 Page Support', () => {
describe('dev mode', () => {
beforeAll(async () => {
await fs.remove(join(appDir, '.next'))
appPort = await findPort()
app = await launchApp(appDir, appPort)
})
Expand All @@ -76,6 +77,7 @@ describe('500 Page Support', () => {

describe('server mode', () => {
beforeAll(async () => {
await fs.remove(join(appDir, '.next'))
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
Expand All @@ -96,6 +98,7 @@ describe('500 Page Support', () => {
}
`
)
await fs.remove(join(appDir, '.next'))
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
Expand All @@ -119,6 +122,7 @@ describe('500 Page Support', () => {
export default page
`
)
await fs.remove(join(appDir, '.next'))
const { stderr, stdout: buildStdout, code } = await nextBuild(appDir, [], {
stderr: true,
stdout: true,
Expand Down Expand Up @@ -148,6 +152,7 @@ describe('500 Page Support', () => {

it('builds 500 statically by default with no pages/500', async () => {
await fs.rename(pages500, `${pages500}.bak`)
await fs.remove(join(appDir, '.next'))
const { stderr, code } = await nextBuild(appDir, [], { stderr: true })
await fs.rename(`${pages500}.bak`, pages500)

Expand Down Expand Up @@ -188,23 +193,57 @@ describe('500 Page Support', () => {
}
})

it('builds 500 statically by default with no pages/500 and custom _error', async () => {
it('builds 500 statically by default with no pages/500 and custom _error without getInitialProps', async () => {
await fs.rename(pages500, `${pages500}.bak`)
await fs.writeFile(
pagesError,
`
function Error({ statusCode }) {
return <p>Error status: {statusCode}</p>
}
Error.getInitialProps = ({ res, err }) => {
export default Error
`
)
await fs.remove(join(appDir, '.next'))
const { stderr: buildStderr, code } = await nextBuild(appDir, [], {
stderr: true,
})
await fs.rename(`${pages500}.bak`, pages500)
await fs.remove(pagesError)
console.log(buildStderr)
expect(buildStderr).not.toMatch(gip500Err)
expect(code).toBe(0)
expect(
await fs.pathExists(join(appDir, '.next/server/pages/500.html'))
).toBe(true)
})

it('does not build 500 statically with no pages/500 and custom getInitialProps in _error', async () => {
await fs.rename(pages500, `${pages500}.bak`)
await fs.writeFile(
pagesError,
`
function Error({ statusCode }) {
return <p>Error status: {statusCode}</p>
}
Error.getInitialProps = ({ req, res, err }) => {
console.error('called _error.getInitialProps')
if (req.url === '/500') {
throw new Error('should not export /500')
}
return {
statusCode: res && res.statusCode ? res.statusCode : err ? err.statusCode : 404
}
}
export default Error
`
)
await fs.remove(join(appDir, '.next'))
const { stderr: buildStderr, code } = await nextBuild(appDir, [], {
stderr: true,
})
Expand All @@ -215,7 +254,7 @@ describe('500 Page Support', () => {
expect(code).toBe(0)
expect(
await fs.pathExists(join(appDir, '.next/server/pages/500.html'))
).toBe(true)
).toBe(false)

let appStderr = ''
const appPort = await findPort()
Expand All @@ -241,6 +280,7 @@ describe('500 Page Support', () => {
export default page
`
)
await fs.remove(join(appDir, '.next'))
const { stderr, code } = await nextBuild(appDir, [], { stderr: true })
await fs.remove(pages500)
await fs.move(`${pages500}.bak`, pages500)
Expand Down Expand Up @@ -288,6 +328,7 @@ describe('500 Page Support', () => {
export default page
`
)
await fs.remove(join(appDir, '.next'))
const { stderr, code } = await nextBuild(appDir, [], { stderr: true })
await fs.remove(pages500)
await fs.move(`${pages500}.bak`, pages500)
Expand Down Expand Up @@ -335,6 +376,7 @@ describe('500 Page Support', () => {
export default page
`
)
await fs.remove(join(appDir, '.next'))
const { stderr, code } = await nextBuild(appDir, [], { stderr: true })
await fs.remove(pages500)
await fs.move(`${pages500}.bak`, pages500)
Expand Down

0 comments on commit d951b23

Please sign in to comment.