Error for ssg and ssr exports from client components in build time (#…

…40106) Follow up for #39953 Detect invalid gSSP/gSP exports in page or layout client components in build time instead of checking them in runtime, in this way we can: * Error to user eariler with traced file path, help user find the incorrect usage easier * Make the flight client loader simpler, headless, aligned with react Co-authored-by: JJ Kasper <22380829+ijjk@users.noreply.github.com>
vercel · Aug 31, 2022 · 481950c · 481950c
1 parent 591f341
commit 481950c
Show file tree

Hide file tree

Showing 6 changed files with 94 additions and 77 deletions.
diff --git a/packages/next/build/webpack/loaders/next-flight-client-loader/index.ts b/packages/next/build/webpack/loaders/next-flight-client-loader/index.ts
@@ -5,31 +5,50 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+import path from 'path'
 import { checkExports } from '../../../analysis/get-page-static-info'
 import { parse } from '../../../swc'
 
+function containsPath(parent: string, child: string) {
+  const relation = path.relative(parent, child)
+  return !!relation && !relation.startsWith('..') && !path.isAbsolute(relation)
+}
+
 export default async function transformSource(
   this: any,
   source: string
 ): Promise<string> {
-  const { resourcePath } = this
-
-  const transformedSource = source
-  if (typeof transformedSource !== 'string') {
+  if (typeof source !== 'string') {
     throw new Error('Expected source to have been transformed to a string.')
   }
 
-  const swcAST = await parse(transformedSource, {
-    filename: resourcePath,
-    isModule: 'unknown',
-  })
-  const { ssg, ssr } = checkExports(swcAST)
+  const appDir = path.join(this.rootContext, 'app')
+  const isUnderAppDir = containsPath(appDir, this.resourcePath)
+  const filename = path.basename(this.resourcePath)
+  const isPageOrLayoutFile = /^(page|layout)\.client\.\w+$/.test(filename)
+
+  const createError = (name: string) =>
+    new Error(
+      `${name} is not supported in client components.\nFrom: ${this.resourcePath}`
+    )
+
+  if (isUnderAppDir && isPageOrLayoutFile) {
+    const swcAST = await parse(source, {
+      filename: this.resourcePath,
+      isModule: 'unknown',
+    })
+    const { ssg, ssr } = checkExports(swcAST)
+    if (ssg) {
+      this.emitError(createError('getStaticProps'))
+    }
+    if (ssr) {
+      this.emitError(createError('getServerSideProps'))
+    }
+  }
 
   const output = `
 const { createProxy } = require("next/dist/build/webpack/loaders/next-flight-client-loader/module-proxy")\n
-module.exports = createProxy(${JSON.stringify(
-    resourcePath
-  )}, { ssr: ${ssr}, ssg: ${ssg} })
+module.exports = createProxy(${JSON.stringify(this.resourcePath)})
 `
   return output
 }
diff --git a/packages/next/build/webpack/loaders/next-flight-client-loader/module-proxy.ts b/packages/next/build/webpack/loaders/next-flight-client-loader/module-proxy.ts
@@ -38,9 +38,6 @@ const proxyHandlers: ProxyHandler<object> = {
           // whole object or just the default export.
           name: '',
           async: target.async,
-
-          ssr: target.ssr,
-          ssg: target.ssg,
         }
         return true
       case 'then':
@@ -57,9 +54,6 @@ const proxyHandlers: ProxyHandler<object> = {
               filepath: target.filepath,
               name: '*', // Represents the whole object instead of a particular import.
               async: true,
-
-              ssr: target.ssr,
-              ssg: target.ssg,
             }
             return Promise.resolve(
               resolve(new Proxy(moduleReference, proxyHandlers))
@@ -74,11 +68,6 @@ const proxyHandlers: ProxyHandler<object> = {
           return then
         }
         break
-
-      case 'ssg':
-        return target.ssg
-      case 'ssr':
-        return target.ssr
       default:
         break
     }
@@ -102,18 +91,12 @@ const proxyHandlers: ProxyHandler<object> = {
   },
 }
 
