alfonsusac / nextjs-better-unstable-cache

Wrapper function for unstable_cache() from next/cache (Next.js Caching)
https://www.npmjs.com/package/nextjs-better-unstable-cache
72 stars 5 forks source link

Usage questions #5

Closed benjick closed 1 year ago

benjick commented 1 year ago

Hello!

Wanted to check in on what the recommended usage is for this library and see if I've gotten the correct understanding of it.

This is a small code-snippet:

export async function getGroupMembers(groupId: number) {
  const { userAbility } = await createUserAbility();
  if (!userAbility.can("view", subject("Group", { groupId }))) {
    throw new Error("Access denied");
  }
  const groupMembers = await getGroupMembers_CACHED(groupId);
  return groupMembers;
}

const getGroupMembers_CACHED = memoize(getGroupMembers_UNSAFE,
  {
    persist: true,
    duration: 60,
    revalidateTags: (id) => ["groups", `group=${id}`],
    additionalCacheKey: ["getGroupMembers"],
  },
);

export async function getGroupMembers_UNSAFE(groupId: number) {
  const groupMembers = await db.query.usersToGroups.findMany({
    where: eq(usersToGroups.groupId, groupId),
    with: {
      user: true,
    },
  });
  return groupMembers.map((g) => g.user);
}

export async function joinGroup(formData: FormData) {
  ... stuff
  revalidateTag(`group=${group.id}`);
}

Am I right in not caching the outer function? Or should I maybe cache that too so I don't have to retrieve the user session every time (Lucia Auth)?

alfonsusac commented 1 year ago

Hi, thanks for checking out this repo. I am in no way an experienced developer but let me give you what my opinion is.

Am I right in not caching the outer function? Or should I maybe cache that too so I don't have to retrieve the user session every time (Lucia Auth)?

For me, i think this is already a correct approach. You dont need to persist the result of the user authentication since that would not achieve an up-to-date session checking. i.e Your session is already expired but the session-checking function is still cached and that would be undesirable.

What I would like to do is in addition to separating the authentication like that, is also cache the auth()-checking function using cache() so that any calls to that auth checks are deduped and you only need to check once PER request.

It totally make sense to check the auth first before going for the cached function, thereby separating the data layer and the auth layer.

(Im just gonna close this as this is a discussion not an issue, feel free to post a message)

benjick commented 1 year ago

Thanks for the reply, I appreciate to be able to brainstorm with another dev!

What I would like to do is in addition to separating the authentication like that, is also cache the auth()-checking function using cache() so that any calls to that auth checks are deduped and you only need to check once PER request.

Thanks for the tip! I checked and the auth framework handles it automatically already, so that's awesome. Thanks for the feedback!