From 9f81baa5e93a1985176fc6f81bb138f024b2ff7d Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 2 Jan 2020 16:08:36 -0600 Subject: [PATCH 1/4] Add warning when providing query values in exportPathMap for auto-export page --- packages/next/export/worker.js | 16 ++++- .../auto-export-query-warn/next.config.js | 10 ++++ .../auto-export-query-warn/pages/amp.js | 3 + .../auto-export-query-warn/pages/hello.js | 1 + .../auto-export-query-warn/pages/ssr.js | 5 ++ .../auto-export-query-warn/test/index.test.js | 59 +++++++++++++++++++ 6 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 test/integration/auto-export-query-warn/next.config.js create mode 100644 test/integration/auto-export-query-warn/pages/amp.js create mode 100644 test/integration/auto-export-query-warn/pages/hello.js create mode 100644 test/integration/auto-export-query-warn/pages/ssr.js create mode 100644 test/integration/auto-export-query-warn/test/index.test.js diff --git a/packages/next/export/worker.js b/packages/next/export/worker.js index 072faae8dd11667..abeb2412a9ede87 100644 --- a/packages/next/export/worker.js +++ b/packages/next/export/worker.js @@ -37,12 +37,24 @@ export default async function({ } try { - let { query = {} } = pathMap + const { query: originalQuery = {} } = pathMap const { page } = pathMap const filePath = path === '/' ? '/index' : path const ampPath = `${filePath}.amp` + let query = { ...originalQuery } let params + // We need to show a warning if they try to provide query values + // for an auto-exported page since they won't be available + const hasOrigQueryValues = Object.keys(originalQuery).length > 0 + const queryWithAutoExportWarn = () => { + if (hasOrigQueryValues) { + console.warn( + `\nWarn: you provided query values for ${path} which is an auto-exported page. These can not be applied since the page can no longer be re-rendered on the server. To disable auto-export for this page add \`getInitialProps\`\n` + ) + } + } + // Check if the page is a specified dynamic route if (isDynamicRoute(page) && page !== path) { params = getRouteMatcher(getRouteRegex(page))(path) @@ -130,6 +142,7 @@ export default async function({ // if it was auto-exported the HTML is loaded here if (typeof mod === 'string') { html = mod + queryWithAutoExportWarn() } else { // for non-dynamic SPR pages we should have already // prerendered the file @@ -176,6 +189,7 @@ export default async function({ if (typeof components.Component === 'string') { html = components.Component + queryWithAutoExportWarn() } else { curRenderOpts = { ...components, ...renderOpts, ampPath } html = await renderMethod(req, res, page, query, curRenderOpts) diff --git a/test/integration/auto-export-query-warn/next.config.js b/test/integration/auto-export-query-warn/next.config.js new file mode 100644 index 000000000000000..39584969c6a1b7c --- /dev/null +++ b/test/integration/auto-export-query-warn/next.config.js @@ -0,0 +1,10 @@ +module.exports = { + // target: 'serverless', + exportPathMap() { + return { + '/': { page: '/hello', query: { first: 'second' } }, + '/amp': { page: '/amp' }, + '/ssr': { page: '/ssr' }, + } + }, +} diff --git a/test/integration/auto-export-query-warn/pages/amp.js b/test/integration/auto-export-query-warn/pages/amp.js new file mode 100644 index 000000000000000..e9c90ebfe5ac7b6 --- /dev/null +++ b/test/integration/auto-export-query-warn/pages/amp.js @@ -0,0 +1,3 @@ +export const config = { amp: 'hybrid' } + +export default () => 'hi from hybrid AMP' diff --git a/test/integration/auto-export-query-warn/pages/hello.js b/test/integration/auto-export-query-warn/pages/hello.js new file mode 100644 index 000000000000000..0957a987fc2f227 --- /dev/null +++ b/test/integration/auto-export-query-warn/pages/hello.js @@ -0,0 +1 @@ +export default () => 'hi' diff --git a/test/integration/auto-export-query-warn/pages/ssr.js b/test/integration/auto-export-query-warn/pages/ssr.js new file mode 100644 index 000000000000000..31534f6e5a8a374 --- /dev/null +++ b/test/integration/auto-export-query-warn/pages/ssr.js @@ -0,0 +1,5 @@ +const Page = () => "I'm SSRed" + +Page.getInitialProps = () => ({ hello: 'world' }) + +export default Page diff --git a/test/integration/auto-export-query-warn/test/index.test.js b/test/integration/auto-export-query-warn/test/index.test.js new file mode 100644 index 000000000000000..1d91b22cbf11344 --- /dev/null +++ b/test/integration/auto-export-query-warn/test/index.test.js @@ -0,0 +1,59 @@ +/* eslint-env jest */ +/* global jasmine */ +import path from 'path' +import fs from 'fs-extra' +import { nextBuild, nextExport } from 'next-test-utils' + +jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 1 +const appDir = path.join(__dirname, '..') +const outdir = path.join(__dirname, 'out') +const nextConfig = path.join(appDir, 'next.config.js') +let stderr + +const runTests = () => { + it('should show warning for query provided for auto exported page correctly', async () => { + expect(stderr).toContain( + 'Warn: you provided query values for / which is an auto-exported page. These can not be applied since the page can no longer be re-rendered on the server. To disable auto-export for this page add `getInitialProps`' + ) + + expect(stderr).not.toContain('/amp') + expect(stderr).not.toContain('/ssr') + expect(stderr).not.toContain('/hello') + }) +} + +let origNextConfig + +describe('Auto Export', () => { + describe('server mode', () => { + beforeAll(async () => { + await nextBuild(appDir) + const { stderr: curStderr } = await nextExport( + appDir, + { outdir }, + { stderr: true } + ) + stderr = curStderr + }) + + runTests() + }) + + describe('serverless mode', () => { + beforeAll(async () => { + origNextConfig = await fs.readFile(nextConfig, 'utf8') + await nextBuild(appDir) + const { stderr: curStderr } = await nextExport( + appDir, + { outdir }, + { stderr: true } + ) + stderr = curStderr + }) + afterAll(async () => { + await fs.writeFile(nextConfig, origNextConfig) + }) + + runTests() + }) +}) From 75d089cddabf79ca6396dde21e079e04106dbf73 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 2 Jan 2020 16:12:05 -0600 Subject: [PATCH 2/4] Update test for SSG page also --- test/integration/auto-export-query-warn/pages/ssg.js | 11 +++++++++++ .../auto-export-query-warn/test/index.test.js | 1 + 2 files changed, 12 insertions(+) create mode 100644 test/integration/auto-export-query-warn/pages/ssg.js diff --git a/test/integration/auto-export-query-warn/pages/ssg.js b/test/integration/auto-export-query-warn/pages/ssg.js new file mode 100644 index 000000000000000..f92e23e36d47857 --- /dev/null +++ b/test/integration/auto-export-query-warn/pages/ssg.js @@ -0,0 +1,11 @@ +const Page = () => "I'm SSRed" + +export async function getStaticProps() { + return { + props: { + hello: 'world', + }, + } +} + +export default Page diff --git a/test/integration/auto-export-query-warn/test/index.test.js b/test/integration/auto-export-query-warn/test/index.test.js index 1d91b22cbf11344..46b9edcdc25c4f4 100644 --- a/test/integration/auto-export-query-warn/test/index.test.js +++ b/test/integration/auto-export-query-warn/test/index.test.js @@ -18,6 +18,7 @@ const runTests = () => { expect(stderr).not.toContain('/amp') expect(stderr).not.toContain('/ssr') + expect(stderr).not.toContain('/ssg') expect(stderr).not.toContain('/hello') }) } From 2faa0229261950294821680de8fa9fe12d68588b Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 16 Jan 2020 19:12:20 -0600 Subject: [PATCH 3/4] Update to error instead of warn --- packages/next/export/worker.js | 2 +- .../next.config.js | 0 .../pages/amp.js | 0 .../pages/hello.js | 0 .../pages/ssg.js | 0 .../pages/ssr.js | 0 .../test/index.test.js | 0 7 files changed, 1 insertion(+), 1 deletion(-) rename test/integration/{auto-export-query-warn => auto-export-query-error}/next.config.js (100%) rename test/integration/{auto-export-query-warn => auto-export-query-error}/pages/amp.js (100%) rename test/integration/{auto-export-query-warn => auto-export-query-error}/pages/hello.js (100%) rename test/integration/{auto-export-query-warn => auto-export-query-error}/pages/ssg.js (100%) rename test/integration/{auto-export-query-warn => auto-export-query-error}/pages/ssr.js (100%) rename test/integration/{auto-export-query-warn => auto-export-query-error}/test/index.test.js (100%) diff --git a/packages/next/export/worker.js b/packages/next/export/worker.js index d86d6260419678a..e2be389d8e1027f 100644 --- a/packages/next/export/worker.js +++ b/packages/next/export/worker.js @@ -49,7 +49,7 @@ export default async function({ const hasOrigQueryValues = Object.keys(originalQuery).length > 0 const queryWithAutoExportWarn = () => { if (hasOrigQueryValues) { - console.warn( + throw new Error( `\nWarn: you provided query values for ${path} which is an auto-exported page. These can not be applied since the page can no longer be re-rendered on the server. To disable auto-export for this page add \`getInitialProps\`\n` ) } diff --git a/test/integration/auto-export-query-warn/next.config.js b/test/integration/auto-export-query-error/next.config.js similarity index 100% rename from test/integration/auto-export-query-warn/next.config.js rename to test/integration/auto-export-query-error/next.config.js diff --git a/test/integration/auto-export-query-warn/pages/amp.js b/test/integration/auto-export-query-error/pages/amp.js similarity index 100% rename from test/integration/auto-export-query-warn/pages/amp.js rename to test/integration/auto-export-query-error/pages/amp.js diff --git a/test/integration/auto-export-query-warn/pages/hello.js b/test/integration/auto-export-query-error/pages/hello.js similarity index 100% rename from test/integration/auto-export-query-warn/pages/hello.js rename to test/integration/auto-export-query-error/pages/hello.js diff --git a/test/integration/auto-export-query-warn/pages/ssg.js b/test/integration/auto-export-query-error/pages/ssg.js similarity index 100% rename from test/integration/auto-export-query-warn/pages/ssg.js rename to test/integration/auto-export-query-error/pages/ssg.js diff --git a/test/integration/auto-export-query-warn/pages/ssr.js b/test/integration/auto-export-query-error/pages/ssr.js similarity index 100% rename from test/integration/auto-export-query-warn/pages/ssr.js rename to test/integration/auto-export-query-error/pages/ssr.js diff --git a/test/integration/auto-export-query-warn/test/index.test.js b/test/integration/auto-export-query-error/test/index.test.js similarity index 100% rename from test/integration/auto-export-query-warn/test/index.test.js rename to test/integration/auto-export-query-error/test/index.test.js From 1103555b68e15f67e0a70363d4342398286de9a5 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 16 Jan 2020 20:30:06 -0600 Subject: [PATCH 4/4] Update warn -> Error and check exit code --- packages/next/export/worker.js | 2 +- .../auto-export-query-error/test/index.test.js | 10 +++++++--- test/lib/next-test-utils.js | 3 ++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/next/export/worker.js b/packages/next/export/worker.js index e2be389d8e1027f..30e7db5d67275fb 100644 --- a/packages/next/export/worker.js +++ b/packages/next/export/worker.js @@ -50,7 +50,7 @@ export default async function({ const queryWithAutoExportWarn = () => { if (hasOrigQueryValues) { throw new Error( - `\nWarn: you provided query values for ${path} which is an auto-exported page. These can not be applied since the page can no longer be re-rendered on the server. To disable auto-export for this page add \`getInitialProps\`\n` + `\nError: you provided query values for ${path} which is an auto-exported page. These can not be applied since the page can no longer be re-rendered on the server. To disable auto-export for this page add \`getInitialProps\`\n` ) } } diff --git a/test/integration/auto-export-query-error/test/index.test.js b/test/integration/auto-export-query-error/test/index.test.js index 46b9edcdc25c4f4..919ca14e4a6968d 100644 --- a/test/integration/auto-export-query-error/test/index.test.js +++ b/test/integration/auto-export-query-error/test/index.test.js @@ -9,11 +9,13 @@ const appDir = path.join(__dirname, '..') const outdir = path.join(__dirname, 'out') const nextConfig = path.join(appDir, 'next.config.js') let stderr +let exitCode const runTests = () => { it('should show warning for query provided for auto exported page correctly', async () => { + expect(exitCode).toBe(1) expect(stderr).toContain( - 'Warn: you provided query values for / which is an auto-exported page. These can not be applied since the page can no longer be re-rendered on the server. To disable auto-export for this page add `getInitialProps`' + 'Error: you provided query values for / which is an auto-exported page. These can not be applied since the page can no longer be re-rendered on the server. To disable auto-export for this page add `getInitialProps`' ) expect(stderr).not.toContain('/amp') @@ -29,12 +31,13 @@ describe('Auto Export', () => { describe('server mode', () => { beforeAll(async () => { await nextBuild(appDir) - const { stderr: curStderr } = await nextExport( + const { stderr: curStderr, code: curCode } = await nextExport( appDir, { outdir }, { stderr: true } ) stderr = curStderr + exitCode = curCode }) runTests() @@ -44,12 +47,13 @@ describe('Auto Export', () => { beforeAll(async () => { origNextConfig = await fs.readFile(nextConfig, 'utf8') await nextBuild(appDir) - const { stderr: curStderr } = await nextExport( + const { stderr: curStderr, code: curCode } = await nextExport( appDir, { outdir }, { stderr: true } ) stderr = curStderr + exitCode = curCode }) afterAll(async () => { await fs.writeFile(nextConfig, origNextConfig) diff --git a/test/lib/next-test-utils.js b/test/lib/next-test-utils.js index b695ca874062931..a4bfcb75f6d71a0 100644 --- a/test/lib/next-test-utils.js +++ b/test/lib/next-test-utils.js @@ -111,8 +111,9 @@ export function runNextCommand(argv, options = {}) { }) } - instance.on('close', () => { + instance.on('close', code => { resolve({ + code, stdout: stdoutOutput, stderr: stderrOutput, })