Skip to content

Commit

Permalink
fix: Add stricter check for "use server" exports (#62821)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
shuding committed Mar 4, 2024
1 parent b80d388 commit 7b2b982
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 2 deletions.
43 changes: 43 additions & 0 deletions errors/invalid-use-server-value.mdx
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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`
)
}
}
Expand Down
69 changes: 68 additions & 1 deletion test/e2e/app-dir/actions/app-action.test.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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'
Expand Down

0 comments on commit 7b2b982

Please sign in to comment.