From b3e279121e6f720387b17a2ae8ecff615453a10c Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 11 Mar 2020 11:18:05 -0500 Subject: [PATCH] Make sure to not show pages/404 GIP error from _app having GIP (#10974) * Make sure to not show pages/404 GIP error from _app having GIP * Add error for getStaticProps in pages/404 too --- packages/next/lib/constants.ts | 2 +- packages/next/next-server/server/render.tsx | 5 +- test/integration/404-page/test/index.test.js | 214 +++++++++++++++---- 3 files changed, 175 insertions(+), 46 deletions(-) diff --git a/packages/next/lib/constants.ts b/packages/next/lib/constants.ts index e8025b25e9ec28b..ae74148ce8256c9 100644 --- a/packages/next/lib/constants.ts +++ b/packages/next/lib/constants.ts @@ -30,4 +30,4 @@ export const SERVER_PROPS_GET_INIT_PROPS_CONFLICT = `You can not use getInitialP export const SERVER_PROPS_SSG_CONFLICT = `You can not use getStaticProps with getServerSideProps. To use SSG, please remove getServerSideProps` -export const PAGES_404_GET_INITIAL_PROPS_ERROR = `\`pages/404\` can not have getInitialProps/getServerSideProps, https://err.sh/zeit/next.js/404-get-initial-props` +export const PAGES_404_GET_INITIAL_PROPS_ERROR = `\`pages/404\` can not have getInitialProps/getServerSideProps/getStaticProps, https://err.sh/zeit/next.js/404-get-initial-props` diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index f36786bfcf78586..7e5e85d2b636671 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -418,7 +418,10 @@ export async function renderToHTML( renderOpts.nextExport = true } - if (pathname === '/404' && !isAutoExport) { + if ( + pathname === '/404' && + (hasPageGetInitialProps || getServerSideProps || isSSG) + ) { throw new Error(PAGES_404_GET_INITIAL_PROPS_ERROR) } } diff --git a/test/integration/404-page/test/index.test.js b/test/integration/404-page/test/index.test.js index e6c28cee3871ac8..6c481628d24aefc 100644 --- a/test/integration/404-page/test/index.test.js +++ b/test/integration/404-page/test/index.test.js @@ -19,6 +19,7 @@ const appDir = join(__dirname, '../') const pages404 = join(appDir, 'pages/404.js') const appPage = join(appDir, 'pages/_app.js') const nextConfig = join(appDir, 'next.config.js') +const gip404Err = /`pages\/404` can not have getInitialProps\/getServerSideProps/ let nextConfigContent let buildId @@ -49,7 +50,7 @@ const runTests = (mode = 'server') => { }) if (mode !== 'dev') { - it('should output _errors/404.html during build', async () => { + it('should output 404.html during build', async () => { expect( await fs.exists( join( @@ -58,17 +59,17 @@ const runTests = (mode = 'server') => { mode === 'serverless' ? 'serverless/pages' : `server/static/${buildId}/pages`, - '_errors/404.html' + '404.html' ) ) ).toBe(true) }) - it('should add _errors/404 to pages-manifest correctly', async () => { + it('should add /404 to pages-manifest correctly', async () => { const manifest = await fs.readJSON( join(appDir, '.next', mode, 'pages-manifest.json') ) - expect('/_errors/404' in manifest).toBe(true) + expect('/404' in manifest).toBe(true) }) } } @@ -104,10 +105,7 @@ describe('404 Page Support', () => { nextConfig, ` module.exports = { - target: 'serverless', - experimental: { - pages404: true - } + target: 'serverless' } ` ) @@ -151,9 +149,7 @@ describe('404 Page Support', () => { await fs.remove(pages404) await fs.move(`${pages404}.bak`, pages404) - expect(stderr).toContain( - `\`pages/404\` can not have getInitialProps/getServerProps, https://err.sh/zeit/next.js/404-get-initial-props` - ) + expect(stderr).toMatch(gip404Err) expect(code).toBe(1) }) @@ -183,51 +179,181 @@ describe('404 Page Support', () => { await fs.remove(pages404) await fs.move(`${pages404}.bak`, pages404) - const error = `\`pages/404\` can not have getInitialProps/getServerProps, https://err.sh/zeit/next.js/404-get-initial-props` + expect(stderr).toMatch(gip404Err) + }) + + it('shows error with getStaticProps in pages/404 build', async () => { + await fs.move(pages404, `${pages404}.bak`) + await fs.writeFile( + pages404, + ` + const page = () => 'custom 404 page' + export const getStaticProps = () => ({ props: { a: 'b' } }) + export default page + ` + ) + const { stderr, code } = await nextBuild(appDir, [], { stderr: true }) + await fs.remove(pages404) + await fs.move(`${pages404}.bak`, pages404) + + expect(stderr).toMatch(gip404Err) + expect(code).toBe(1) + }) + + it('shows error with getStaticProps in pages/404 dev', async () => { + await fs.move(pages404, `${pages404}.bak`) + await fs.writeFile( + pages404, + ` + const page = () => 'custom 404 page' + export const getStaticProps = () => ({ props: { a: 'b' } }) + export default page + ` + ) + + let stderr = '' + appPort = await findPort() + app = await launchApp(appDir, appPort, { + onStderr(msg) { + stderr += msg || '' + }, + }) + await renderViaHTTP(appPort, '/abc') + await waitFor(1000) + + await killApp(app) + + await fs.remove(pages404) + await fs.move(`${pages404}.bak`, pages404) + + expect(stderr).toMatch(gip404Err) + }) + + it('shows error with getServerSideProps in pages/404 build', async () => { + await fs.move(pages404, `${pages404}.bak`) + await fs.writeFile( + pages404, + ` + const page = () => 'custom 404 page' + export const getServerSideProps = () => ({ props: { a: 'b' } }) + export default page + ` + ) + const { stderr, code } = await nextBuild(appDir, [], { stderr: true }) + await fs.remove(pages404) + await fs.move(`${pages404}.bak`, pages404) + + expect(stderr).toMatch(gip404Err) + expect(code).toBe(1) + }) + + it('shows error with getServerSideProps in pages/404 dev', async () => { + await fs.move(pages404, `${pages404}.bak`) + await fs.writeFile( + pages404, + ` + const page = () => 'custom 404 page' + export const getServerSideProps = () => ({ props: { a: 'b' } }) + export default page + ` + ) + + let stderr = '' + appPort = await findPort() + app = await launchApp(appDir, appPort, { + onStderr(msg) { + stderr += msg || '' + }, + }) + await renderViaHTTP(appPort, '/abc') + await waitFor(1000) + + await killApp(app) + + await fs.remove(pages404) + await fs.move(`${pages404}.bak`, pages404) - expect(stderr).toContain(error) + expect(stderr).toMatch(gip404Err) }) describe('_app with getInitialProps', () => { - beforeAll(async () => { - await fs.writeFile( + beforeAll(() => + fs.writeFile( appPage, ` - import NextApp from 'next/app' - const App = ({ Component, pageProps }) => - App.getInitialProps = NextApp.getInitialProps - export default App - ` + import NextApp from 'next/app' + const App = ({ Component, pageProps }) => + App.getInitialProps = NextApp.getInitialProps + export default App + ` ) - await nextBuild(appDir) - appPort = await findPort() - app = await nextStart(appDir, appPort) - buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8') - }) - afterAll(async () => { - await fs.remove(appPage) - await killApp(app) - }) + ) + afterAll(() => fs.remove(appPage)) - it('should not output static 404 if _app has getInitialProps', async () => { - expect( - await fs.exists( - join(appDir, '.next/server/static', buildId, 'pages/_errors/404.html') + describe('production mode', () => { + afterAll(() => killApp(app)) + + it('should build successfully', async () => { + const { code, stderr, stdout } = await nextBuild(appDir, [], { + stderr: true, + stdout: true, + }) + + expect(code).toBe(0) + expect(stderr).not.toMatch(gip404Err) + expect(stdout).not.toMatch(gip404Err) + + appPort = await findPort() + app = await nextStart(appDir, appPort) + buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8') + }) + + it('should not output static 404 if _app has getInitialProps', async () => { + expect( + await fs.exists( + join(appDir, '.next/server/static', buildId, 'pages/404.html') + ) + ).toBe(false) + }) + + it('specify to use the 404 page still in the routes-manifest', async () => { + const manifest = await fs.readJSON( + join(appDir, '.next/routes-manifest.json') ) - ).toBe(false) - }) + expect(manifest.pages404).toBe(true) + }) - it('specify to use the 404 page still in the routes-manifest', async () => { - const manifest = await fs.readJSON( - join(appDir, '.next/routes-manifest.json') - ) - expect(manifest.pages404).toBe(true) + it('should still use 404 page', async () => { + const res = await fetchViaHTTP(appPort, '/abc') + expect(res.status).toBe(404) + expect(await res.text()).toContain('custom 404 page') + }) }) - it('should still use 404 page', async () => { - const res = await fetchViaHTTP(appPort, '/abc') - expect(res.status).toBe(404) - expect(await res.text()).toContain('custom 404 page') + describe('dev mode', () => { + let stderr = '' + let stdout = '' + + beforeAll(async () => { + appPort = await findPort() + app = await launchApp(appDir, appPort, { + onStderr(msg) { + stderr += msg + }, + onStdout(msg) { + stdout += msg + }, + }) + }) + afterAll(() => killApp(app)) + + it('should not show pages/404 GIP error if _app has GIP', async () => { + const res = await fetchViaHTTP(appPort, '/abc') + expect(res.status).toBe(404) + expect(await res.text()).toContain('custom 404 page') + expect(stderr).not.toMatch(gip404Err) + expect(stdout).not.toMatch(gip404Err) + }) }) }) })