From 8eb9e788fcad8995b174a7d68b15b7b3e3f41947 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 30 Aug 2022 19:30:03 +0200 Subject: [PATCH 1/5] Disable ssr and ssg for layout client components --- .../next-flight-client-loader/index.ts | 41 +++++++---- .../next-flight-client-loader/module-proxy.ts | 19 +---- packages/next/server/app-render.tsx | 18 +---- .../get-server-side-props/page.client.js | 4 +- .../get-static-props/page.client.js | 4 +- test/e2e/app-dir/index.test.ts | 69 ++++++++++++------- 6 files changed, 79 insertions(+), 76 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..d6f4fea7c40f 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,48 @@ * 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 createError = (name: string) => + new Error( + `${name} is not supported in client components.\nFrom: ${this.resourcePath}` + ) + + if (isUnderAppDir) { + 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..81b5bc6f2c82 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. */ @@ -765,7 +751,7 @@ export async function renderToHTMLOrFlight( } // TODO-APP: pass a shared cache from previous getStaticProps/getServerSideProps calls? - if (!isClientComponentModule && layoutOrPageMod.getServerSideProps) { + if (layoutOrPageMod.getServerSideProps) { // TODO-APP: recommendation for i18n // locales: (renderOpts as any).locales, // always the same // locale: (renderOpts as any).locale, // /nl/something -> nl @@ -789,7 +775,7 @@ export async function renderToHTMLOrFlight( ) } // TODO-APP: implement layout specific caching for getStaticProps - if (!isClientComponentModule && layoutOrPageMod.getStaticProps) { + if (layoutOrPageMod.getStaticProps) { const getStaticPropsContext: | GetStaticPropsContext | GetStaticPropContextPage = { 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..89145c9fca2a 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,6 +1,4 @@ -export function getServerSideProps() { - return { props: {} } -} +// export function getServerSideProps() { { props: {} } } export default function Page() { return null 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..b3f840fb3fce 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,6 +1,4 @@ -export function getStaticProps() { - return { props: {} } -} +// export function getStaticProps() { return { props: {} }} export default function Page() { return null diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index e22823c3d79d..528ccd08ef81 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1023,31 +1023,52 @@ 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 getStaticProps is used', async () => { + const pageFile = + 'app/client-with-errors/get-static-props/page.client.js' + const content = await next.readFile(pageFile) + await next.patchFile( + pageFile, + content.replace( + '// export function getStaticProps', + 'export function getStaticProps' + ) + ) + const res = await fetchViaHTTP( + next.url, + '/client-with-errors/get-static-props' + ) + await next.patchFile(pageFile, content) + expect(res.status).toBe(500) + expect(await res.text()).toContain( + 'getStaticProps is not supported in client components' + ) + }) - 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' - ) - }) + 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) + await next.patchFile( + pageFile, + content.replace( + '// export function getServerSideProps', + 'export function getServerSideProps' + ) + ) + const res = await fetchViaHTTP( + next.url, + '/client-with-errors/get-server-side-props' + ) + await next.patchFile(pageFile, content) + + expect(res.status).toBe(500) + expect(await res.text()).toContain( + 'getServerSideProps is not supported in client components' + ) + }) + } }) describe('css support', () => { From 84bca3c2c301236b8c4b2a3d3d70625f7bc510e5 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 30 Aug 2022 19:36:43 +0200 Subject: [PATCH 2/5] only apply to page or layout files --- .../build/webpack/loaders/next-flight-client-loader/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 d6f4fea7c40f..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 @@ -24,13 +24,15 @@ export default async function transformSource( 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) { + if (isUnderAppDir && isPageOrLayoutFile) { const swcAST = await parse(source, { filename: this.resourcePath, isModule: 'unknown', From 27f3c0e29be80a834a848a73191ee887d75be94b Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 30 Aug 2022 19:48:07 +0200 Subject: [PATCH 3/5] keep the client component checking patch --- packages/next/server/app-render.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index 81b5bc6f2c82..b8ba02cc10f9 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -751,7 +751,7 @@ export async function renderToHTMLOrFlight( } // TODO-APP: pass a shared cache from previous getStaticProps/getServerSideProps calls? - if (layoutOrPageMod.getServerSideProps) { + if (!isClientComponentModule && layoutOrPageMod.getServerSideProps) { // TODO-APP: recommendation for i18n // locales: (renderOpts as any).locales, // always the same // locale: (renderOpts as any).locale, // /nl/something -> nl @@ -775,7 +775,7 @@ export async function renderToHTMLOrFlight( ) } // TODO-APP: implement layout specific caching for getStaticProps - if (layoutOrPageMod.getStaticProps) { + if (!isClientComponentModule && layoutOrPageMod.getStaticProps) { const getStaticPropsContext: | GetStaticPropsContext | GetStaticPropContextPage = { From b5dbe5b1eb66e13c2a31ed5052ee04d1a483d157 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 31 Aug 2022 01:19:47 +0200 Subject: [PATCH 4/5] fix flaky test --- .../get-server-side-props/page.client.js | 2 +- .../get-static-props/page.client.js | 2 +- test/e2e/app-dir/index.test.ts | 61 ++++++++++++------- 3 files changed, 42 insertions(+), 23 deletions(-) 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 89145c9fca2a..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,5 +1,5 @@ // 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 b3f840fb3fce..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,5 +1,5 @@ // 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 528ccd08ef81..488472843c6e 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1,6 +1,13 @@ 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, + getBrowserBodyText, + hasRedbox, + renderViaHTTP, + waitFor, +} from 'next-test-utils' import path from 'path' import cheerio from 'cheerio' import webdriver from 'next-webdriver' @@ -1024,48 +1031,60 @@ describe('app dir', () => { }) if (isDev) { - it('should throw an error when getStaticProps is used', async () => { + it('should throw an error when getServerSideProps is used', async () => { const pageFile = - 'app/client-with-errors/get-static-props/page.client.js' + 'app/client-with-errors/get-server-side-props/page.client.js' const content = await next.readFile(pageFile) - await next.patchFile( - pageFile, - content.replace( - '// export function getStaticProps', - 'export function getStaticProps' - ) + 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-static-props' + '/client-with-errors/get-server-side-props' ) await next.patchFile(pageFile, content) + + 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( - 'getStaticProps is not supported in client components' + 'getServerSideProps is not supported in client components' ) }) - it('should throw an error when getServerSideProps is used', async () => { + it('should throw an error when getStaticProps is used', async () => { const pageFile = - 'app/client-with-errors/get-server-side-props/page.client.js' + 'app/client-with-errors/get-static-props/page.client.js' const content = await next.readFile(pageFile) - await next.patchFile( - pageFile, - content.replace( - '// export function getServerSideProps', - 'export function getServerSideProps' - ) + 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-server-side-props' + '/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( - 'getServerSideProps is not supported in client components' + 'getStaticProps is not supported in client components' ) }) } From a616d2868a9a8636f0748034b4acec140e3b7fd0 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 30 Aug 2022 16:56:11 -0700 Subject: [PATCH 5/5] lint fix --- test/e2e/app-dir/index.test.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 488472843c6e..51d41c4931d9 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1,13 +1,6 @@ import { createNext, FileRef } from 'e2e-utils' import { NextInstance } from 'test/lib/next-modes/base' -import { - check, - fetchViaHTTP, - getBrowserBodyText, - hasRedbox, - 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'