-
Notifications
You must be signed in to change notification settings - Fork 997
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
📝 Add env variable for deploying to Vercel #10295
Conversation
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.
I'll defer to @ahaywood and @jtoar here, but this seems like a big enough break from the previous flow to warrant giving more explicit "next step" instructions within the rw setup deploy vercel
output:
https://github.com/redwoodjs/redwood/blob/main/packages/cli/src/commands/setup/deploy/providers/vercel.js
Also, it seems like this change would also be required across all deploy providers. Has there been any other QA sanity checks for new projects on Netlify, Render, or others?
@thedavidprice yes, I've done the QA sanity checks. I don't want to waste an hour digging up links but here's what I found in 5 minutes. (More like 30 mins now.) FWIW all other deploy providers don't need you to set an env var like this. Netlify and Coherence just work, and I've made the relevant PRs for Render and Flightcontrol (and Docker):
The repos in deploy target CI have all been using corepack for months. E.g. Netlify deploy logs: Coherence just works because their version of Nixpacks; Netlify may use a similar strategy. Flightcontrol does. Coherence deploy logs: |
Great! Thank you for taking time to catch me up. "yes, I've done the QA sanity checks"... would have been a good enough answer, fwiw 😅 Since you've done the work, it makes me think we should run this PR by the Vercel team to see if they'd be up for handling this internally via build config. Thoughts? (Or maybe you've already done that, too.) |
Option per @styfle (via DM)
I like adding this as a step in the setup command. Thoughts either way @ahaywood and @jtoar? Reference: |
Note that this may stop working with future versions of Node.js if this PR gets merged: Although the env var might not be needed at all if this other PR gets merged:
Feel free to add 👍 or 👎 to the linked PRs above. |
Thanks @styfle—I added a lot of both 👍s and 👎s to different comments. 😅 @thedavidprice I'd prefer if the setup command just added that file as well yeah. |
Follow up to #10295. Implements the strategy suggested in #10295 (comment). Still need to validate with deploy target CI.
Follow up to #10295. Implements the strategy suggested in #10295 (comment). Still need to validate with deploy target CI.
Follow up to #10295. Implements the strategy suggested in #10295 (comment). Still need to validate with deploy target CI.
Follow up to #10295. Implements the strategy suggested in #10295 (comment). Still need to validate with deploy target CI.
Closing since this is resolved with the setup command #10355 |
Fixes #10100
When you create a fresh install of Redwood and then try deploying to Vercel, it will error out unless you add the following environmental variable:
This PR adds documentation for deploying to Vercel.