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

Docs: How to use react cache with server actions if "use server" files only allow async exports? #62860

Closed
yannxaver opened this issue Mar 5, 2024 · 12 comments
Labels
Documentation Related to Next.js' official documentation. locked

Comments

@yannxaver
Copy link

What is the improvement or update you wish to see?

From 14.1.2 "use server" files only allow async exports. How can we utilize react cache now?

Is there any context that might help us understand?

#62821

Does the docs page already exist? Please link to it.

No response

@yannxaver yannxaver added the Documentation Related to Next.js' official documentation. label Mar 5, 2024
@sannajammeh
Copy link
Contributor

ref: #62821 (comment)

@shuding
Copy link
Member

shuding commented Mar 5, 2024

What’s the use case of caching a mutation function (Server Action)?

I think React.cache is designed to NOT cache anything when executed in a Server Action because there might be mutations:

"use server"

const getUser = cache(_getUser)

export async function deleteUser(name) {
  const user = await getUser(name)

  await db.delete(user.id)

  await getUser(name) // this won’t return any cache (and shouldn’t)
}

@yannxaver
Copy link
Author

yannxaver commented Mar 5, 2024

Okay, I see. Seems like the problem is more about project structure / code organization. So far I haven't used "use server" files exclusively for mutations.

I have one "use server" file per entity and this file includes "get" and "post" requests. I use React cache for the "get" requests. Seems like this structure won't work anymore and I need to create separate files for get and post requests.

/edit: To elaborate a bit on that: I have interpreted 'use server' as a message to the bundler that the code in this file should only run on the server. Now after reading your message, I assume that I should have two files - one with 'use server' for mutations and the other one with import server-only for fetches. Personally, I would prefer to have both types of functions in the same file.

Is the implication here that 'use server' files should not include get requests at all? There are two instances in my codebase in which I call a server action (which is no mutation but just returns data) from a client component. That would not be possible anymore if the function was not in a 'use server' file.

@chungweileong94
Copy link
Contributor

I do agree that you not suppose to use React cache with server action, as it literally does nothing. However, library that produces server action (async function) will cause the same error as well, as the code will get transpiled into Promise instead of action.constructor.name === "AsyncFunction"

@laurens-mesure
Copy link

I understand that caching mutations does nothing, but why limit this? It breaks colocation of getters and setters in a single file.

@mordechaim
Copy link

This is an absolute breaking change. I use React.cache to colocated getters (accessed in server components) and mutations.

@mordechaim
Copy link

mordechaim commented Mar 13, 2024

Workaround, drop-in replacement for React.cache:

import { cache as reactCache } from 'react'

export const cache = fn => {
    const cachedFn = reactCache(fn)
    return async (...params) => {
        return await cachedFn(...params)
    }
}

@chungweileong94
Copy link
Contributor

Workaround for React.cache:

import { cache as reactCache } from 'react'

export const cache = fn => {
    const cachedFn = reactCache(fn)
    return async (...params) => {
        return await cachedFn(...params)
    }
}

I'm not sure about the details, but chances are we are not supposed to call cache(...) within a function, I might be wrong. But maybe we should do it like this instead, more code but safer?

import { cache } from 'react'

const _fn = cache(async () => {
    return "...";
})

export const fn = async (...params) => {
    return _fn(...params)
}

@mordechaim
Copy link

Your code is useful when you colocate the cached function inside the server actions file. My version is a utility you could use as a dropin replacement to React.cache

but chances are we are not supposed to call cache(...) within a function

Tested, works perfectly. The only requirement is that the cached function shouldn't change identity so that's why I create cachedFn outside the returned async function.

@chungweileong94
Copy link
Contributor

chungweileong94 commented Mar 14, 2024

Looks like the stricter "use server" check has been reverted in the latest canary version #63200.

In the meantime, you can use this wrapper to workaround any async function produced by the library

function asyncFunction<T extends (...params: any[]) => Promise<any>>(fn: T) {
  return async (...params: Parameters<T>) => fn(...params);
}

@shuding
Copy link
Member

shuding commented Mar 17, 2024

The "AsyncFunction" requirement has been reverted (so I think this issue can be closed now, thanks everyone!).

It's totally fine to co-locate data access utilities, however this requirement still applies and we need to be careful. All functions exported from a "use server" file will become async once imported, even if they're defined as sync functions.

@shuding shuding closed this as completed Mar 17, 2024
shuding added a commit that referenced this issue Mar 25, 2024
For problems like #62821, #62860 and other, the only way to improve the
DX would be relying on the type checker to ensure that Server Actions
are async functions. Inlined definitions will always be checked by SWC
(as they're always syntactically defined as functions already), but
export values are sometimes determined at the runtime.

Also added `react-dom` related methods to the disallow list for the
server layer.

Examples:


https://github.com/vercel/next.js/assets/3676859/ac0b12fa-829b-42a4-a4c6-e1c321b68a8e


https://github.com/vercel/next.js/assets/3676859/2e2e3ab8-6743-4281-9783-30bd2a82fb5c


https://github.com/vercel/next.js/assets/3676859/b61a4c0a-1ad4-4ad6-9d50-311ef3450e13



Closes NEXT-2913
Copy link
Contributor

github-actions bot commented Apr 1, 2024

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot added the locked label Apr 1, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation Related to Next.js' official documentation. locked
Projects
None yet
Development

No branches or pull requests

6 participants