Skip to content
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

Merged
merged 13 commits into from Jul 21, 2022
28 changes: 26 additions & 2 deletions packages/next/build/analysis/extract-const-value.ts
Expand Up @@ -11,6 +11,7 @@ import type {
ObjectExpression,
RegExpLiteral,
StringLiteral,
TemplateLiteral,
VariableDeclaration,
} from '@swc/core'

Expand Down Expand Up @@ -124,8 +125,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)) {
Expand Down Expand Up @@ -191,6 +196,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'`?
Copy link
Member

Choose a reason for hiding this comment

The 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()
}
Expand Down
35 changes: 32 additions & 3 deletions 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'
Expand Down Expand Up @@ -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
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion(blocker): rather than discontinuing tryToExtractExportedConstValue, you could enhance it to print the warning.

Copy link
Contributor Author

@SukkaW SukkaW Jul 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have thought about adding the logging in tryToExtractExportedConstValue. But I'd like to include pageFilePath or route path in the warning message (the developer can then locate and identify the problematic file).

So I just use extractExportedConstValue directly instead, and I don't need to pass the context (route path, page file path, etc.) down to tryToExtractExportedConstValue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. Let's get rid of tryToExtractExportedConstValue since it's not used anywhere else.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! I have removed it in e34ba69 (#38750).


if (
typeof config.runtime !== 'string' &&
Expand Down Expand Up @@ -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`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we mention the specific field that we couldn't analyze?

Also can we ensure we link to an err.sh document here potentially this existing doc

Copy link
Contributor Author

@SukkaW SukkaW Jul 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we mention the specific field that we couldn't analyze?

The UnsupportedValueError is thrown directly when encounter invalid/unknown/unsupported syntax without more details. Thus it can't be done currently.

Maybe I could implement it in future PR.

Also can we ensure we link to an err.sh document here potentially this existing doc

Sure! I will update the existing doc and link to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d7a713c (#38750)

)
warnedUnsupportedValueMap.set(pageFilePath, true)
}
42 changes: 36 additions & 6 deletions test/e2e/switchable-runtime/index.test.ts
Expand Up @@ -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')
Expand All @@ -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/edge',
page: '/api/edge',
regexp: '^/api/edge$',
wasm: [],
},
},
})
}
Expand Down Expand Up @@ -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')
Expand All @@ -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: [],
},
},
})
}
Expand Down
7 changes: 7 additions & 0 deletions 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`,
}
Expand Up @@ -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 }
)
Expand All @@ -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`
),
})
})
})
@@ -0,0 +1,7 @@
export default function Page() {
return <p>hello world</p>
}

export const config = {
runtime: `something-${'real' + 1 + 'y odd'}`,
}