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

use vercel fork #239

Merged
merged 9 commits into from
May 16, 2023
Merged

Conversation

dario-piotrowicz
Copy link
Member

@dario-piotrowicz dario-piotrowicz commented May 13, 2023

This PR makes Pages CI build using the new v2 beta image work.

The issue is that the current Vercel CLI opens too many file handlers at once (which the v2 beta image doesn't seem to be handling correctly) when building a Next.js >= v.13.3.0 app that contains prerendered routes.

The prerendered routes are problematic because as part of the building process Vercel generates lambda functions for them, such lambda functions include node modules, the parallel downloading of such node modules causes too many files to be open at once.

This PR introduces as a temporary fix the use of a fork of the Vercel CLI that sequentializes the above mentioned problematic downloads solving the issue (see dario-piotrowicz/vercel@4f63551).

This is meant to be a very temporary fix to be in place only until the v2 image has been fixed/improved to handle more files handlers at once.


I've tested it with the following apps from my testing apps repo:

@changeset-bot
Copy link

changeset-bot bot commented May 13, 2023

⚠️ No Changeset found

Latest commit: ce6b0c6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@github-actions
Copy link
Contributor

github-actions bot commented May 13, 2023

🧪 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/4989651998/npm-package-next-on-pages-239

Or you can immediately run this with npx:

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

@dario-piotrowicz dario-piotrowicz marked this pull request as ready for review May 14, 2023 11:16
@petebacondarwin
Copy link
Collaborator

IMO this PR (and maybe even the changeset) needs a bit more information about what this fork is, what it does, and why it is needed.

README.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Member

@GregBrimble GregBrimble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd for sure pin the version, and I'd strongly consider removing any reference to this in public docs.

@GregBrimble
Copy link
Member

Where is the fork? If not in the Cloudflare org, I think it should be open-source at least somewhere (your personal account?) so inquisitive folks can see what code we're running.

@dario-piotrowicz
Copy link
Member Author

Where is the fork? If not in the Cloudflare org, I think it should be open-source at least somewhere (your personal account?) so inquisitive folks can see what code we're running.

It's in the PR's description (I had the link wrong, I've just updated it 🙇) I hope that's enough

@dario-piotrowicz
Copy link
Member Author

This console.log intentional? vercel/[email protected]:vercel:next-on-pages-mod#diff-e38f85b3385148c0b7d002fe5ec2e5f8d8488e9a786f91cba00eeb556da437b4R552

no sorry it's a leftover of my debugging 😓, I've removed that, updated the next-on-pages dependency and updated the PR's description with the latest commit, thanks for noticing it 🙂

@dario-piotrowicz dario-piotrowicz merged commit 39cfa74 into cloudflare:main May 16, 2023
@dario-piotrowicz dario-piotrowicz deleted the use-vercel-fork branch May 16, 2023 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants