-
Notifications
You must be signed in to change notification settings - Fork 124
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
Replace append with set in adjustRequestForVercel (Fixes #507) #508
Conversation
🦋 Changeset detectedLatest commit: 50ecd26 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi there, thank you very much for raising this PR 🙂
Would it be possible for you to add a small patch changeset to it using npx changeset
, briefly outlining what this PR fixes?
I have added a changeset, thanks for the heads-up 😃 |
It's because they're fixed to each other, so it releases them both for every version change in each of the two packages. It's so that the version of the eslint package that people use is tied to their version of next-on-pages, instead of them needing to have different version numbers for each one. Makes things simpler to manage. |
🧪 Prereleases are available for testing 🧪 @cloudflare/next-on-pagesYou can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/6539644090/npm-package-next-on-pages-508 @cloudflare/eslint-plugin-next-on-pagesYou can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/6539644090/npm-package-eslint-plugin-next-on-pages-508 |
@james-elicx Could you tell me/point me towards docs on how to deploy a build of a page made with the preview build of next-on-pages so I can validate that the fix has worked? |
Sure thing :). You can deploy a build using Wrangler, and it will prompt you to sign in with your Cloudflare account if you haven't already. npx wrangler pages deploy .vercel/output/static You can choose an existing or a new project when deploying. If you are deploying a new project, in the Cloudflare Pages dashboard, you need to ensure that you add the Alternatively, if you have a project that is using the Pages Git Integration / CI that you would like to deploy with a prerelease, you could replace the build command with |
Alright, I have made a deployment with wrangler, and tested it out. With this patch, the headers are now inline with Vercel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @rphsoftware 😃 ❤️
This pull request aims to fix the small incompatibility between real Vercel and the simulated Vercel headers by replacing headers, instead of appending to their values. This should fix #507