-export function createProxy(
-  moduleId: string,
-  { ssr, ssg }: { ssr: boolean; ssg: boolean }
-) {
+export function createProxy(moduleId: string) {
   const moduleReference = {
     $$typeof: MODULE_REFERENCE,
     filepath: moduleId,
     name: '*', // Represents the whole object instead of a particular import.
     async: false,
-
-    ssr,
-    ssg,
   }
   return new Proxy(moduleReference, proxyHandlers)
 }
diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx
@@ -639,20 +639,6 @@ export async function renderToHTMLOrFlight(
     const isClientComponentModule =
       layoutOrPageMod && !layoutOrPageMod.hasOwnProperty('__next_rsc__')
 
-    // Only server components can have getServerSideProps / getStaticProps
-    // TODO-APP: friendly error with correct stacktrace. Potentially this can be part of the compiler instead.
-    if (isClientComponentModule) {
-      if (layoutOrPageMod.ssr) {
-        throw new Error(
-          'getServerSideProps is not supported on Client Components'
-        )
-      }
-
-      if (layoutOrPageMod.ssg) {
-        throw new Error('getStaticProps is not supported on Client Components')
-      }
-    }
-
     /**
      * The React Component to render.
      */

diff --git a/test/e2e/app-dir/app/app/client-with-errors/get-server-side-props/page.client.js b/test/e2e/app-dir/app/app/client-with-errors/get-server-side-props/page.client.js
@@ -1,7 +1,5 @@
-export function getServerSideProps() {
-  return { props: {} }
-}
+// export function getServerSideProps() { { props: {} } }
 
 export default function Page() {
-  return null
+  return 'client-gssp'
 }
diff --git a/test/e2e/app-dir/app/app/client-with-errors/get-static-props/page.client.js b/test/e2e/app-dir/app/app/client-with-errors/get-static-props/page.client.js
@@ -1,7 +1,5 @@
-export function getStaticProps() {
-  return { props: {} }
-}
+// export function getStaticProps() { return { props: {} }}
 
 export default function Page() {
-  return null
+  return 'client-gsp'
 }
diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts
@@ -1,6 +1,6 @@
 import { createNext, FileRef } from 'e2e-utils'
 import { NextInstance } from 'test/lib/next-modes/base'
-import { fetchViaHTTP, renderViaHTTP, waitFor } from 'next-test-utils'
+import { check, fetchViaHTTP, renderViaHTTP, waitFor } from 'next-test-utils'
 import path from 'path'
 import cheerio from 'cheerio'
 import webdriver from 'next-webdriver'
@@ -1023,31 +1023,64 @@ describe('app dir', () => {
         })
       })
 
-      it('should throw an error when getStaticProps is used', async () => {
-        const res = await fetchViaHTTP(
-          next.url,
-          '/client-with-errors/get-static-props'
-        )
-        expect(res.status).toBe(500)
-        expect(await res.text()).toContain(
-          isDev
-            ? 'getStaticProps is not supported on Client Components'
-            : 'Internal Server Error'
-        )
-      })
+      if (isDev) {
+        it('should throw an error when getServerSideProps is used', async () => {
+          const pageFile =
+            'app/client-with-errors/get-server-side-props/page.client.js'
+          const content = await next.readFile(pageFile)
+          const uncomment = content.replace(
+            '// export function getServerSideProps',
+            'export function getServerSideProps'
+          )
+          await next.patchFile(pageFile, uncomment)
+          const res = await fetchViaHTTP(
+            next.url,
+            '/client-with-errors/get-server-side-props'
+          )
+          await next.patchFile(pageFile, content)
 
-      it('should throw an error when getServerSideProps is used', async () => {
-        const res = await fetchViaHTTP(
-          next.url,
-          '/client-with-errors/get-server-side-props'
-        )
-        expect(res.status).toBe(500)
-        expect(await res.text()).toContain(
-          isDev
-            ? 'getServerSideProps is not supported on Client Components'
-            : 'Internal Server Error'
-        )
-      })
+          await check(async () => {
+            const { status } = await fetchViaHTTP(
+              next.url,
+              '/client-with-errors/get-server-side-props'
+            )
+            return status
+          }, /200/)
+
+          expect(res.status).toBe(500)
+          expect(await res.text()).toContain(
+            'getServerSideProps is not supported in client components'
+          )
+        })
+
+        it('should throw an error when getStaticProps is used', async () => {
+          const pageFile =
+            'app/client-with-errors/get-static-props/page.client.js'
+          const content = await next.readFile(pageFile)
+          const uncomment = content.replace(
+            '// export function getStaticProps',
+            'export function getStaticProps'
+          )
+          await next.patchFile(pageFile, uncomment)
+          const res = await fetchViaHTTP(
+            next.url,
+            '/client-with-errors/get-static-props'
+          )
+          await next.patchFile(pageFile, content)
+          await check(async () => {
+            const { status } = await fetchViaHTTP(
+              next.url,
+              '/client-with-errors/get-static-props'
+            )
+            return status
+          }, /200/)
+
+          expect(res.status).toBe(500)
+          expect(await res.text()).toContain(
+            'getStaticProps is not supported in client components'
+          )
+        })
+      }
     })
 
     describe('css support', () => {