From 7b2b9823432fb1fa28ae0ac3878801d638d93311 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Mon, 4 Mar 2024 18:50:19 +0100 Subject: [PATCH] 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. --- errors/invalid-use-server-value.mdx | 43 ++++++++++++ .../next-flight-loader/action-validate.ts | 17 ++++- test/e2e/app-dir/actions/app-action.test.ts | 69 ++++++++++++++++++- 3 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 errors/invalid-use-server-value.mdx diff --git a/errors/invalid-use-server-value.mdx b/errors/invalid-use-server-value.mdx new file mode 100644 index 0000000000..a3a0a9786d --- /dev/null +++ b/errors/invalid-use-server-value.mdx @@ -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) diff --git a/packages/next/src/build/webpack/loaders/next-flight-loader/action-validate.ts b/packages/next/src/build/webpack/loaders/next-flight-loader/action-validate.ts index 08b9b53785..19940619d6 100644 --- a/packages/next/src/build/webpack/loaders/next-flight-loader/action-validate.ts +++ b/packages/next/src/build/webpack/loaders/next-flight-loader/action-validate.ts @@ -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` ) } } diff --git a/test/e2e/app-dir/actions/app-action.test.ts b/test/e2e/app-dir/actions/app-action.test.ts index 46775c1cdd..c4b1f95652 100644 --- a/test/e2e/app-dir/actions/app-action.test.ts +++ b/test/e2e/app-dir/actions/app-action.test.ts @@ -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'