From 481950c34b9c7b2eeb051eb4dc966d0ed3bb9605 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 31 Aug 2022 15:13:47 +0200 Subject: [PATCH] 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> --- .../next-flight-client-loader/index.ts | 43 +++++++--- .../next-flight-client-loader/module-proxy.ts | 19 +---- packages/next/server/app-render.tsx | 14 ---- .../get-server-side-props/page.client.js | 6 +- .../get-static-props/page.client.js | 6 +- test/e2e/app-dir/index.test.ts | 83 +++++++++++++------ 6 files changed, 94 insertions(+), 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 index d761f8f7df8f..6c27a5a49f2d 100644 --- 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 { - 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 index 80f7edf54b9b..4d9f6bcad03d 100644 --- 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 = { // 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 = { 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 = { return then } break - - case 'ssg': - return target.ssg - case 'ssr': - return target.ssr default: break } @@ -102,18 +91,12 @@ const proxyHandlers: ProxyHandler = { }, } -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 index c27393ec3a8c..b8ba02cc10f9 100644 --- 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 index 8b4ae46ca94b..899f4d9b0697 100644 --- 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 index 5acfaee644a8..717782b1a38e 100644 --- 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 index e22823c3d79d..51d41c4931d9 100644 --- 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', () => {