Coder Social home page Coder Social logo

Comments (20)

jplhomer avatar jplhomer commented on September 18, 2024 1

Pinging @graygilmore @ofuerst @genevieve who may want to follow along with our Hydrogen-specific implementation for dealing with secrets and env vars!

from hydrogen-v1.

jplhomer avatar jplhomer commented on September 18, 2024 1

Based on our discussion with Oxygen runtime folks this week, and @frandiox's code spike above, here's what I think we should build for Hydrogen environment variable support.

It's important to note the difference between server and client contexts, because each context has unique implications on how we support variables in Hydrogen.

Server contexts

When building Hydrogen with Oxygen as a deployment target, developers will use the Oxygen.env global variable to access environment variables:

// pages/product/product.server.jsx

export default function Product() {
  const activeProductFeature = Oxygen.env.FEATURED_PRODUCT_ID;
  // ...
}

Development mode

During development, Hydrogen will inject a global Oxygen.env object via Vite middleware.

The value of this object will be populated from multiple sources:

  • A .env file
  • An environment-specific .env file like .env.testing which is matched using vite.mode. This defaults to .env.development during development, and .env.production when in production. You could easily introduce a .env.testing by passing --mode testing during vite build.
  • Any environment variables passed to the vite dev bash command. These can be accessed via process.env in the Node.js development runtime

Production mode

In production, Oxygen will inject the global Oxygen.env object in whatever way makes sense. These variables will be populated from the admin UI Oxygen provides to set environment variables.

Client contexts

In client contexts, aka JS bundles built for the browser, it's critical not to leak sensitive environment variables.

To mitigate this, we will still populate a Oxygen.env global variable, but only with variables that are prefixed with a HYDROGEN_PUBLIC_ string. This follows a similar pattern set by Next.js.

// components/Counter.client.jsx

export default function Counter() {
  const counterText = Oxygen.env.HYDROGEN_PUBLIC_COUNTER_TEXT;

  return <button>{counterText}</button>;
}

Development mode

Behaves the same as server contexts (see above), only that we inject the middleware with ssr: false in Vite.

Production mode

An important constraint here is that the inlining happens during build time and not at runtime/worker boot.

If the developer needs to provide an environment variable that is used in the client context, they will need to provide the variable prior to deploying the bundle to Oxygen.

This can happen in a couple ways:

  • In GitHub Actions workflow as env arguments to the Deploy Oxygen workflow
  • Locally as ENV vars in bash (or .env.production) when deploying to Oxygen via CLI (not yet supported)

However, this presents a mismatch of expectations:

  • if I declared HYDROGEN_PUBLIC_FOO in my Oxygen admin, would I expect this to be set automatically for me in my build step?
  • Should Oxygen be exposing public variables to Vite during a local build step in CI/CLI?

@ofuerst @mklocke-shopify curious what your thoughts are on the above ^

A note on secrets

You'll notice that I've focused exclusively on environment variables so far, and not on the concept of "secrets." I see secrets as a merely a special type of environment variable.

  • Developers should rely on environment variables to inject secrets as needed.
  • By default, environment variables are only injected into server contexts, meaning it's not being inlined in the public build
  • Developers would have to be doing something very wrong to think they are safe writing sensitive environment variables as HYDROGEN_PUBLIC_SECRET_SHHHH

Some platforms handle secrets differently. For example, Cloudflare Workers allows you to mark an environment variable as a secret, and it encrypts the value. However, the developer does not need to be concerned about decrypting the variable since it is automatically decrypted at runtime by Cloudflare.

I think Hydrogen should follow this expectation and assume the runtime is taking care of encrypting secrets if it wants to.

What about Cloudflare/Vercel/Deno/etc?

It's important to call out that environment variable support will be unique to each hosting provider. This changes based on whether you're deploying to Oxygen, Cloudflare, Vercel or Deno Deploy.

