-
Notifications
You must be signed in to change notification settings - Fork 325
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 Process.env instead of Oxygen.env #1314
base: v1.x-2022-07
Are you sure you want to change the base?
Conversation
e4e79d2
to
f517862
Compare
This comment has been minimized.
This comment has been minimized.
fb50a07
to
ad5c94d
Compare
ad5c94d
to
6dfe9ce
Compare
6dfe9ce
to
b6a38c0
Compare
Unfortunately, it appears that Vite will find/replace any usage of This means all Marking this as blocked until we can find a workaround. |
@wizardlyhel @blittle — should we close this PR in favour of |
@frandiox |
It is a vite thing. I'm wondering how it gets bundled / compiled, because runtimes (vercel, netlify, cloudflare, etc) aren't going to have it defined. |
I remember suggesting adding a custom |
It's just really hard to know what to compile to, because each runtime does env variables slightly different. Which is why we are pushing for some sort of unification standard: wintercg/environment-metadata#1 I guess the CLI could have an option to compile to different runtime targets? Vercel, cloudflare, oxygen, etc? |
Now tracking in #1879 |
Description
Fixes Shopify/hydrogen#1226 by populating process.env in our
platforms/worker.ts
and hydrogen middleware.Additional context
Before submitting the PR, please make sure you do the following:
fixes #123
)yarn changeset add
if this PR cause a version bump based on Keep a Changelog and adheres to Semantic Versioning