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
36 changes: 34 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,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'`?
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
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
}
SukkaW marked this conversation as resolved.
Show resolved Hide resolved
return extractValue(cooked)
SukkaW marked this conversation as resolved.
Show resolved Hide resolved
} else {
throw new UnsupportedValueError()
}
Expand Down
24 changes: 21 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,28 @@ 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) {
// `export config` is found, but can't extract its value
Log.warn(
SukkaW marked this conversation as resolved.
Show resolved Hide resolved
`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
}
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
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`,
}