While Hydrogen's out-of-the-box development experience is optimized for Oxygen deployment with the injection of Oxygen.env, nothing prevents a developer from using Cloudflare's globalThis style environment variables (or env object in module syntax). The burden will be on the developer to shim their development runtime to support their host's expectations, e.g. Deno.env or CF-style.

Likewise, if your deployment target is a Docker container or a Node.js runtime, you can use process.env. As of Vite 2.7, process.env is left intact for SSR bundles, so it can be used within server components in this particular case. If a developer wants to use process.env variable in a client context, they can prefix their variables with VITE_PUBLIC_ and access it with import.meta.env per VIte's guidelines.

from hydrogen-v1.

frandiox avatar frandiox commented on September 18, 2024

TL;DR

I think we should not rely on process.env or import.meta.env because there might be conflicts with Vite (as shown in that issue) and it is not available in a prod environment like CFW or (I assume?) Oxygen.
Instead, we could place the secrets somewhere in globalThis.

As a way to improve DX, perhaps we can provide import.meta.secrets object or import secrets from '.../hydrogen' (I started thinking the latter was cool, but after rewriting this comment 10 times now I think the former might be better).



Here are some thoughts on this. I guess the first decision is if we want to:

  1. Inline strings in a server-only build.
  2. Pass secrets directly to the running process as globals.

