From f3d56614d3fff703941eb2bcf9e7e86cd4f99163 Mon Sep 17 00:00:00 2001
From: Jan Potoms <2109932+Janpot@users.noreply.github.com>
Date: Thu, 6 Feb 2020 07:54:19 +0100
Subject: [PATCH 1/2] Remove any type and fix edge cases

Removed the "as any" and use the @babel/types typeguards instead. This revealed some edge cases that would just error.
---
 .../build/babel/plugins/next-page-config.ts   | 25 ++++++++---
 test/integration/page-config/pages/index.js   |  2 -
 .../page-config/pages/invalid/no-init.js      |  3 ++
 .../pages/invalid/spread-config.js            |  5 +++
 .../pages/invalid/string-config.js            |  3 ++
 .../page-config/test/index.test.js            | 45 ++++++++++++++++---
 6 files changed, 68 insertions(+), 15 deletions(-)
 create mode 100644 test/integration/page-config/pages/invalid/no-init.js
 create mode 100644 test/integration/page-config/pages/invalid/spread-config.js
 create mode 100644 test/integration/page-config/pages/invalid/string-config.js

diff --git a/packages/next/build/babel/plugins/next-page-config.ts b/packages/next/build/babel/plugins/next-page-config.ts
index 9fb733db40a1643..a2ce281e476d44a 100644
--- a/packages/next/build/babel/plugins/next-page-config.ts
+++ b/packages/next/build/babel/plugins/next-page-config.ts
@@ -50,28 +50,39 @@ export default function nextPageConfig({
                   return
                 }
 
-                const { declarations } = path.node.declaration as any
-                const config: PageConfig = {}
-
-                if (!declarations) {
+                if (!BabelTypes.isVariableDeclaration(path.node.declaration)) {
                   return
                 }
+
+                const { declarations } = path.node.declaration
+                const config: PageConfig = {}
+
                 for (const declaration of declarations) {
-                  if (declaration.id.name !== 'config') {
+                  if (
+                    !BabelTypes.isIdentifier(declaration.id, { name: 'config' })
+                  ) {
                     continue
                   }
 
-                  if (declaration.init.type !== 'ObjectExpression') {
+                  if (!BabelTypes.isObjectExpression(declaration.init)) {
                     const pageName =
                       (state.filename || '').split(state.cwd || '').pop() ||
                       'unknown'
 
+                    const got = declaration.init
+                      ? declaration.init.type
+                      : 'undefined'
                     throw new Error(
-                      `Invalid page config export found. Expected object but got ${declaration.init.type} in file ${pageName}. See: https://err.sh/zeit/next.js/invalid-page-config`
+                      `Invalid page config export found. Expected object but got ${got} in file ${pageName}. See: https://err.sh/zeit/next.js/invalid-page-config`
                     )
                   }
 
                   for (const prop of declaration.init.properties) {
+                    if (BabelTypes.isSpreadElement(prop)) {
+                      throw new Error(
+                        `Invalid page config export found. Property spread is not supported.`
+                      )
+                    }
                     const { name } = prop.key
                     if (configKeys.has(name)) {
                       // @ts-ignore
diff --git a/test/integration/page-config/pages/index.js b/test/integration/page-config/pages/index.js
index 8f84e202bb551e1..7daea25f7d90018 100644
--- a/test/integration/page-config/pages/index.js
+++ b/test/integration/page-config/pages/index.js
@@ -1,8 +1,6 @@
 import { config as hello } from '../something'
 import { config as world } from '../config'
 
-// export const config = 'hello world'
-
 export default () => (
   <p>
     {hello} {world}
diff --git a/test/integration/page-config/pages/invalid/no-init.js b/test/integration/page-config/pages/invalid/no-init.js
new file mode 100644
index 000000000000000..fc1b6e41d4f0022
--- /dev/null
+++ b/test/integration/page-config/pages/invalid/no-init.js
@@ -0,0 +1,3 @@
+// export let config
+
+export default () => <p>hello world</p>
diff --git a/test/integration/page-config/pages/invalid/spread-config.js b/test/integration/page-config/pages/invalid/spread-config.js
new file mode 100644
index 000000000000000..93e2e73dda7cfcc
--- /dev/null
+++ b/test/integration/page-config/pages/invalid/spread-config.js
@@ -0,0 +1,5 @@
+const props = { amp: true }
+
+// export const config = { ...props }
+
+export default () => <p>{props}</p>
diff --git a/test/integration/page-config/pages/invalid/string-config.js b/test/integration/page-config/pages/invalid/string-config.js
new file mode 100644
index 000000000000000..c3d4d570d0ee41d
--- /dev/null
+++ b/test/integration/page-config/pages/invalid/string-config.js
@@ -0,0 +1,3 @@
+// export const config = 'hello world'
+
+export default () => <p>hello world</p>
diff --git a/test/integration/page-config/test/index.test.js b/test/integration/page-config/test/index.test.js
index 7adb5316169ba67..f85b0747a16a89a 100644
--- a/test/integration/page-config/test/index.test.js
+++ b/test/integration/page-config/test/index.test.js
@@ -5,20 +5,40 @@ import { join } from 'path'
 import { nextBuild } from 'next-test-utils'
 
 const appDir = join(__dirname, '..')
-const indexPage = join(appDir, 'pages/index.js')
 
 jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2
 
+async function uncommentExport(page) {
+  const pagePath = join(appDir, 'pages', page)
+  const origContent = await fs.readFile(pagePath, 'utf8')
+  const newContent = origContent.replace('// export', 'export')
+  await fs.writeFile(pagePath, newContent, 'utf8')
+  return async () => {
+    await fs.writeFile(pagePath, origContent, 'utf8')
+  }
+}
+
 describe('Page Config', () => {
   it('builds without error when export const config is used outside page', async () => {
     const { stderr } = await nextBuild(appDir, undefined, { stderr: true })
     expect(stderr).not.toMatch(/Failed to compile\./)
   })
 
-  it('shows valid error on invalid page config', async () => {
-    const origContent = await fs.readFile(indexPage, 'utf8')
-    const newContent = origContent.replace('// export', 'export')
-    await fs.writeFile(indexPage, newContent, 'utf8')
+  it('shows valid error when page config is a string', async () => {
+    const reset = await uncommentExport('invalid/string-config.js')
+
+    try {
+      const { stderr } = await nextBuild(appDir, undefined, { stderr: true })
+      expect(stderr).toMatch(
+        /https:\/\/err\.sh\/zeit\/next\.js\/invalid-page-config/
+      )
+    } finally {
+      await reset()
+    }
+  })
+
+  it('shows valid error when page config has no init', async () => {
+    const reset = await uncommentExport('invalid/no-init.js')
 
     try {
       const { stderr } = await nextBuild(appDir, undefined, { stderr: true })
@@ -26,7 +46,20 @@ describe('Page Config', () => {
         /https:\/\/err\.sh\/zeit\/next\.js\/invalid-page-config/
       )
     } finally {
-      await fs.writeFile(indexPage, origContent, 'utf8')
+      await reset()
+    }
+  })
+
+  it('shows error when page config has spread properties', async () => {
+    const reset = await uncommentExport('invalid/spread-config.js')
+
+    try {
+      const { stderr } = await nextBuild(appDir, undefined, { stderr: true })
+      expect(stderr).toMatch(
+        /Invalid page config export found\. Property spread is not supported\./
+      )
+    } finally {
+      await reset()
     }
   })
 })

From 5afce7fd4f0c0fceb4cb7c29719d82b59bd50e27 Mon Sep 17 00:00:00 2001
From: Jan Potoms <2109932+Janpot@users.noreply.github.com>
Date: Fri, 7 Feb 2020 07:31:08 +0100
Subject: [PATCH 2/2] Remove ts-ignore

---
 .../build/babel/plugins/next-page-config.ts   | 33 +++++++++++++------
 .../pages/invalid/invalid-property.js         |  3 ++
 .../pages/invalid/invalid-value.js            |  5 +++
 .../page-config/test/index.test.js            | 28 +++++++++++++++-
 4 files changed, 58 insertions(+), 11 deletions(-)
 create mode 100644 test/integration/page-config/pages/invalid/invalid-property.js
 create mode 100644 test/integration/page-config/pages/invalid/invalid-value.js

diff --git a/packages/next/build/babel/plugins/next-page-config.ts b/packages/next/build/babel/plugins/next-page-config.ts
index a2ce281e476d44a..c968e15c73a9455 100644
--- a/packages/next/build/babel/plugins/next-page-config.ts
+++ b/packages/next/build/babel/plugins/next-page-config.ts
@@ -2,7 +2,6 @@ import { NodePath, PluginObj } from '@babel/core'
 import * as BabelTypes from '@babel/types'
 import { PageConfig } from 'next/types'
 
-const configKeys = new Set(['amp'])
 const STRING_LITERAL_DROP_BUNDLE = '__NEXT_DROP_CLIENT_FILE__'
 
 // replace program path with just a variable with the drop identifier
@@ -26,6 +25,12 @@ function replaceBundle(path: any, t: typeof BabelTypes) {
   )
 }
 
+function errorMessage(state: any, details: string) {
+  const pageName =
+    (state.filename || '').split(state.cwd || '').pop() || 'unknown'
+  return `Invalid page config export found. ${details} in file ${pageName}. See: https://err.sh/zeit/next.js/invalid-page-config`
+}
+
 interface ConfigState {
   bundleDropped?: boolean
 }
@@ -65,28 +70,36 @@ export default function nextPageConfig({
                   }
 
                   if (!BabelTypes.isObjectExpression(declaration.init)) {
-                    const pageName =
-                      (state.filename || '').split(state.cwd || '').pop() ||
-                      'unknown'
-
                     const got = declaration.init
                       ? declaration.init.type
                       : 'undefined'
                     throw new Error(
-                      `Invalid page config export found. Expected object but got ${got} in file ${pageName}. See: https://err.sh/zeit/next.js/invalid-page-config`
+                      errorMessage(state, `Expected object but got ${got}`)
                     )
                   }
 
                   for (const prop of declaration.init.properties) {
                     if (BabelTypes.isSpreadElement(prop)) {
                       throw new Error(
-                        `Invalid page config export found. Property spread is not supported.`
+                        errorMessage(state, `Property spread is not allowed`)
                       )
                     }
                     const { name } = prop.key
-                    if (configKeys.has(name)) {
-                      // @ts-ignore
-                      config[name] = prop.value.value
+                    if (BabelTypes.isIdentifier(prop.key, { name: 'amp' })) {
+                      if (!BabelTypes.isObjectProperty(prop)) {
+                        throw new Error(
+                          errorMessage(state, `Invalid property "${name}"`)
+                        )
+                      }
+                      if (
+                        !BabelTypes.isBooleanLiteral(prop.value) &&
+                        !BabelTypes.isStringLiteral(prop.value)
+                      ) {
+                        throw new Error(
+                          errorMessage(state, `Invalid value for "${name}"`)
+                        )
+                      }
+                      config.amp = prop.value.value as PageConfig['amp']
                     }
                   }
                 }
diff --git a/test/integration/page-config/pages/invalid/invalid-property.js b/test/integration/page-config/pages/invalid/invalid-property.js
new file mode 100644
index 000000000000000..fae2214867d3fe6
--- /dev/null
+++ b/test/integration/page-config/pages/invalid/invalid-property.js
@@ -0,0 +1,3 @@
+// export const config = { amp() {} }
+
+export default () => <p>hello world</p>
diff --git a/test/integration/page-config/pages/invalid/invalid-value.js b/test/integration/page-config/pages/invalid/invalid-value.js
new file mode 100644
index 000000000000000..10fcd812d31fb7f
--- /dev/null
+++ b/test/integration/page-config/pages/invalid/invalid-value.js
@@ -0,0 +1,5 @@
+const amp = 'hybrid'
+
+// export const config = { amp }
+
+export default () => <p>hello ${amp}</p>
diff --git a/test/integration/page-config/test/index.test.js b/test/integration/page-config/test/index.test.js
index f85b0747a16a89a..f4aca874c052ea7 100644
--- a/test/integration/page-config/test/index.test.js
+++ b/test/integration/page-config/test/index.test.js
@@ -56,7 +56,33 @@ describe('Page Config', () => {
     try {
       const { stderr } = await nextBuild(appDir, undefined, { stderr: true })
       expect(stderr).toMatch(
-        /Invalid page config export found\. Property spread is not supported\./
+        /https:\/\/err\.sh\/zeit\/next\.js\/invalid-page-config/
+      )
+    } finally {
+      await reset()
+    }
+  })
+
+  it('shows error when page config has invalid properties', async () => {
+    const reset = await uncommentExport('invalid/invalid-property.js')
+
+    try {
+      const { stderr } = await nextBuild(appDir, undefined, { stderr: true })
+      expect(stderr).toMatch(
+        /https:\/\/err\.sh\/zeit\/next\.js\/invalid-page-config/
+      )
+    } finally {
+      await reset()
+    }
+  })
+
+  it('shows error when page config has invalid property value', async () => {
+    const reset = await uncommentExport('invalid/invalid-value.js')
+
+    try {
+      const { stderr } = await nextBuild(appDir, undefined, { stderr: true })
+      expect(stderr).toMatch(
+        /https:\/\/err\.sh\/zeit\/next\.js\/invalid-page-config/
       )
     } finally {
       await reset()