feat(@netlify/remix-edge-adapter): support Hydrogen Vite sites #441
Conversation
"Entrypoint" is a loaded term here with multiple possible meanings, as is "server". Rename for clarity and consistency with Remix naming.
serhalp
added
the
type: feature
✅ Deploy Preview for remix-serverless ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for remix-edge ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
tests/e2e/fixtures/hydrogen-vite-site-no-entrypoint/app/components/Footer.tsx
Dismissed
Show dismissed
Hide dismissed
tests/e2e/fixtures/hydrogen-vite-site-no-entrypoint/app/components/Header.tsx
Dismissed
Show dismissed
Hide dismissed
tests/e2e/fixtures/hydrogen-vite-site-no-entrypoint/app/entry.client.tsx
Dismissed
Show dismissed
Hide dismissed
tests/e2e/fixtures/hydrogen-vite-site/app/components/Footer.tsx
Dismissed
Show dismissed
Hide dismissed
tests/e2e/fixtures/hydrogen-vite-site/app/components/Header.tsx
Dismissed
Show dismissed
Hide dismissed
serhalp
force-pushed
the
serhalp/frp-1264-hydrogen-sites-cannot-work-on-netlify-when-using-remix-vite
branch
2 times, most recently
from
45ac950
to
3863ddd
Compare
It appears to be (nearly?) imposssible to provide an out-of-the-box Netlify experience for Hydrogen sites that use Remix Vite. We may be able to solve for this at some point, but for now our only option is to expect these sites to contain a `server.ts` (or similar) file at the root. This is what Hydrogen templates do and what the Netlify Hydrogen template will do. There may be value in supporting this for Remix sites as well, but I haven't heard of a use case so I didn't bother supporting it here.
serhalp
force-pushed
the
serhalp/frp-1264-hydrogen-sites-cannot-work-on-netlify-when-using-remix-vite
branch
from
3863ddd
to
e1ffb39
Compare
|
Just checking about Hydrogen and non-edge variant - this part still doesn't work, correct? Should we maybe add checks to non-edge variant to fail with actionable message to use edge variant? (and if so, this doesn't need to be part of this PR and could be another item for "Future improvements") |
Yep, great idea. Once we've reached a good mergeable point with all this, I'd love to look at this again from the fresh perspective of someone trying to deploy a Hydrogen site to Netlify from the Hydrogen docs and make sure we're giving actionable pointers wherever we can. |
serhalp
force-pushed
the
serhalp/frp-1264-hydrogen-sites-cannot-work-on-netlify-when-using-remix-vite
branch
from
6aeaf11
to
f3fa359
Compare
| return () => { | ||
| if (isHydrogenSite && !viteDevServer.config.server.middlewareMode) { | ||
| viteDevServer.middlewares.use(async (nodeReq, nodeRes, next) => { |
There was a problem hiding this comment.
Some Hydrogen utils expect crypto to be available on global. Because for dev we run in Node this will be causing errors on node@18 (Node only added crypto to global in Node@19 without --experimental-global-webcrypto toggle - see https://nodejs.org/api/globals.html#crypto_1 -> expand "History")
If we want to "polyfill" this we could maybe do this:
| return () => { | |
| if (isHydrogenSite && !viteDevServer.config.server.middlewareMode) { | |
| viteDevServer.middlewares.use(async (nodeReq, nodeRes, next) => { | |
| return async () => { | |
| if (isHydrogenSite && !viteDevServer.config.server.middlewareMode) { | |
| if (!globalThis.crypto) { | |
| globalThis.crypto = (await import('node:crypto')).webcrypto | |
| } | |
| viteDevServer.middlewares.use(async (nodeReq, nodeRes, next) => { |
Otherwise we would require either using handling that OR say to use at least node@19 instead of 18.
There was a problem hiding this comment.
I could include a note about this in the Troubleshooting section I added to the README in netlify/hydrogen-template#4?
serhalp
deleted the
serhalp/frp-1264-hydrogen-sites-cannot-work-on-netlify-when-using-remix-vite
branch
Description
It appears to be (nearly?) impossible to provide an out-of-the-box Netlify experience for Hydrogen sites that use Remix Vite. This is in part because our Remix Vite (edge) adapter builds an SSR entrypoint that is not compatible with Hydrogen Vite. It seems impossible (or at least very difficult) to provide an alternative Hydrogen SSR entrypoint due to the directionality of configuration (I won't get into that rabbit hole here, but user-land code, framework-land code, meta-framework-land code, and platform-land that all need to interact in complex ways and our Remix adapter approach just doesn't play nicely with this flow at all.)
We may be able to solve for this at some point, but for now our only option is to expect these sites to contain a
server.ts(or similar) file at the root and use that as a custom SSR entrypoint. Hydrogen templates include such a file and so will the Netlify Hydrogen template, so this seems ok, though a bit on the magical side. I did make sure to fail the build with an actionable error for misconfigured Hydrogen sites.This also seems to be in line with Remix's philosophy: https://remix.run/docs/en/main/guides/deployment. 🤷🏼♂️
(There may be value in supporting this for non-Hydrogen Remix sites as well, but I haven't heard of a use case so I didn't bother supporting it here.)
Refactors
I also refactored a bit in a separate commit. I mostly improved naming and inline documentation. This space is very complicated.
Future improvements
As a next step, we could probably trim down our template'sEnded up doing this in this PRserver.tsby about half or so by providing helper functions from this package (or a new Hydrogen package) that abstract out the platform-specific parts.Related Tickets & Documents
See netlify/hydrogen-template#4.
QA Instructions, Screenshots, Recordings
I added e2e tests for a working Hydrogen Vite site (from the updated template in netlify/hydrogen-template#4, with a bit of fat trimmed) and a misconfigured one without a
server.ts. I've also done extensive testing. This repo also already has existing Remix e2e tests.