Since we are using RSC, 1 might be challenging (or at least I don't know how well delimited is a server-only build in this context), plus secrets can be revealed by checking that server bundle, which is not ideal.

Therefore, I guess option 2 would be preferable?

How to provide secrets to Hydrogen

For prod environments:
It really depends on the running platform. In CFW you can either use Wrangler or their Dashboard, and then they are exposed as global variables in code (more on this later). I assume we'd have something similar in Oxygen?

For non-prod environments (running in Node):

  1. Special .env file (e.g. .env.secrets.development or .env.secrets.test) that is read in our plugin using Vite's loadEnv function.
  2. As Node environment variables with a HYDROGEN_ prefix (e.g. $> HYDROGEN_MY_SECRET=42 vite dev).

Implementation constraints and details

As far as I know, in CFW you get secrets and env vars as global variables (they are not inlined but real variables). For example, if you declare MY_SECRET="42", you can access it later with console.log(MY_SECRET) anywhere. This should be similar to globalThis.MY_SECRET. I would assume Oxygen will have a similar feature?

Considering this, we could rely on globalThis also in the development server, so we would not be affected by Vite replacing process.env (and it all runs in 1 process so we can populate globalThis in our plugin's configureServer, I think).

So far, this approach seems easy and is compatible with every environment (Node, CFW and likely Oxygen). However, I personally don't like having the secrets directly as globals, and this also means that TypeScript will complain a lot.

User interface

As mentioned, calling global secrets would work but might not be the best experience for the developer. We could provide some sugar at the framework level. Some alternatives:

  1. Move all the secrets to a globalThis.SECRETS: Record<string, string | undefined> object, so it can be used as console.log(SECRETS.MY_SECRET).

    • This would need some runtime in production that finds global secrets and moves them to the object in CFW (and possibly Oxygen).
    • Would require HYDROGEN_ prefix so we can find the secrets in prod.
  2. Import a secrets object from Hydrogen and use it directly: import secrets from '@shopify/hydrogen/secrets.

    • It's basically a file that reads from globalThis and exports all the secrets found.
    • Would require HYDROGEN_ prefix so we can find the secrets in prod.
  3. Implement some Vite-like syntax sugar: import.meta.secrets.MY_SECRET and replace that with globalThis.MY_SECRET in dev and prod (as a variable, not as a string value).

    • I think this doesn't need any runtime in prod, but we need to transform files (only *.server.jsx?) in dev and build.
    • Prefixes are not required but can be added anyway if we want.
    • We can add some generic types to that global object as well.
    • In development, we could replace this by string values directly if we don't want to pollute globalThis.

from hydrogen-v1.

 avatar commented on September 18, 2024

My only suggestion at this point would be to expose it in Oxygen as oxygen.env.FOO_BAR for example. Nice namespace with room for extension if we should wish.

from hydrogen-v1.

frandiox avatar frandiox commented on September 18, 2024

@jplhomer Looks like Oxygen is going to support global properties first. Should I start implementing this in Hydrogen to support CFW as well?

If so, what syntax do you think is better from what I mentioned? Or something different?
I personally like reading from .env.secrets files and using import.meta.secrets.XXX which is replaced to process.env.XXX in dev or globalThis.XXX in worker-prod.

from hydrogen-v1.

jplhomer avatar jplhomer commented on September 18, 2024

Oooh interesting update: vitejs/vite#5404

process.env will be left in-tact for SSR loads in Vite 2.7.

from hydrogen-v1.

maxshirshin avatar maxshirshin commented on September 18, 2024

@jplhomer Regarding other providers: do you think it would make sense for Hydrogen to provide an optional library to "mirror" other popular env APIs to Oxygen.env? it could be Proxy-based and shouldn't be very hard to implement (unless it's CF's globalThis which may be difficult to filter because there's no namespace). Might be one of those sweet little gimmicks that make people fall in love with dev experience.

from hydrogen-v1.

frandiox avatar frandiox commented on September 18, 2024

@jplhomer I was under the impression that all client/public variables would use the normal Vite worflow (VITE_ prefix in dotenv files) and that Oxygen.env would only be used in server components for variables that cannot be leaked. That's why the previous implementation simply passes secrets to handleEvent πŸ€”

I can see that, however, it can be useful getting public variables as well from Oxygen.env since that's something the merchants can modify directly, right? But as you say, this is tricky because we need to get the variables when building the frontend bundle so I'm not sure how this will work.

I'd like to have some clarification about environment variables vs secrets. Perhaps we should call them "vite variables" vs "hydrogen variables" instead. Is the following correct?

  • Vite variables:
    • Read from dotenv files and prefixed with VITE_*.
    • Inlined in both client and server bundle (replaced as strings) at build time.
  • Hydrogen variables (secrets?):
    • Read from dotenv files in non-production environments (with a certain prefix, e.g. HYDROGEN_*).
    • Read from admin panel in production (also with the same prefix?).
    • Can be marked as "public" with a different prefix (e.g. HYDROGEN_PUBLIC_*).
    • Public ones are inlined in client bundle at build time.
    • All of them are stored in memory in the server, and can be accessed with Oxygen.env, etc.

The burden will be on the developer to shim their development runtime to support their host's expectations, e.g. Deno.env or CF-style.

This feels like a point of friction for both developers and ourselves writing docs/answering issues.
As @maxshirshin suggests, what about a centralized proxy at Hydrogen level that finds the right place to read from?
It would check typeof Oxygen, process, Deno, etc. just once to find the right place to read from (with a fallback to reading from globalThis).

This somewhat connects with what I wrote at the end of Shopify/hydrogen#55 (comment) , the proxy could be imported (import {secrets} from '@shopify/hydrogen/secrets'), or global just like Vite's approach (import.meta.secrets.XYZ).

Another random idea would be merging "vite variables" and "hydrogen variables" into just import.meta.env.XYZ, which is a nice API. This would require hacking Vite a bit but might be possible πŸ€”

from hydrogen-v1.

mklocke-shopify avatar mklocke-shopify commented on September 18, 2024

Hey @jplhomer a few clarificaions/questions on your comment from abive.

Build Step

if I declared HYDROGEN_PUBLIC_FOO in my Oxygen admin, would I expect this to be set automatically for me in my build step?

Can you elaborate on that please? I'm not sure I understand the question fully. Essentially any value you would put into the Admin UI (e.g. FOO=BAR or APIKEY=foobar) would be injected into your runtime after you deploy these variablese.

I can't speak to the exact format of this in the Admin UI but I assume it will be some key/value input field. So if you set those, deploy them with Oxygen then they would be available in your runtime.

Currently, we expose all variables that are defined for a deployment with no distinction.

Local Vars

Should Oxygen be exposing public variables to Vite during a local build step in CI/CLI?

There will be support in oxygen-run to pass an environments file, in the same structure as production, which will be injected in the runtime in the same way that we would do it in production. But as you mention there is no support for deploying to Oxygen via CLI at all yet.

I'm not sure if I followed the part with Vite though, would that be more for local testing?

Secrets

I think Hydrogen should follow this expectation and assume the runtime is taking care of encrypting secrets if it wants to.

I very much agree with this, encryption/decryption should be handled at runtime. Once secrets they are injected into the runtime they should be automatically decrypted at time of injection.
There have been a few discussions around encryption/decryption (cc @lancelafontaine @pirivan ) but as I understand it (subject to change I'm sure) the plan is that DMS will encrypt from the UI side and then the runtime will decrypt upon injection (either at the SWS layer or the OAS layer).

from hydrogen-v1.

frandiox avatar frandiox commented on September 18, 2024

@mklocke-shopify

Can you elaborate on that please? I'm not sure I understand the question fully. Essentially any value you would put into the Admin UI (e.g. FOO=BAR or APIKEY=foobar) would be injected into your runtime after you deploy these variablese.

The problem is that we need to build a client bundle before deploying. This bundle is what runs in the browser so it won't have access to the Oxygen runtime. Therefore, we need to inline the public environment variables in the bundle as text (i.e. literally code.replace('process.env.MY_VARIABLE', 'actual value')).

This is easy to do with variables located in local dotenv files. However, what do we do with variables that come from the admin UI? Can they be injected somehow in the build process where the client bundle is generated (so they can be inlined like the others)?

from hydrogen-v1.

graygilmore avatar graygilmore commented on September 18, 2024

Can they be injected somehow in the build process where the client bundle is generated (so they can be inlined like the others)?

I'm coming into this a little late but: I think it's better to think about the variables set in the admin as "Oxygen Runtime Variables" and maybe not "Environment Variables". For public strings meant to be exposed through the browser this wouldn't be Oxygen.env, right? What do we think about that?

from hydrogen-v1.

mklocke-shopify avatar mklocke-shopify commented on September 18, 2024

Thanks for the clarification @graygilmore ! You're exactly right there was confusion on my end about this.

The "Oxygen Runtime Variables" are really the runtime / server side variables defined in the Admin UI (please correct me if I am confusing terminology again here).

The public strings to be exposed are what needs some more conversation I think on how we want to achieve this, we would not use Oxygen.env, or at least if we want to do something we need to discuss that a little more.

Do we imagine these public strings coming from the Admin UI as well?

I definitely had some confusion on the Runtime vs Public Variables kind of thing so apologies if it lead to more confusion.

from hydrogen-v1.

jplhomer avatar jplhomer commented on September 18, 2024

Thanks all for your feedback and questions!

Perhaps we should call them "vite variables" vs "hydrogen variables" instead.

I think one thing that jumped out to me from @frandiox's comment is that we are still mixing concerns.

This should be our north star:

  • Hydrogen should be responsible for injecting variables into the client bundle
  • The hosting runtime should be responsible for injecting variables into the server bundle

Based on that, I'd like to make some adjustments to my most recent proposal:

  • In client bundles, Hydrogen will NOT populate a Oxygen.env object. Instead, we should lean into the import.meta.env API provided by Vite. Variables provided to the build process that are prefixed with HYDROGEN_PUBLIC_ will be available to the client bundle.
  • In server bundles, the hosting runtime will be responsible for exposing environment variables. In the case of Oxygen, this will be Oxygen.env.
  • In server bundles, Hydrogen will still inline import.meta.env variables. This is necessary because client components will reference these variables, and client components render on the server during SSR.
  • Environment variables set in the Oxygen Admin UI will NOT be available in client bundles, because no Oxygen.env is available. Per @graygilmore's point, maybe "Oxygen Runtime Variables" Is a more descriptive name than env vars.
  • During development, Hydrogen will still automatically inject Oxygen.env into the server bundle, per my original proposal. This is because Oxygen is meant to be a first-class citizen in Hydrogen. Note: This doesn't prevent other hosting runtimes from injecting their own mechanisms into the dev server bundles with whatever tooling they use β€”Β but it's a separate concern from Hydrogen.

On automatic hosting runtime env var conversion > Oxygen.env

While this idea is a convenient developer tool, I don't think this scales. We'd have to make a decision which hosting runtimes to support, which all might have unique requirements. Additionally, making e.g. Vercel variables into an object called Oxygen.env is potentially confusing.

By leaving server bundle runtime variables up to the hosting runtime (with a convenience mechanism built-in for Oxygen), Hydrogen doesn't need to be concerned with this.

On using import.meta.env on the server

I considered this! Related to the point above, if we did want to provide a pluggable mechanism into Hydrogen that allowed you to access your hosting runtime's environment variables in a unified way, we could offer this API:

// vite.config.js
hydrogen({
  provideEnvironmentVariables: () => MY_HOSTING_VARIABLES
})

The hosting variables would be injected into a unified import.meta.env object.

A couple issues with this:

  • Not all hosting runtimes are created equal. Vercel uses process.env, Oxygen uses Oxygen.env, which are both available globally. However, Cloudflare is moving to a module syntax which no longer uses globalThis and instead passes the variables to the script event handler. This means we couldn't make this part of Hydrogen's Vite server bundle process, and we'd need to do it at runtime.
  • If we did it at runtime, e.g. in worker.js or server.js, we would need to somehow write/mutate import.meta.env. AFAIK, this would be problematic because Vite assumes this is inlined during build instead of available during runtime. More thought needed here; maybe it's possible.
  • Finally, and perhaps most importantly: it's confusing to developers to have access to import.meta.env in both server and client bundles, but having inconsistent data available. By having a clear separation of concerns (hosting runtime on server; import.meta.env on the client), it's hopefully less confusing.

More guidance for variable usage

With this approach, we'll need to make it ultra clear in our docs when and where certain variables can be used by developers within the Hydrogen framework:

  • Server components: If you need to access variables you're planning to store in the hosting runtime: use Oxygen.env if you're hosting on Oxygen; use other stuff like process.env etc if you're not hosting on Oxygen. If you need access to generic variables set in .env during build time, use import.meta.env.
  • Client components: Use import.meta.env
  • Shared components: Use import.meta.env

How does all of this sound?

from hydrogen-v1.

frandiox avatar frandiox commented on September 18, 2024

@jplhomer That's very clear now, thanks!

provideEnvironmentVariables: () => MY_HOSTING_VARIABLES

I was thinking we could do this in the handleEvent call. Every worker entry point is calling Hydrogen's handleEvent in production, so it can pass a local env or a global process.env / Oxygen.env / etc.

However, since it's clear now that Oxygen Runtime Variables are only for server components, I don't think using import.meta.env is that nice anymore. On the other hand, the current implementation is passing a secrets parameter to server components instead of relying on global variables. The parameter could be renamed to env and default to Oxygen.env. Would the global approach still be preferable (i.e. calling global process.env / Oxygen.env directly in server components)?

Side question: in development, how do we mock the Oxygen Runtime Variables? I assume we read them from .env as well but we will need a different prefix (so that they are not loaded by Vite in the browser). OXYGEN_*? HYDROGEN_PRIVATE_*? The problem with prefixes is that we would be forced to use them in the Admin UI as well. Perhaps using separate .env.oxygen files or something like that could work without prefixes.

from hydrogen-v1.

jplhomer avatar jplhomer commented on September 18, 2024

Would the global approach still be preferable (i.e. calling global process.env / Oxygen.env directly in server components)?

@frandiox That's a good question. For global-ish things like Oxygen.env and process.env, it feels easier to rely on the global, hosting-platform-specific implementations rather than inject them into our rendering pipeline.

On the other hand, for CF Worker's new module syntax, it would be much harder to grab the env vars if we required devs to use a global syntax. In which case, passing env to page server components would make developers' lives easier.

One thing we need to revisit in Hydrogen is our general "plugin" strategy. You've done a really good job in vitedge of providing hooks and injection points. Is there an opportunity for us to do the same in Hydrogen, so it would be easy for a developer to plugin in a bespoke set of variables in worker.js that get forwarded to server components?

Side question: in development, how do we mock the Oxygen Runtime Variables? ... I assume we read them from .env as well but we will need a different prefix (so that they are not loaded by Vite in the browser).

We'll need to write to the bundle manually, similar to how Vite does for its own env-injection, to a Oxygen.env = {} string. We'll be writing all .env variables, not just prefixed ones, for the server bundle.

We don't need to worry about Vite collision, since Vite only picks up prefixed VITE_ variables by default.

Does that answer your question?

from hydrogen-v1.

frandiox avatar frandiox commented on September 18, 2024

@jplhomer Thanks!

In which case, passing env to page server components would make developers' lives easier.

Should we then pass env parameter to server components, and then document that you can either rely on that or on global Oxygen.env if you deploy to Oxygen? Or we don't want two ways of doing things πŸ˜…
If we want to make it work for CFW Module syntax, I think we need this parameter... or tell them to do globalThis.env = env or something ugly like that.

Is there an opportunity for us to do the same in Hydrogen, so it would be easy for a developer to plugin in a bespoke set of variables in worker.js that get forwarded to server components?

This is easy to do in production, but not so in development because we don't run worker.js there. If we adopt the modules syntax, we could import fetch from worker.js or something like that πŸ€”

We'll need to write to the bundle manually, similar to how Vite does for its own env-injection, to a Oxygen.env = {} string. We'll be writing all .env variables, not just prefixed ones, for the server bundle.

If I'm not understanding wrong what you say, we need to:

  1. In development, load all local .env variables to Oxygen.env.
  2. At build time, replace all Oxygen.env.VARIABLE that matches any local .env variable with strings, but leave any Oxygen.env.X that doesn't match in place.
  3. In production, load Oxygen Runtime Variables from admin panel in Oxygen.env, so that Oxygen.env.X cases that didn't match at build time, will access the runtime variables.

Is this correct?

from hydrogen-v1.

jplhomer avatar jplhomer commented on September 18, 2024

Or we don't want two ways of doing things

Haha yeah that's what I'm trying to avoid. I think we should choose one or the other. IMO global access is the most simple, and for CF Modules, we could recommend using a globalThis/module injection pattern similar to how we setCache etc to make it easier.

Is this correct?

Almost what I'm thinking! Here's my modified version:

  1. In development, load all local .env variables to Oxygen.env for the server bundle. Load all local .env variables to import.meta.env to the client bundle.
  2. At build time, do nothing with Oxygen.env variables, since Oxygen will be responsible for injecting them at runtime.
  3. In production, Oxygen loads Runtime Variables from admin panel, and Hydrogen server bundles "just work."

from hydrogen-v1.

mklocke-shopify avatar mklocke-shopify commented on September 18, 2024

Just as a note, the most recent version of oxygen-run has support for loading an env-vars file into the runtime for local development.

./oxygen-run --worker-env-path <path-to-worker.env>

Where worker.env is a simple properties file of the format:

ENV_VAR=some_awesome_api_key
ENV_VAR_2=some_awesome_api_key_2

Additionally it will automatically load any currently defined shell environment variables into the runtime as well if you want to do something like:

ENV_VAR=some_awesome_api_key ./oxygen-run --worker-env-path

from hydrogen-v1.

jplhomer avatar jplhomer commented on September 18, 2024

Update: after chatting with Fran, we think we'll use the public client bundle prefix PUBLIC_ instead of HYDROGEN_PUBLIC_ because it's shorter.

from hydrogen-v1.

frandiox avatar frandiox commented on September 18, 2024

This was implemented in Shopify/hydrogen#304 and released in 0.8.1. Docs are also deployed so we can close this for now.

from hydrogen-v1.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.