Handle cases where config is exported after its declaration (vercel…

…#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 vercel#15704.

errors/invalid-page-config.md

@@ -6,14 +6,27 @@ 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
 
 ```js
 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

packages/next/build/babel/plugins/next-page-config.ts

@@ -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
                   }
 

test/integration/amphtml/pages/nav.js

@@ -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>
   )
 }

test/integration/amphtml/pages/var-before-export.js

@@ -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 }

test/integration/amphtml/test/index.test.js

@@ -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', () => {

test/integration/page-config/pages/invalid/export-from.js

@@ -0,0 +1,3 @@
+// export { config } from '../../config'
+
+export default () => <p>hello world</p>

test/integration/page-config/pages/invalid/import-export.js

@@ -0,0 +1,6 @@
+// eslint-disable-next-line no-unused-vars
+import { config } from '../../config'
+
+// export { config }
+
+export default () => <p>hello world</p>

test/integration/page-config/pages/invalid/spread-config.js

@@ -2,4 +2,4 @@ const props = { amp: true }
 
 // export const config = { ...props }
 
-export default () => <p>{props}</p>
+export default () => <p>{JSON.stringify(props)}</p>

test/integration/page-config/test/index.test.js

@@ -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()
+    }
+  })
 })