vercel / platforms

A full-stack Next.js app with multi-tenancy and custom domain support. Built with Next.js App Router and the Vercel Domains API.
https://app.vercel.pub
5.38k stars 688 forks source link

minor: (invalid?) use of promises #390

Closed thoaionline closed 3 months ago

thoaionline commented 4 months ago

1. invalid promise type check

in this repo, under apps/magic/lib/actions.ts:

import { revalidateTag } from "next/cache";

then subsequently:

await revalidateTag(
  `${subdomain}.${process.env.NEXT_PUBLIC_ROOT_DOMAIN}-metadata`,
);

however, the declared return type is

export declare function revalidateTag(tag: string): void;

which is not a promise, and thus shouldn't be await-ed.

i checked the implementation here and it's indeed not a promise.

2. promise executor functions should not be async.

there are a couple of

    new Promise<void>(async (resolve, reject) => ...

the async kindas defeat the purpose of a Promise constructor these days. should be a trivial fix.

those 2 issues are the main bugs that come up in sonarcloud from scanning a fresh clone.

i'm happy to make a PR for both but wanted first to check if the await was intentional.

justinh00k commented 3 months ago

+1

Vercel docs say server actions need to be async.

https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions-and-mutations

If it is not async, Next throws: Server actions must be async functions

And revalidateCache needs to be a server action: https://nextjs.org/docs/app/api-reference/functions/revalidateTag

I'm guessing it's not currently async because it isn't possible for the server to actually return the promise?

Canary build refactors the revalidate functions, but none are marked async.

https://github.com/vercel/next.js/blob/canary/packages/next/src/server/web/spec-extension/revalidate.ts

Simplest solution for now is to mark the function async and disable the typescript await rule:

// eslint-disable-next-line @typescript-eslint/require-await

The documentation linked above shows that the function should not be awaited.