From 5bc2585707726dfe1643a02431c58016d1d8af12 Mon Sep 17 00:00:00 2001 From: SukkaW Date: Mon, 18 Jul 2022 16:04:08 +0800 Subject: [PATCH 1/9] fix(#38743): config.runtime support template literal --- .../build/analysis/extract-const-value.ts | 32 ++++++++++++++ test/e2e/switchable-runtime/index.test.ts | 42 ++++++++++++++++--- test/e2e/switchable-runtime/pages/api/edge.js | 7 ++++ 3 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 test/e2e/switchable-runtime/pages/api/edge.js diff --git a/packages/next/build/analysis/extract-const-value.ts b/packages/next/build/analysis/extract-const-value.ts index 0cfb372c5b8f..edfb89ea5e29 100644 --- a/packages/next/build/analysis/extract-const-value.ts +++ b/packages/next/build/analysis/extract-const-value.ts @@ -11,6 +11,7 @@ import type { ObjectExpression, RegExpLiteral, StringLiteral, + TemplateLiteral, VariableDeclaration, } from '@swc/core' @@ -124,6 +125,10 @@ function isRegExpLiteral(node: Node): node is RegExpLiteral { return node.type === 'RegExpLiteral' } +function isTemplateLiteral(node: Node): node is TemplateLiteral { + return node.type === 'TemplateLiteral' +} + class UnsupportedValueError extends Error {} class NoSuchDeclarationError extends Error {} @@ -191,6 +196,33 @@ function extractValue(node: Node): any { } return obj + } else if (isTemplateLiteral(node)) { + // e.g. `abc` + if (node.expressions.length !== 0) { + // TODO: should we add support for `${'e'}d${'g'}'e'`? + throw new UnsupportedValueError() + } + + // When TemplateLiteral has 0 expressions, the length of quasis is always 1. + // Because when parsing TemplateLiteral, the parser yields the first quasi, + // then the first expression, then the next quasi, then the next expression, etc., + // until the last quasi. + // Thus if there is no expression, the parser ends at the frst and also last quasis + const firstQuasis = node.quasis[0] + + // A "cooked" interpretation where backslashes have special meaning, while a + // "raw" interpretation where backslashes do not have special meaning + // https://exploringjs.com/impatient-js/ch_template-literals.html#template-strings-cooked-vs-raw + const { cooked } = firstQuasis + + // FIXME: The type definition of "cooked" and "raw" (from swc) are outdated. + // Both of them should be string | null | undefined, not StringLiteral. + // It is a temporary type guard to make TypeScript happy. + // https://github.com/swc-project/swc/issues/4501 + if (cooked == null || typeof cooked === 'string') { + return cooked + } + return extractValue(cooked) } else { throw new UnsupportedValueError() } diff --git a/test/e2e/switchable-runtime/index.test.ts b/test/e2e/switchable-runtime/index.test.ts index 6c05d4ad30cf..2840750c0b4d 100644 --- a/test/e2e/switchable-runtime/index.test.ts +++ b/test/e2e/switchable-runtime/index.test.ts @@ -115,11 +115,15 @@ describe('Switchable runtime', () => { ) }) - it('should build /api/hello as an api route with edge runtime', async () => { - const response = await fetchViaHTTP(context.appPort, '/api/hello') - const text = await response.text() + it('should build /api/hello and /api/edge as an api route with edge runtime', async () => { + let response = await fetchViaHTTP(context.appPort, '/api/hello') + let text = await response.text() expect(text).toMatch(/Hello from .+\/api\/hello/) + response = await fetchViaHTTP(context.appPort, '/api/edge') + text = await response.text() + expect(text).toMatch(/Returned by Edge API Route .+\/api\/edge/) + if (!(global as any).isNextDeploy) { const manifest = await readJson( join(context.appDir, '.next/server/middleware-manifest.json') @@ -137,6 +141,17 @@ describe('Switchable runtime', () => { regexp: '^/api/hello$', wasm: [], }, + '/api/edge': { + env: [], + files: [ + 'server/edge-runtime-webpack.js', + 'server/pages/api/edge.js', + ], + name: 'pages/api/hello', + page: '/api/edge', + regexp: '^/api/edge$', + wasm: [], + }, }, }) } @@ -235,11 +250,15 @@ describe('Switchable runtime', () => { }) }) - it('should build /api/hello as an api route with edge runtime', async () => { - const response = await fetchViaHTTP(context.appPort, '/api/hello') - const text = await response.text() + it('should build /api/hello and /api/edge as an api route with edge runtime', async () => { + let response = await fetchViaHTTP(context.appPort, '/api/hello') + let text = await response.text() expect(text).toMatch(/Hello from .+\/api\/hello/) + response = await fetchViaHTTP(context.appPort, '/api/edge') + text = await response.text() + expect(text).toMatch(/Returned by Edge API Route .+\/api\/edge/) + if (!(global as any).isNextDeploy) { const manifest = await readJson( join(context.appDir, '.next/server/middleware-manifest.json') @@ -257,6 +276,17 @@ describe('Switchable runtime', () => { regexp: '^/api/hello$', wasm: [], }, + '/api/edge': { + env: [], + files: [ + 'server/edge-runtime-webpack.js', + 'server/pages/api/edge.js', + ], + name: 'pages/api/edge', + page: '/api/edge', + regexp: '^/api/edge$', + wasm: [], + }, }, }) } diff --git a/test/e2e/switchable-runtime/pages/api/edge.js b/test/e2e/switchable-runtime/pages/api/edge.js new file mode 100644 index 000000000000..d375a24aaab7 --- /dev/null +++ b/test/e2e/switchable-runtime/pages/api/edge.js @@ -0,0 +1,7 @@ +export default (req) => { + return new Response(`Returned by Edge API Route ${req.url}`) +} + +export const config = { + runtime: `experimental-edge`, +} From f35478e0a6fc0a3abdf9ca3cae13660d9d21b9ea Mon Sep 17 00:00:00 2001 From: SukkaW Date: Mon, 18 Jul 2022 16:25:12 +0800 Subject: [PATCH 2/9] test: fix fixture --- test/e2e/switchable-runtime/index.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/switchable-runtime/index.test.ts b/test/e2e/switchable-runtime/index.test.ts index 2840750c0b4d..8d704a6c6e1c 100644 --- a/test/e2e/switchable-runtime/index.test.ts +++ b/test/e2e/switchable-runtime/index.test.ts @@ -147,7 +147,7 @@ describe('Switchable runtime', () => { 'server/edge-runtime-webpack.js', 'server/pages/api/edge.js', ], - name: 'pages/api/hello', + name: 'pages/api/edge', page: '/api/edge', regexp: '^/api/edge$', wasm: [], From dcbfbde89484393c4c006fbe7a9030e0ca6e35c3 Mon Sep 17 00:00:00 2001 From: SukkaW Date: Mon, 18 Jul 2022 23:18:05 +0800 Subject: [PATCH 3/9] feat: show a warning about unrecognized `export config` --- .../build/analysis/extract-const-value.ts | 4 ++-- .../build/analysis/get-page-static-info.ts | 24 ++++++++++++++++--- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/packages/next/build/analysis/extract-const-value.ts b/packages/next/build/analysis/extract-const-value.ts index edfb89ea5e29..0cca5dfaba89 100644 --- a/packages/next/build/analysis/extract-const-value.ts +++ b/packages/next/build/analysis/extract-const-value.ts @@ -129,8 +129,8 @@ function isTemplateLiteral(node: Node): node is TemplateLiteral { return node.type === 'TemplateLiteral' } -class UnsupportedValueError extends Error {} -class NoSuchDeclarationError extends Error {} +export class UnsupportedValueError extends Error {} +export class NoSuchDeclarationError extends Error {} function extractValue(node: Node): any { if (isNullLiteral(node)) { diff --git a/packages/next/build/analysis/get-page-static-info.ts b/packages/next/build/analysis/get-page-static-info.ts index 60d94aa800e3..7d2c4d73dbd7 100644 --- a/packages/next/build/analysis/get-page-static-info.ts +++ b/packages/next/build/analysis/get-page-static-info.ts @@ -1,6 +1,9 @@ import { isServerRuntime, ServerRuntime } from '../../server/config-shared' import type { NextConfig } from '../../server/config-shared' -import { tryToExtractExportedConstValue } from './extract-const-value' +import { + extractExportedConstValue, + UnsupportedValueError, +} from './extract-const-value' import { escapeStringRegexp } from '../../shared/lib/escape-regexp' import { parseModule } from './parse-module' import { promises as fs } from 'fs' @@ -32,13 +35,28 @@ export async function getPageStaticInfo(params: { isDev?: boolean page?: string }): Promise { - const { isDev, pageFilePath, nextConfig } = params + const { isDev, pageFilePath, nextConfig, page } = params const fileContent = (await tryToReadFile(pageFilePath, !isDev)) || '' if (/runtime|getStaticProps|getServerSideProps|matcher/.test(fileContent)) { const swcAST = await parseModule(pageFilePath, fileContent) const { ssg, ssr } = checkExports(swcAST) - const config = tryToExtractExportedConstValue(swcAST, 'config') || {} + + // default / failsafe value for config + let config: any = {} + try { + config = extractExportedConstValue(swcAST, 'config') + } catch (e) { + if (e instanceof UnsupportedValueError) { + // `export config` is found, but can't extract its value + Log.warn( + `You have exported a \`config\` field in "${ + page || pageFilePath + }" that Next.js can't recognize, so it will be ignored` + ) + } + // `export config` doesn't exist, or other unknown error throw by swc, silence them + } if ( typeof config.runtime !== 'string' && From 83f753fa2107c7bf30cad18225ed522eb4613def Mon Sep 17 00:00:00 2001 From: SukkaW Date: Thu, 21 Jul 2022 01:16:51 +0800 Subject: [PATCH 4/9] refactor: simplify the logic Co-authored-by: feugy --- .../next/build/analysis/extract-const-value.ts | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/next/build/analysis/extract-const-value.ts b/packages/next/build/analysis/extract-const-value.ts index 0cca5dfaba89..2e415ba30d5f 100644 --- a/packages/next/build/analysis/extract-const-value.ts +++ b/packages/next/build/analysis/extract-const-value.ts @@ -208,21 +208,13 @@ function extractValue(node: Node): any { // then the first expression, then the next quasi, then the next expression, etc., // until the last quasi. // Thus if there is no expression, the parser ends at the frst and also last quasis - const firstQuasis = node.quasis[0] - + // // A "cooked" interpretation where backslashes have special meaning, while a // "raw" interpretation where backslashes do not have special meaning // https://exploringjs.com/impatient-js/ch_template-literals.html#template-strings-cooked-vs-raw - const { cooked } = firstQuasis - - // FIXME: The type definition of "cooked" and "raw" (from swc) are outdated. - // Both of them should be string | null | undefined, not StringLiteral. - // It is a temporary type guard to make TypeScript happy. - // https://github.com/swc-project/swc/issues/4501 - if (cooked == null || typeof cooked === 'string') { - return cooked - } - return extractValue(cooked) + const [{ cooked, raw }] = node.quasis + + return cooked ?? raw } else { throw new UnsupportedValueError() } From 6dc3e282c425790733924f84a6c77b55b23056f3 Mon Sep 17 00:00:00 2001 From: SukkaW Date: Thu, 21 Jul 2022 02:32:11 +0800 Subject: [PATCH 5/9] fix: make sure unrecognized value is only warned once --- .../build/analysis/get-page-static-info.ts | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/next/build/analysis/get-page-static-info.ts b/packages/next/build/analysis/get-page-static-info.ts index 7d2c4d73dbd7..ee4679a38fae 100644 --- a/packages/next/build/analysis/get-page-static-info.ts +++ b/packages/next/build/analysis/get-page-static-info.ts @@ -48,12 +48,7 @@ export async function getPageStaticInfo(params: { config = extractExportedConstValue(swcAST, 'config') } catch (e) { if (e instanceof UnsupportedValueError) { - // `export config` is found, but can't extract its value - Log.warn( - `You have exported a \`config\` field in "${ - page || pageFilePath - }" that Next.js can't recognize, so it will be ignored` - ) + warnAboutUnsupportedValue(pageFilePath, page) } // `export config` doesn't exist, or other unknown error throw by swc, silence them } @@ -236,3 +231,19 @@ function warnAboutExperimentalEdgeApiFunctions() { } let warnedAboutExperimentalEdgeApiFunctions = false + +const warnedUnsupportedValueMap = new Map() +function warnAboutUnsupportedValue( + pageFilePath: string, + page: string | undefined +) { + if (warnedUnsupportedValueMap.has(pageFilePath)) { + return + } + Log.warn( + `You have exported a \`config\` field in ${ + page ? `route "${page}"` : `"${pageFilePath}"` + } that Next.js can't recognize, so it will be ignored` + ) + warnedUnsupportedValueMap.set(pageFilePath, true) +} From f55f103a88cbd87b518187173034c87e2241b15b Mon Sep 17 00:00:00 2001 From: SukkaW Date: Thu, 21 Jul 2022 02:32:21 +0800 Subject: [PATCH 6/9] test: add "warns on unrecognized runtimes value" --- .../index.test.ts | 17 ++++++++++++++++- .../{ => invalid-runtime}/app/pages/index.js | 0 .../unsupported-syntax/app/pages/index.js | 7 +++++++ 3 files changed, 23 insertions(+), 1 deletion(-) rename test/production/exported-runtimes-value-validation/{ => invalid-runtime}/app/pages/index.js (100%) create mode 100644 test/production/exported-runtimes-value-validation/unsupported-syntax/app/pages/index.js diff --git a/test/production/exported-runtimes-value-validation/index.test.ts b/test/production/exported-runtimes-value-validation/index.test.ts index 73d4009ff442..70157cab6498 100644 --- a/test/production/exported-runtimes-value-validation/index.test.ts +++ b/test/production/exported-runtimes-value-validation/index.test.ts @@ -4,7 +4,7 @@ import path from 'path' describe('Exported runtimes value validation', () => { test('fails to build on malformed input', async () => { const result = await nextBuild( - path.resolve(__dirname, './app'), + path.resolve(__dirname, './invalid-runtime/app'), undefined, { stdout: true, stderr: true } ) @@ -15,4 +15,19 @@ describe('Exported runtimes value validation', () => { ), }) }) + + test('warns on unrecognized runtimes value', async () => { + const result = await nextBuild( + path.resolve(__dirname, './unsupported-syntax/app'), + undefined, + { stdout: true, stderr: true } + ) + + expect(result).toMatchObject({ + code: 0, + stderr: expect.stringContaining( + `You have exported a \`config\` field in route "/" that Next.js can't recognize, so it will be ignored` + ), + }) + }) }) diff --git a/test/production/exported-runtimes-value-validation/app/pages/index.js b/test/production/exported-runtimes-value-validation/invalid-runtime/app/pages/index.js similarity index 100% rename from test/production/exported-runtimes-value-validation/app/pages/index.js rename to test/production/exported-runtimes-value-validation/invalid-runtime/app/pages/index.js diff --git a/test/production/exported-runtimes-value-validation/unsupported-syntax/app/pages/index.js b/test/production/exported-runtimes-value-validation/unsupported-syntax/app/pages/index.js new file mode 100644 index 000000000000..a340aa42d830 --- /dev/null +++ b/test/production/exported-runtimes-value-validation/unsupported-syntax/app/pages/index.js @@ -0,0 +1,7 @@ +export default function Page() { + return

hello world

+} + +export const config = { + runtime: `something-${'real' + 1 + 'y odd'}`, +} From d7a713c906bd903f20d56bfc7717cf1fab663347 Mon Sep 17 00:00:00 2001 From: SukkaW Date: Thu, 21 Jul 2022 13:10:41 +0800 Subject: [PATCH 7/9] docs(errors): update invalid page config --- errors/invalid-page-config.md | 38 ++++++++++++++++++- .../build/analysis/get-page-static-info.ts | 2 +- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/errors/invalid-page-config.md b/errors/invalid-page-config.md index 6984a9bb1b9e..bc4ed298a42e 100644 --- a/errors/invalid-page-config.md +++ b/errors/invalid-page-config.md @@ -1,12 +1,13 @@ -# Invalid Page Config +# Invalid Page / API Route Config #### Why This Error Occurred -In one of your pages you did `export const config` with an invalid value. +In one of your pages or API Routes you did `export const config` with an invalid value. #### Possible Ways to Fix It The page's config must be an object initialized directly when being exported and not modified dynamically. +The config object must only contains static constant literals without expressions. This is not allowed @@ -23,17 +24,50 @@ config.amp = true This is not allowed +```js +export const config = { + amp: 1 + 1 > 2, +} +``` + +This is not allowed + ```js export { config } from '../config' ``` +This is not allowed + +```js +export const config = { + runtime: `n${'od'}ejs`, +} +``` + This is allowed ```js export const config = { amp: true } ``` +This is allowed + +```js +export const config = { + runtime: 'nodejs', +} +``` + +This is allowed + +```js +export const config = { + runtime: `nodejs`, +} +``` + ### Useful Links - [Enabling AMP Support](https://nextjs.org/docs/advanced-features/amp-support/introduction) - [API Middlewares](https://nextjs.org/docs/api-routes/api-middlewares) +- [Switchable Runtime](https://nextjs.org/docs/advanced-features/react-18/switchable-runtime) diff --git a/packages/next/build/analysis/get-page-static-info.ts b/packages/next/build/analysis/get-page-static-info.ts index ee4679a38fae..77dda52ce689 100644 --- a/packages/next/build/analysis/get-page-static-info.ts +++ b/packages/next/build/analysis/get-page-static-info.ts @@ -243,7 +243,7 @@ function warnAboutUnsupportedValue( Log.warn( `You have exported a \`config\` field in ${ page ? `route "${page}"` : `"${pageFilePath}"` - } that Next.js can't recognize, so it will be ignored` + } that Next.js can't recognize, so it will be ignored. See: https://nextjs.org/docs/messages/invalid-page-config` ) warnedUnsupportedValueMap.set(pageFilePath, true) } From e34ba6953f95bac6590436a1d739e6d32e6b4197 Mon Sep 17 00:00:00 2001 From: SukkaW Date: Thu, 21 Jul 2022 17:36:47 +0800 Subject: [PATCH 8/9] refactor: remove `tryToExtractExportedConstValue` --- .../build/analysis/extract-const-value.ts | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/packages/next/build/analysis/extract-const-value.ts b/packages/next/build/analysis/extract-const-value.ts index 2e415ba30d5f..db9ce53187e7 100644 --- a/packages/next/build/analysis/extract-const-value.ts +++ b/packages/next/build/analysis/extract-const-value.ts @@ -61,26 +61,6 @@ export function extractExportedConstValue( throw new NoSuchDeclarationError() } -/** - * A wrapper on top of `extractExportedConstValue` that returns undefined - * instead of throwing when the thrown error is known. - */ -export function tryToExtractExportedConstValue( - module: Module, - exportedName: string -) { - try { - return extractExportedConstValue(module, exportedName) - } catch (error) { - if ( - error instanceof UnsupportedValueError || - error instanceof NoSuchDeclarationError - ) { - return undefined - } - } -} - function isExportDeclaration(node: Node): node is ExportDeclaration { return node.type === 'ExportDeclaration' } From 1858c1a2fa0359bb58c8abf3cdb7ee93796aa6b7 Mon Sep 17 00:00:00 2001 From: SukkaW Date: Thu, 21 Jul 2022 19:29:39 +0800 Subject: [PATCH 9/9] docs(errors): improve invalid-page-config readability --- errors/invalid-page-config.md | 84 +++++++++++++++++++++++++++-------- 1 file changed, 65 insertions(+), 19 deletions(-) diff --git a/errors/invalid-page-config.md b/errors/invalid-page-config.md index bc4ed298a42e..6d24391d2ea4 100644 --- a/errors/invalid-page-config.md +++ b/errors/invalid-page-config.md @@ -9,63 +9,109 @@ In one of your pages or API Routes you did `export const config` with an invalid The page's config must be an object initialized directly when being exported and not modified dynamically. The config object must only contains static constant literals without expressions. -This is not allowed + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Not AllowedAllowed
```js +// `config` should be an object export const config = 'hello world' ``` -This is not allowed + ```js -const config = {} -config.amp = true +export const config = {} ``` -This is not allowed +
```js +export const config = {} +// `config.amp` is defined after `config` is exported +config.amp = true + +// `config.amp` contains a dynamic expression export const config = { amp: 1 + 1 > 2, } ``` -This is not allowed + ```js -export { config } from '../config' -``` - -This is not allowed +export const config = { + amp: true, +} -```js export const config = { - runtime: `n${'od'}ejs`, + amp: false, } ``` -This is allowed +
```js -export const config = { amp: true } +// `config.runtime` contains a dynamic expression +export const config = { + runtime: `node${'js'}`, +} ``` -This is allowed + ```js export const config = { runtime: 'nodejs', } +export const config = { + runtime: `nodejs`, +} ``` -This is allowed +
```js -export const config = { - runtime: `nodejs`, -} +// Re-exported `config` is not allowed +export { config } from '../config' ``` + + +```js +export const config = {} +``` + +
+ ### Useful Links - [Enabling AMP Support](https://nextjs.org/docs/advanced-features/amp-support/introduction)