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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[hydrogen] Polyfill Oxygen global in Hydrogen #8496

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mperreux
Copy link

Related Issues

Description stolen from #8413
It's currently not possible to access environment variables in Hydrogen projects deployed to Vercel. The reason is that Hydrogen currently relies on a global Oxygen.env that needs to be polyfilled in the platform. This PR adds the global object using process.env, which is mentioned in the docs as the way to access environment variables in Edge Functions.

馃搵 Checklist

Tests

  • The code changed/added as part of this PR has been covered with tests
  • All tests pass locally with yarn test-unit

Code Review

  • This PR has a concise title and thorough description useful to a reviewer
  • Issue from task tracker has a link to this PR


// Hydrogen exposes env vars through `Oxygen.env`
Oxygen: {
env: process.env
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I actually think that this won't work the way you're expecting. This is because for Edge functions, process.env usage is statically analyzed to determine which variables to include in the edge function bundle.

To make it work with Oxygen.env usage then we would have to do similar static analysis in @vercel/hydrogen and populate the envVarsInUse array in the EdgeFunction constructor. Perhaps Hydrogen could provide that information if it is already doing static analysis during the build process?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not doing any static analysis in the build process in Hydrogen, afaik 馃
Perhaps you can replace Oxygen.env.XYZ matches in @vercel/hydrogen?

Also, we are moving to using Hydrogen.env instead of Oxygen.env to avoid confusion when deploying to other platforms, so something to keep in mind.

@styfle styfle added semver: patch PR contains bug fixes area: hydrogen labels Aug 31, 2022
@ryanleichty
Copy link

Any idea when this might get merged/resolved? Would love to use Vercel for hosting a current project 馃檶馃徎

@TooTallNate
Copy link
Member

@ryanleichty The current workaround would be to just use process.env instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: hydrogen semver: patch PR contains bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants