Handle cases where config
is exported after its declaration (#16211)
AMP will still not work correctly when switching between non-AMP and AMP pages in development mode because of https://github.com/vercel/next.js/blob/canary/packages/next/build/babel/plugins/next-page-config.ts#L116-L120. Fixes #15704.
This commit is contained in:
parent
f8534a6e62
commit
aa4b87e357
9 changed files with 126 additions and 9 deletions
|
@ -6,7 +6,7 @@ In one of your pages 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.
|
||||
The page's config must be an object initialized directly when being exported and not modified dynamically.
|
||||
|
||||
This is not allowed
|
||||
|
||||
|
@ -14,6 +14,19 @@ This is not allowed
|
|||
export const config = 'hello world'
|
||||
```
|
||||
|
||||
This is not allowed
|
||||
|
||||
```js
|
||||
const config = {}
|
||||
config.amp = true
|
||||
```
|
||||
|
||||
This is not allowed
|
||||
|
||||
```js
|
||||
export { config } from '../config'
|
||||
```
|
||||
|
||||
This is allowed
|
||||
|
||||
```js
|
||||
|
|
|
@ -2,6 +2,8 @@ import { NodePath, PluginObj, types as BabelTypes } from '@babel/core'
|
|||
import { PageConfig } from 'next/types'
|
||||
import { STRING_LITERAL_DROP_BUNDLE } from '../../../next-server/lib/constants'
|
||||
|
||||
const CONFIG_KEY = 'config'
|
||||
|
||||
// replace program path with just a variable with the drop identifier
|
||||
function replaceBundle(path: any, t: typeof BabelTypes): void {
|
||||
path.parentPath.replaceWith(
|
||||
|
@ -41,27 +43,59 @@ export default function nextPageConfig({
|
|||
enter(path, state: ConfigState) {
|
||||
path.traverse(
|
||||
{
|
||||
ExportDeclaration(exportPath, exportState) {
|
||||
if (
|
||||
BabelTypes.isExportNamedDeclaration(exportPath) &&
|
||||
(exportPath.node as BabelTypes.ExportNamedDeclaration).specifiers?.some(
|
||||
(specifier) => {
|
||||
return specifier.exported.name === CONFIG_KEY
|
||||
}
|
||||
) &&
|
||||
BabelTypes.isStringLiteral(
|
||||
(exportPath.node as BabelTypes.ExportNamedDeclaration)
|
||||
.source
|
||||
)
|
||||
) {
|
||||
throw new Error(
|
||||
errorMessage(
|
||||
exportState,
|
||||
'Expected object but got export from'
|
||||
)
|
||||
)
|
||||
}
|
||||
},
|
||||
ExportNamedDeclaration(
|
||||
exportPath: NodePath<BabelTypes.ExportNamedDeclaration>,
|
||||
exportState: any
|
||||
) {
|
||||
if (exportState.bundleDropped || !exportPath.node.declaration) {
|
||||
return
|
||||
}
|
||||
|
||||
if (
|
||||
!BabelTypes.isVariableDeclaration(exportPath.node.declaration)
|
||||
exportState.bundleDropped ||
|
||||
(!exportPath.node.declaration &&
|
||||
exportPath.node.specifiers.length === 0)
|
||||
) {
|
||||
return
|
||||
}
|
||||
|
||||
const { declarations } = exportPath.node.declaration
|
||||
const config: PageConfig = {}
|
||||
const declarations = [
|
||||
...(exportPath.node.declaration?.declarations || []),
|
||||
exportPath.scope.getBinding(CONFIG_KEY)?.path.node,
|
||||
].filter(Boolean)
|
||||
|
||||
for (const declaration of declarations) {
|
||||
if (
|
||||
!BabelTypes.isIdentifier(declaration.id, { name: 'config' })
|
||||
!BabelTypes.isIdentifier(declaration.id, {
|
||||
name: CONFIG_KEY,
|
||||
})
|
||||
) {
|
||||
if (BabelTypes.isImportSpecifier(declaration)) {
|
||||
throw new Error(
|
||||
errorMessage(
|
||||
exportState,
|
||||
`Expected object but got import`
|
||||
)
|
||||
)
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
|
|
|
@ -12,6 +12,11 @@ export default function Nav() {
|
|||
<a id="only-amp-link">AMP First Page</a>
|
||||
</Link>
|
||||
</li>
|
||||
<li>
|
||||
<Link href="/var-before-export">
|
||||
<a id="var-before-export-link">Another AMP First Page</a>
|
||||
</Link>
|
||||
</li>
|
||||
</ul>
|
||||
)
|
||||
}
|
||||
|
|
12
test/integration/amphtml/pages/var-before-export.js
Normal file
12
test/integration/amphtml/pages/var-before-export.js
Normal file
|
@ -0,0 +1,12 @@
|
|||
const config = { amp: true }
|
||||
|
||||
const Page = () => {
|
||||
return (
|
||||
<div>
|
||||
<p id="var-before-export">Only AMP for me...</p>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
export default Page
|
||||
export { config }
|
|
@ -74,6 +74,15 @@ describe('AMP Usage', () => {
|
|||
expect(result).toBe(null)
|
||||
})
|
||||
|
||||
it('should not output client pages for AMP only with config exported after declaration', async () => {
|
||||
const browser = await webdriver(appPort, '/nav')
|
||||
await browser.elementByCss('#var-before-export-link').click()
|
||||
|
||||
const result = await browser.eval('window.NAV_PAGE_LOADED')
|
||||
|
||||
expect(result).toBe(null)
|
||||
})
|
||||
|
||||
it('should add link preload for amp script', async () => {
|
||||
const html = await renderViaHTTP(appPort, '/?amp=1')
|
||||
await validateAMP(html)
|
||||
|
@ -267,6 +276,15 @@ describe('AMP Usage', () => {
|
|||
|
||||
expect(result).toBe(null)
|
||||
})
|
||||
|
||||
it('should not output client pages for AMP only with config exported after declaration', async () => {
|
||||
const browser = await webdriver(appPort, '/nav')
|
||||
await browser.elementByCss('#var-before-export-link').click()
|
||||
|
||||
const result = await browser.eval('window.NAV_PAGE_LOADED')
|
||||
|
||||
expect(result).toBe(null)
|
||||
})
|
||||
})
|
||||
|
||||
describe('AMP dev no-warn', () => {
|
||||
|
|
|
@ -0,0 +1,3 @@
|
|||
// export { config } from '../../config'
|
||||
|
||||
export default () => <p>hello world</p>
|
|
@ -0,0 +1,6 @@
|
|||
// eslint-disable-next-line no-unused-vars
|
||||
import { config } from '../../config'
|
||||
|
||||
// export { config }
|
||||
|
||||
export default () => <p>hello world</p>
|
|
@ -2,4 +2,4 @@ const props = { amp: true }
|
|||
|
||||
// export const config = { ...props }
|
||||
|
||||
export default () => <p>{props}</p>
|
||||
export default () => <p>{JSON.stringify(props)}</p>
|
||||
|
|
|
@ -88,4 +88,30 @@ describe('Page Config', () => {
|
|||
await reset()
|
||||
}
|
||||
})
|
||||
|
||||
it('shows error when page config is export from', async () => {
|
||||
const reset = await uncommentExport('invalid/export-from.js')
|
||||
|
||||
try {
|
||||
const { stderr } = await nextBuild(appDir, undefined, { stderr: true })
|
||||
expect(stderr).toMatch(
|
||||
/https:\/\/err\.sh\/vercel\/next\.js\/invalid-page-config/
|
||||
)
|
||||
} finally {
|
||||
await reset()
|
||||
}
|
||||
})
|
||||
|
||||
it('shows error when page config is imported and exported', async () => {
|
||||
const reset = await uncommentExport('invalid/import-export.js')
|
||||
|
||||
try {
|
||||
const { stderr } = await nextBuild(appDir, undefined, { stderr: true })
|
||||
expect(stderr).toMatch(
|
||||
/https:\/\/err\.sh\/vercel\/next\.js\/invalid-page-config/
|
||||
)
|
||||
} finally {
|
||||
await reset()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
|
Loading…
Reference in a new issue