Fix the issue of refetching the vercel package every single time when using yarn berry.

[noobnooc]

As described in #240 , when using yarn berry, next-on-pages will refetch the vercel package every single time even it's in --watch mode. So, I fixed that by checking whether the vercel package has been installed locally. If it's installed, then run Vercel building with yarn vercel build instead of yarn dlx vercel build.

Fixes #240 .

[changeset-bot]

🦋 Changeset detected

Latest commit: 3d90720

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@cloudflare/next-on-pages	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

🧪 A prerelease is available for testing 🧪

You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/5001431737/npm-package-next-on-pages-247

Or you can immediately run this with npx:

npx https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/5001431737/npm-package-next-on-pages-247

[dario-piotrowicz]

Thanks a lot @noobnooc 🙂

Seems like your change has some type issues, also your if statement seems a bit hard to read (I imagine that that's why you added those comments in)

I think that something like the following you be pretty nice:

index c055633..bb4764a 100644
--- a/src/buildApplication/buildVercelOutput.ts
+++ b/src/buildApplication/buildVercelOutput.ts
@@ -1,5 +1,5 @@
 import { writeFile, mkdir, rm, rmdir } from 'fs/promises';
-import { spawn } from 'child_process';
+import { spawn, type ChildProcessWithoutNullStreams } from 'child_process';
 import { join, resolve } from 'path';
 import { cliLog } from '../cli';
 import { validateDir, validateFile } from '../utils';
@@ -81,25 +81,7 @@ async function runVercelBuild(pkgMng: PackageManager): Promise<void> {
 
        cliLog('Building project...');
 
-       let vercelBuild: ReturnType<typeof spawn>;
-
-       // If the package manager is yarn (berry), and the `vercel` package has not been installed,
-       // Then execute vercel building with `yarn dlx`.
-       if (
-               pkgMng === 'yarn (berry)' &&
-               // Check if the vercel package has been installed
-               !(await new Promise(resolve => {
-                       const vercelChecking = spawn(pkgMngCMD, ['info', 'vercel']);
-
-                       vercelChecking.on('exit', code => {
-                               resolve(code === 0);
-                       });
-               }))
-       ) {
-               vercelBuild = spawn(pkgMngCMD, ['dlx', 'vercel', 'build']);
-       } else {
-               vercelBuild = spawn(pkgMngCMD, ['vercel', 'build']);
-       }
+       const vercelBuild = await getVercelBuildChildProcess(pkgMng);
 
        vercelBuild.stdout.on('data', data =>
                cliLog(`\n${data}`, { fromVercelCli: true })
@@ -122,6 +104,29 @@ async function runVercelBuild(pkgMng: PackageManager): Promise<void> {
        });
 }
 
+async function getVercelBuildChildProcess(
+       pkgMng: PackageManager,
+): Promise<ChildProcessWithoutNullStreams> {
+       const pkgMngCMD = getPackageManagerSpawnCommand(pkgMng);
+       if (pkgMng === 'yarn (berry)') {
+               const vercelPackageIsInstalled = await isVercelPackageInstalled(pkgMng);
+               if (!vercelPackageIsInstalled) {
+                       return spawn(pkgMngCMD, ['dlx', 'vercel', 'build']);
+               }
+       }
+
+       return spawn(pkgMngCMD, ['vercel', 'build']);
+}
+
+async function isVercelPackageInstalled(pkgMng: PackageManager): Promise<boolean> {
+       const pkgMngCMD = getPackageManagerSpawnCommand(pkgMng);
+       const infoVercelExitCode = await new Promise(resolve => spawn(pkgMngCMD, ['info', 'vercel']).on('exit', resolve));
+       return infoVercelExitCode === 0;
+}
+
 /**
  * Vercel and Next.js generate *private* files for telemetry purposes that are accessible as static assets (`.vercel/output/static/_next/__private/...`).
  *

it fixes both issues and makes the if statement more readable, making extra comments unncessary

Please let me know what you think 🙂
(and/or if you have other ideas)

[noobnooc]

Due to some reason, my vscode did not show the type error, so I didn't realize it, sorry for that.

The change you proposed dose make the code much clearer. Since I rarely participate in open source projects and am not familiar with the process, I try to make as few changes to the code as possible, which led to me mixing everything together.

I have pushed a new commit to do that change, or you can also make changes in your own way.. If there is any other issue, you can let me know, so that I can become familiar with the process of participating in open-source projects and make some contributions to the community in the future :)

[dario-piotrowicz]

Due to some reason, my vscode did not show the type error, so I didn't realize it, sorry for that.

No problem at all, it happens, we do have the CI check for that! 😄

The change you proposed dose make the code much clearer. Since I rarely participate in open source projects and am not familiar with the process, I try to make as few changes to the code as possible, which led to me mixing everything together.

Ah I see, I totally get it, thanks for being considerate 🙂, yeah what you're suggesting does make sense I also do try to not change more than necessary when making PRs to open source projects 🙂, however I don't think that should be at the expense of the code quality. Anyways my suggestion is just to do what you think it's best then anyways you can get comments and suggestions and get things more in line with what project maintainers want anyways 🙂

I have pushed a new commit to do that change, or you can also make changes in your own way.. If there is any other issue, you can let me know, so that I can become familiar with the process of participating in open-source projects and make some contributions to the community in the future :)

Thanks so much, you change looks great! 😄

But you do have a merge conflict now 😢, please have a look

(PS: you're always welcome to contribute here! ❤️ if you want to get involved please look for the help wanted and/or good first issue issues we have 😃 and if you can't find anything you can even ping me and we can see if there is something you could contribute to 🙂)

---

PS: your code does look great anyways I noticed that (opposite to what I thought) pnpx does seem to have the same issue, would you like to update your PR so that if fixes that as well? (or would you prefer to just keep this yarn-only and we can look into pnpm separately?)

[noobnooc]

I resolved the conflict :)

But when I try to test in pnpm, some unknown errors occur. I haven't used pnpm before, and there are no prompts, only errors. I don't know how to resolve and test with it, so I think we can merge it first before I resolve this issue. Or you can fix and test it on your end. 🤣

[dario-piotrowicz]

@noobnooc thanks a lot for you changes thus far! 😃

I can play with pnpm and update your code if you're ok with it? 🙂
if fixing pnpm turns out to be a small/simple change then we can go with that otherwise we can just merge your PR and do that separately

does that sound ok to you? 🙂

[noobnooc]

Of course it's ok :)

[dario-piotrowicz]

@noobnooc I have just tried pnpm and the solution with it would be quite different from the yarn one you added (since I think pnpm info fetches from the registry and gives you the information of non-installed packages, it seems like it's also undocumented: pnpm/pnpm#5935 😓)

So I think for now we should just go with your solution and handle pnpm separately 🙂

[dario-piotrowicz]

Thanks a lot again for the contribution 😄

[dario-piotrowicz]

@noobnooc I've opened an issue for pnpm: #262
(I'll look into that soon 🙂)