Skip to content

Commit 481950c

Browse files
huozhiijjk
huozhi
and
ijjk
authored
Error for ssg and ssr exports from client components in build time (vercel#40106)
Follow up for vercel#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 <[email protected]>
1 parent 591f341 commit 481950c

File tree

6 files changed

+94
-77
lines changed

6 files changed

+94
-77
lines changed

‎packages/next/build/webpack/loaders/next-flight-client-loader/index.ts‎

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,31 +5,50 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8+
import path from 'path'
89
import { checkExports } from '../../../analysis/get-page-static-info'
910
import { parse } from '../../../swc'
1011

12+
function containsPath(parent: string, child: string) {
13+
const relation = path.relative(parent, child)
14+
return !!relation && !relation.startsWith('..') && !path.isAbsolute(relation)
15+
}
16+
1117
export default async function transformSource(
1218
this: any,
1319
source: string
1420
): Promise<string> {
15-
const { resourcePath } = this
16-
17-
const transformedSource = source
18-
if (typeof transformedSource !== 'string') {
21+
if (typeof source !== 'string') {
1922
throw new Error('Expected source to have been transformed to a string.')
2023
}
2124

22-
const swcAST = await parse(transformedSource, {
23-
filename: resourcePath,
24-
isModule: 'unknown',
25-
})
26-
const { ssg, ssr } = checkExports(swcAST)
25+
const appDir = path.join(this.rootContext, 'app')
26+
const isUnderAppDir = containsPath(appDir, this.resourcePath)
27+
const filename = path.basename(this.resourcePath)
28+
const isPageOrLayoutFile = /^(page|layout)\.client\.\w+$/.test(filename)
29+
30+
const createError = (name: string) =>
31+
new Error(
32+
`${name} is not supported in client components.\nFrom: ${this.resourcePath}`
33+
)
34+
35+
if (isUnderAppDir && isPageOrLayoutFile) {
36+
const swcAST = await parse(source, {
37+
filename: this.resourcePath,
38+
isModule: 'unknown',
39+
})
40+
const { ssg, ssr } = checkExports(swcAST)
41+
if (ssg) {
42+
this.emitError(createError('getStaticProps'))
43+
}
44+
if (ssr) {
45+
this.emitError(createError('getServerSideProps'))
46+
}
47+
}
2748

2849
const output = `
2950
const { createProxy } = require("next/dist/build/webpack/loaders/next-flight-client-loader/module-proxy")\n
30-
module.exports = createProxy(${JSON.stringify(
31-
resourcePath
32-
)}, { ssr: ${ssr}, ssg: ${ssg} })
51+
module.exports = createProxy(${JSON.stringify(this.resourcePath)})
3352
`
3453
return output
3554
}

‎packages/next/build/webpack/loaders/next-flight-client-loader/module-proxy.ts‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,6 @@ const proxyHandlers: ProxyHandler<object> = {
3838
// whole object or just the default export.
3939
name: '',
4040
async: target.async,
41-
42-
ssr: target.ssr,
43-
ssg: target.ssg,
4441
}
4542
return true
4643
case 'then':
@@ -57,9 +54,6 @@ const proxyHandlers: ProxyHandler<object> = {
5754
filepath: target.filepath,
5855
name: '*', // Represents the whole object instead of a particular import.
5956
async: true,
60-
61-
ssr: target.ssr,
62-
ssg: target.ssg,
6357
}
6458
return Promise.resolve(
6559
resolve(new Proxy(moduleReference, proxyHandlers))
@@ -74,11 +68,6 @@ const proxyHandlers: ProxyHandler<object> = {
7468
return then
7569
}
7670
break
77-
78-
case 'ssg':
79-
return target.ssg
80-
case 'ssr':
81-
return target.ssr
8271
default:
8372
break
8473
}
@@ -102,18 +91,12 @@ const proxyHandlers: ProxyHandler<object> = {
10291
},
10392
}
10493

105-
export function createProxy(
106-
moduleId: string,
107-
{ ssr, ssg }: { ssr: boolean; ssg: boolean }
108-
) {
94+
export function createProxy(moduleId: string) {
10995
const moduleReference = {
11096
$$typeof: MODULE_REFERENCE,
11197
filepath: moduleId,
11298
name: '*', // Represents the whole object instead of a particular import.
11399
async: false,
114-
115-
ssr,
116-
ssg,
117100
}
118101
return new Proxy(moduleReference, proxyHandlers)
119102
}

‎packages/next/server/app-render.tsx‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -639,20 +639,6 @@ export async function renderToHTMLOrFlight(
639639
const isClientComponentModule =
640640
layoutOrPageMod && !layoutOrPageMod.hasOwnProperty('__next_rsc__')
641641

642-
// Only server components can have getServerSideProps / getStaticProps
643-
// TODO-APP: friendly error with correct stacktrace. Potentially this can be part of the compiler instead.
644-
if (isClientComponentModule) {
645-
if (layoutOrPageMod.ssr) {
646-
throw new Error(
647-
'getServerSideProps is not supported on Client Components'
648-
)
649-
}
650-
651-
if (layoutOrPageMod.ssg) {
652-
throw new Error('getStaticProps is not supported on Client Components')
653-
}
654-
}
655-
656642
/**
657643
* The React Component to render.
658644
*/

‎test/e2e/app-dir/app/app/client-with-errors/get-server-side-props/page.client.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
export function getServerSideProps() {
2-
return { props: {} }
3-
}
1+
// export function getServerSideProps() { { props: {} } }
42

53
export default function Page() {
6-
return null
4+
return 'client-gssp'
75
}

‎test/e2e/app-dir/app/app/client-with-errors/get-static-props/page.client.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
export function getStaticProps() {
2-
return { props: {} }
3-
}
1+
// export function getStaticProps() { return { props: {} }}
42

53
export default function Page() {
6-
return null
4+
return 'client-gsp'
75
}

‎test/e2e/app-dir/index.test.ts‎

Lines changed: 58 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { createNext, FileRef } from 'e2e-utils'
22
import { NextInstance } from 'test/lib/next-modes/base'
3-
import { fetchViaHTTP, renderViaHTTP, waitFor } from 'next-test-utils'
3+
import { check, fetchViaHTTP, renderViaHTTP, waitFor } from 'next-test-utils'
44
import path from 'path'
55
import cheerio from 'cheerio'
66
import webdriver from 'next-webdriver'
@@ -1023,31 +1023,64 @@ describe('app dir', () => {
10231023
})
10241024
})
10251025

1026-
it('should throw an error when getStaticProps is used', async () => {
1027-
const res = await fetchViaHTTP(
1028-
next.url,
1029-
'/client-with-errors/get-static-props'
1030-
)
1031-
expect(res.status).toBe(500)
1032-
expect(await res.text()).toContain(
1033-
isDev
1034-
? 'getStaticProps is not supported on Client Components'
1035-
: 'Internal Server Error'
1036-
)
1037-
})
1026+
if (isDev) {
1027+
it('should throw an error when getServerSideProps is used', async () => {
1028+
const pageFile =
1029+
'app/client-with-errors/get-server-side-props/page.client.js'
1030+
const content = await next.readFile(pageFile)
1031+
const uncomment = content.replace(
1032+
'// export function getServerSideProps',
1033+
'export function getServerSideProps'
1034+
)
1035+
await next.patchFile(pageFile, uncomment)
1036+
const res = await fetchViaHTTP(
1037+
next.url,
1038+
'/client-with-errors/get-server-side-props'
1039+
)
1040+
await next.patchFile(pageFile, content)
10381041

1039-
it('should throw an error when getServerSideProps is used', async () => {
1040-
const res = await fetchViaHTTP(
1041-
next.url,
1042-
'/client-with-errors/get-server-side-props'
1043-
)
1044-
expect(res.status).toBe(500)
1045-
expect(await res.text()).toContain(
1046-
isDev
1047-
? 'getServerSideProps is not supported on Client Components'
1048-
: 'Internal Server Error'
1049-
)
1050-
})
1042+
await check(async () => {
1043+
const { status } = await fetchViaHTTP(
1044+
next.url,
1045+
'/client-with-errors/get-server-side-props'
1046+
)
1047+
return status
1048+
}, /200/)
1049+
1050+
expect(res.status).toBe(500)
1051+
expect(await res.text()).toContain(
1052+
'getServerSideProps is not supported in client components'
1053+
)
1054+
})
1055+
1056+
it('should throw an error when getStaticProps is used', async () => {
1057+
const pageFile =
1058+
'app/client-with-errors/get-static-props/page.client.js'
1059+
const content = await next.readFile(pageFile)
1060+
const uncomment = content.replace(
1061+
'// export function getStaticProps',
1062+
'export function getStaticProps'
1063+
)
1064+
await next.patchFile(pageFile, uncomment)
1065+
const res = await fetchViaHTTP(
1066+
next.url,
1067+
'/client-with-errors/get-static-props'
1068+
)
1069+
await next.patchFile(pageFile, content)
1070+
await check(async () => {
1071+
const { status } = await fetchViaHTTP(
1072+
next.url,
1073+
'/client-with-errors/get-static-props'
1074+
)
1075+
return status
1076+
}, /200/)
1077+
1078+
expect(res.status).toBe(500)
1079+
expect(await res.text()).toContain(
1080+
'getStaticProps is not supported in client components'
1081+
)
1082+
})
1083+
}
10511084
})
10521085

10531086
describe('css support', () => {

0 commit comments

Comments
 (0)