fix: Add stricter check for "use server" exports (#62821)

As mentioned in the new-added error messages, and the [linked
resources](https://react.dev/reference/react/use-server#:~:text=Because%20the%20underlying%20network%20calls%20are%20always%20asynchronous%2C%20%27use%20server%27%20can%20only%20be%20used%20on%20async%20functions.):

> Because the underlying network calls are always asynchronous, 'use
server' can only be used on async functions.
> https://react.dev/reference/react/use-server

It's a requirement that only async functions are allowed to be exported
and annotated with `'use server'`. Currently, we already have compiler
check so this will already error:

```js
'use server'

export function foo () {} // missing async
```

However, since exported values can be very dynamic the compiler can't
catch all mistakes like that. We also have a runtime check for all
exports in a `'use server'` function, but it only covers `typeof value
=== 'function'`.

This PR adds a stricter check for "use server" annotated values to also
make sure they're async functions (`value.constructor.name ===
'AsyncFunction'`).

That said, there are still cases like synchronously returning a promise
to make a function "async", but it's still very different by definition.
For example:

```js
const f = async () => { throw 1; return 1 }
const g = () => { throw 1; return Promise.resolve(1) }
```

Where `g()` can be synchronously caught (`try { g() } catch {}`) but
`f()` can't even if they have the same types. If we allow `g` to be a
Server Action, this behavior is no longer always true but depending on
where it's called (server or client).

Closes #62727.
This commit is contained in:
Shu Ding 2024-03-04 18:50:19 +01:00 committed by GitHub
parent b80d388032
commit 7b2b982343
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 127 additions and 2 deletions

View file

@ -0,0 +1,43 @@
---
title: 'Invalid "use server" Value'
---
## Why This Error Occurred
This error occurs when a `"use server"` file exports a value that is not an async function. It might happen when you unintentionally export something like a configuration object, an arbitrary value, or missed the `async` keyword in the exported function declaration.
These functions are required to be defined as async, because `"use server"` marks them as [Server Actions](https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions-and-mutations) and they can be invoked directly from the client through a network request.
Examples of incorrect code:
```js
'use server'
// ❌ This is incorrect: only async functions are allowed.
export const value = 1
// ❌ This is incorrect: missed the `async` keyword.
export function getServerData() {
return '...'
}
```
Correct code:
```js
'use server'
// ✅ This is correct: an async function is exported.
export async function getServerData() {
return '...'
}
```
## Possible Ways to Fix It
Check all exported values in the `"use server"` file (including `export *`) and make sure that they are all defined as async functions.
## Useful Links
- [Server Actions and Mutations - Next.js](https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions-and-mutations)
- ['use server' directive - React](https://react.dev/reference/react/use-server)

View file

@ -6,7 +6,22 @@ export function ensureServerEntryExports(actions: any[]) {
const action = actions[i]
if (typeof action !== 'function') {
throw new Error(
`A "use server" file can only export async functions, found ${typeof action}.`
`A "use server" file can only export async functions, found ${typeof action}.\nRead more: https://nextjs.org/docs/messages/invalid-use-server-value`
)
}
if (action.constructor.name !== 'AsyncFunction') {
const actionName: string = action.name || ''
// Note: if it's from library code with `async` being transpiled to returning a promise,
// it would be annoying. But users can still wrap it in an async function to work around it.
throw new Error(
`A "use server" file can only export async functions.${
// If the function has a name, we'll make the error message more specific.
actionName
? ` Found "${actionName}" that is not an async function.`
: ''
}\nRead more: https://nextjs.org/docs/messages/invalid-use-server-value`
)
}
}

View file

@ -1,6 +1,6 @@
/* eslint-disable jest/no-standalone-expect */
import { createNextDescribe } from 'e2e-utils'
import { check, waitFor } from 'next-test-utils'
import { check, waitFor, getRedboxSource, hasRedbox } from 'next-test-utils'
import type { Request, Response, Route } from 'playwright'
import fs from 'fs-extra'
import { join } from 'path'
@ -508,6 +508,73 @@ createNextDescribe(
}
if (isNextDev) {
describe('"use server" export values', () => {
it('should error when exporting non async functions at build time', async () => {
const filePath = 'app/server/actions.js'
const origContent = await next.readFile(filePath)
try {
const browser = await next.browser('/server')
const cnt = await browser.elementByCss('h1').text()
expect(cnt).toBe('0')
// This can be caught by SWC directly
await next.patchFile(
filePath,
origContent + '\n\nexport const foo = 1'
)
expect(await hasRedbox(browser)).toBe(true)
expect(await getRedboxSource(browser)).toContain(
'Only async functions are allowed to be exported in a "use server" file.'
)
} finally {
await next.patchFile(filePath, origContent)
}
})
it('should error when exporting non async functions during runtime', async () => {
const logs: string[] = []
next.on('stdout', (log) => {
logs.push(log)
})
next.on('stderr', (log) => {
logs.push(log)
})
const filePath = 'app/server/actions.js'
const origContent = await next.readFile(filePath)
try {
const browser = await next.browser('/server')
const cnt = await browser.elementByCss('h1').text()
expect(cnt).toBe('0')
// This requires the runtime to catch
await next.patchFile(
filePath,
origContent + '\n\nconst f = () => {}\nexport { f }'
)
await check(
() =>
logs.some((log) =>
log.includes(
'Error: A "use server" file can only export async functions. Found "f" that is not an async function.'
)
)
? 'true'
: '',
'true'
)
} finally {
await next.patchFile(filePath, origContent)
}
})
})
describe('HMR', () => {
it('should support updating the action', async () => {
const filePath = 'app/server/actions-3.js'