Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can get the specific line number getStaticProps/getServerSideProps is at it would be nice if we could update the stack for the Error created here so the dev overlay can show the file/code

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't get the line number from swc at the moment, so I only give the file path there at least user can search themselves. Will keep this in mind to see if we can optimize it later in the future

`${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
}
Expand Up @@ -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':
Expand All @@ -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))
Expand All @@ -74,11 +68,6 @@ const proxyHandlers: ProxyHandler<object> = {
return then
}
break

case 'ssg':
return target.ssg
case 'ssr':
return target.ssr
default:
break
}
Expand All @@ -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)
}
14 changes: 0 additions & 14 deletions packages/next/server/app-render.tsx
Expand Up @@ -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.
*/
Expand Down
@@ -1,7 +1,5 @@
export function getServerSideProps() {
return { props: {} }
}
// export function getServerSideProps() { { props: {} } }

export default function Page() {
return null
return 'client-gssp'
}
@@ -1,7 +1,5 @@
export function getStaticProps() {
return { props: {} }
}
// export function getStaticProps() { return { props: {} }}

export default function Page() {
return null
return 'client-gsp'
}
83 changes: 58 additions & 25 deletions 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'
Expand Down Expand Up @@ -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', () => {
Expand Down