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
fix(#38743): config.runtime support template literal #38750
Changes from 10 commits
5bc2585
f35478e
dcbfbde
754c515
83f753f
6dc3e28
f55f103
d7a713c
e34ba69
1858c1a
3615bd9
edd4b40
adfe7b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,39 +1,119 @@ | ||
# 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 | ||
<table> | ||
<thead> | ||
<tr> | ||
<th>Not Allowed</th> | ||
<th>Allowed</th> | ||
</tr> | ||
</thead> | ||
<tbody> | ||
|
||
<tr> | ||
<td> | ||
|
||
```js | ||
// `config` should be an object | ||
export const config = 'hello world' | ||
``` | ||
|
||
This is not allowed | ||
</td> | ||
<td> | ||
|
||
```js | ||
export const config = {} | ||
``` | ||
|
||
</td> | ||
</tr> | ||
|
||
<tr> | ||
<td> | ||
|
||
```js | ||
const config = {} | ||
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, | ||
} | ||
``` | ||
|
||
</td> | ||
<td> | ||
|
||
```js | ||
export const config = { | ||
amp: true, | ||
} | ||
|
||
export const config = { | ||
amp: false, | ||
} | ||
``` | ||
|
||
This is not allowed | ||
</td> | ||
</tr> | ||
|
||
<tr> | ||
<td> | ||
|
||
```js | ||
// `config.runtime` contains a dynamic expression | ||
export const config = { | ||
runtime: `node${'js'}`, | ||
} | ||
``` | ||
|
||
</td> | ||
<td> | ||
|
||
```js | ||
export const config = { | ||
runtime: 'nodejs', | ||
} | ||
export const config = { | ||
runtime: `nodejs`, | ||
} | ||
``` | ||
|
||
</td> | ||
</tr> | ||
|
||
<tr> | ||
<td> | ||
|
||
```js | ||
// Re-exported `config` is not allowed | ||
export { config } from '../config' | ||
``` | ||
|
||
This is allowed | ||
</td> | ||
<td> | ||
|
||
```js | ||
export const config = { amp: true } | ||
export const config = {} | ||
``` | ||
|
||
</td> | ||
</tr> | ||
|
||
</tbody> | ||
</table> | ||
|
||
### 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) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ import type { | |
ObjectExpression, | ||
RegExpLiteral, | ||
StringLiteral, | ||
TemplateLiteral, | ||
VariableDeclaration, | ||
} from '@swc/core' | ||
|
||
|
@@ -60,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' | ||
} | ||
|
@@ -124,8 +105,12 @@ function isRegExpLiteral(node: Node): node is RegExpLiteral { | |
return node.type === 'RegExpLiteral' | ||
} | ||
|
||
class UnsupportedValueError extends Error {} | ||
class NoSuchDeclarationError extends Error {} | ||
function isTemplateLiteral(node: Node): node is TemplateLiteral { | ||
return node.type === 'TemplateLiteral' | ||
} | ||
|
||
export class UnsupportedValueError extends Error {} | ||
export class NoSuchDeclarationError extends Error {} | ||
|
||
function extractValue(node: Node): any { | ||
if (isNullLiteral(node)) { | ||
|
@@ -191,6 +176,25 @@ 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'`? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thought: It is fair to not support real interpolation. The best option would be to load the file and check the exported value. However, that's not an option, so we can go with this tradeoff. |
||
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 | ||
// | ||
// 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, raw }] = node.quasis | ||
|
||
return cooked ?? raw | ||
} else { | ||
throw new UnsupportedValueError() | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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,23 @@ export async function getPageStaticInfo(params: { | |
isDev?: boolean | ||
page?: string | ||
}): Promise<PageStaticInfo> { | ||
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) { | ||
warnAboutUnsupportedValue(pageFilePath, page) | ||
} | ||
// `export config` doesn't exist, or other unknown error throw by swc, silence them | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion(blocker): rather than discontinuing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have thought about adding the logging in So I just use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough. Let's get rid of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure! I have removed it in |
||
|
||
if ( | ||
typeof config.runtime !== 'string' && | ||
|
@@ -218,3 +231,19 @@ function warnAboutExperimentalEdgeApiFunctions() { | |
} | ||
|
||
let warnedAboutExperimentalEdgeApiFunctions = false | ||
|
||
const warnedUnsupportedValueMap = new Map<string, boolean>() | ||
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. See: https://nextjs.org/docs/messages/invalid-page-config` | ||
) | ||
warnedUnsupportedValueMap.set(pageFilePath, true) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
export default (req) => { | ||
return new Response(`Returned by Edge API Route ${req.url}`) | ||
} | ||
|
||
export const config = { | ||
runtime: `experimental-edge`, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
export default function Page() { | ||
return <p>hello world</p> | ||
} | ||
|
||
export const config = { | ||
runtime: `something-${'real' + 1 + 'y odd'}`, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: it is hard to spot what is allowed and what is not.
What about using a table, or bold, or any artifact helping readers to immediately find what is allowed and what is not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@feugy Agree.
And an alternative I have thought of is to replicate the ESLint documentation style:
https://eslint.org/docs/latest/rules/valid-typeof
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in
1858c1a
(#38750)