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

Update build script to use remix vite:build and update Readme with Vercel hosting instructions for Remix with Vite #687

Merged
merged 4 commits into from
May 16, 2024

Conversation

jeffsee55
Copy link
Contributor

@jeffsee55 jeffsee55 commented Apr 25, 2024

Vercel recently shipped a Remix Vite plugin that no longer requires a forked version of Remix. The package also exports platform-specific utilities that may need to be imported at runtime, detailed in the docs link above. This PR updates the README for the Vercel-specific instructions.

It also modifies the build script, I noticed that the build command was reverted from remix vite:build here, I'm not sure on the specifics of why that was necessary but the Vercel preset won't work without the remix vite:build command as far as I know. If there's a reason the remix command can't be used then this PR should just be closed as it won't help.

Checklist

Note: once this PR is merged, it becomes a new release for this template.

  • I have added/updated tests for this change
  • I have made changes to the README.md file and other related documentation, if applicable

@jeffsee55 jeffsee55 marked this pull request as ready for review April 25, 2024 17:22
@jeffsee55 jeffsee55 requested a review from a team as a code owner April 25, 2024 17:22
@lizkenyon
Copy link
Contributor

lizkenyon commented Apr 29, 2024

Hi there @jeffsee55 👋

Thank you for this! If we can eliminate special cases for deployment that is great!

I was wondering if you happened to test deploying one of our app templates?
We have just gotten reports of Vercel deploys now failing 😞 , we are just trying to determine if this is related to our recent release, the changes to Vercels deployment or a combination.

@jeffsee55
Copy link
Contributor Author

Thanks @lizkenyon, I did get this to deploy successfully with an existing template, I'm not sure exactly what you mean by one of your app templates.

I don't think this relates to the Vercel deploys which are failing, but I did want to highlight again that the build script that I changed in this PR was intentionally rolled back in a previous PR, presumably for good reason #564 but I couldn't gather enough info from the PR to know why.

@lizkenyon
Copy link
Contributor

Hi @jeffsee55

Sorry by one of our templates, I was referring to the code in this repository.

Yes, we should be good to change the build script to what you have. If you are able to sign the CLA, we will test this on our side and get this merged! Thanks again!

@jeffsee55
Copy link
Contributor Author

Thanks @lizkenyon, signed! I also resolved a merge conflict that was created from #358. I think the area that was updated in that PR was a typo (extra space) so went ahead and continued to delete that sentence

@lizkenyon
Copy link
Contributor

Hey @jeffsee55

This is slightly unrelated, but it seems new deploys of Shopify App Remix templates are now failing.

We tracked this down to our use ofimport assertions in one of our libraries.

I created a discussion on the Vercel github, and I was wondering if you could help getting some eyes on this by the right person?

@jeffsee55
Copy link
Contributor Author

Thanks @lizkenyon, we'll take a look!

@lizkenyon
Copy link
Contributor

Thanks again for your contribution here!

@lizkenyon lizkenyon merged commit 95b68ac into Shopify:main May 16, 2024
3 checks passed
github-actions bot pushed a commit that referenced this pull request May 16, 2024
Update build script to use `remix vite:build` and update Readme with Vercel hosting instructions for Remix with Vite
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants