netlify edge functions SSR environment variable lookup

[saolsen]

What version of astro are you using?

1.5.0

Are you using an SSR adapter? If so, which one?

@astrojs/netlify/edge-functions

What package manager are you using?

npm

What operating system are you using?

Linux

Describe the Bug

Using import.meta.env with ssr and @astrojs/netlify/edge-functions doesn't work.
For example if you have some code that references import.meta.env.FOO.
The generated code has calls to process.env.FOO but to look up environment values in the netlify edge runtime it needs to call Deno.env.get("FOO").

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-gubo61?file=src/pages/index.astro

Participation

• I am willing to submit a pull request for this issue.

[bluwy]

This may be related to #4416 (comment)

[AirBorne04]

I have created a PR #5103 to address the access to runtime issue (for cloudflare). Is there a way to do a similar thing with Netlify too? Than we can generalize this a little more, since that should be a task for the adapter to handle.

[matthewp]

It probably can't be generic as each runtime does this a bit differently.

@bluwy is there any control we have over when import.meta.env compiles to? If so maybe there could be an option that the adapter controls for this.

[AirBorne04]

It probably can't be generic as each runtime does this a bit differently.

Sorry I did not mean to generalize what the adapter is doing, I meant the access to the specific runtime additions. Like ˋAstro.getRuntime()ˋ and each adapter can add to that runtime via a ˋSymbolˋ or ˋAstro.setRuntime()ˋ or even context, since it is also request related? I will adjust the draft PR.

[bluwy]

@bluwy is there any control we have over when import.meta.env compiles to? If so maybe there could be an option that the adapter controls for this.

Yeah it's currently being transformed to process.env.* at

astro/packages/astro/src/vite-plugin-env/index.ts

Line 37 in ef0c543

if (typeof process.env[key] !== 'undefined') return [key, `process.env.${key}`];

We could expose an adapter option to customize this. I think Cloudflare has a special case where functions env vars can only be accessed throught the env parameter, but this doc says otherwise that it can be read globally.

Either ways I think it would still be a win.

[matthewp]

@AirBorne04 we would need an RFC for a generic solution. Your current cloudflare-only PR I think is fine without one. That's a bit different from the environment variable problem being discussed here though.

[matthewp]

@bluwy oh, I didn't realize it was us. In that case I concur that an option that adapters can implement is the way to go here.

[matthewp]

Maybe build.envKey with a signature of (key: string) => string

[matthewp]

Rough usage example:

'astro:config:setup': ({ updateConfig, mode }) => {
  if(mode === 'build') {
    updateConfig({
      build: {
        envKey: (key: string) => `Deno.env.get("${key}")`
      }
    })
  }
}

[AirBorne04]

I think Cloudflare has a special case where functions env vars can only be accessed throught the env parameter, but this doc says otherwise that it can be read globally.

When deploying pages you need to access it via the function parameters. Only in the workers you can read them globally. Pages and workers are working very similar but unfortunately not exactly the same. I ran a quick test in pages and it outputs:

Attempted to access binding using global in modules.
[w:server] You must use the 2nd `env` parameter passed to exported handlers/Durable Object constructors, or `context.env` with Pages Functions.

so definitely functions parameter for the pages platform, for workers it is either function parameter or global.

[AirBorne04]

@AirBorne04 we would need an RFC for a generic solution. Your current cloudflare-only PR I think is fine without one. That's a bit different from the environment variable problem being discussed here though.

@matthewp just wondered if it makes sense to combine the env vars with the additional platform features or request contexts.

[matthewp]

@AirBorne04 I was talking about this with @bluwy, would it be possible, make sense for the adapter to assign the Env to a global so that it can be accessed via this envKey idea? Adapter would do something like:

const handler = (request, env) => {
  globalThis.WORKER_ENV = env;
  // ...
}

And then in the hook:

'astro:config:setup': ({ updateConfig, mode }) => {
  if(mode === 'build') {
    updateConfig({
      build: {
        envKey: (key: string) => `globalThis.WORKER_ENV["${key}"]`
      }
    })
  }
}

I know this is weird but I can't think of another way to wire up import.meta.env (which is what you want).

Am I right that the env is not different between requests?

[AirBorne04]

@matthewp Yes it is possible to assign them like this to a global. I am not sure how the envKey would be working just like this envKey("NETLIFY_SECRET")? And that would work with runtime specified env vars?

I know this is weird but I can't think of another way to wire up import.meta.env (which is what you want).

Actually i am not a fan of implicit merging of env variables into import.meta.env, cause it would be rather difficult to find the actual source of the definition. I would slightly adjust the naming to something like runtimeEnv(key: string) to be more explicit where they are defined.

Am I right that the env is not different between requests?

I checked this and the env vars are static per deployment

[matthewp]

@AirBorne04 no, if you notice, the envKey returns a string. That string is what gets compiled in place of where import.meta.env is used. So in this case it would be pulling from a global.

Thinking about it, it might need to be a little more complex than that. If someone tries to use it at the top level (outside of a request) then the env will not exist yet and it would through. So that code would need to guard against the global not being defined with.

Very well noted about differentiating between runtime vs. build-time env vars. I agree it's confusing / not clear. But that's the existing behavior so I think we have to stick with that until a better way is figured out.

[AirBorne04]

Ah ok, if that is pushing them into the import.meta.env i think it would be fine to do it this way. I can put together an example and try it out for cloudflare. I am not so familiar with netlify yet, but its a good opportunity to look at it also 😀

For the guard i will also check what can be done there.

[bluwy]

Thinking about how we should implement this, it looks like currently we support:

	Public	Private
Static	✅	❌
Dynamic	❌	✅

I don't see a usecase yet for the two ❌ and it seems to have worked well for us so far. I think we can go with envKey method. For a guard, the envKey should be flexible enough to orchestrate checking in runtime.

@AirBorne04 have you started working on this? I'm also planning to test some things out soon.

[AirBorne04]

yes @bluwy, I did start exploring a bit, you can have a look at my code. So far i was looking into how to make it work and have not managed to get the vars into the import.meta.env but that might be related to the cloudflare runtime as well.

[bluwy]

Thanks @AirBorne04. Looks like you got the implementation pretty close. We'd need to replace using envKey at

astro/packages/astro/src/vite-plugin-env/index.ts

Line 37 in 37d664e

if (typeof process.env[key] !== 'undefined') return [key, `process.env.${key}`];

and it should be complete.

I'll start working on a PR based on your branch too and credit you as a co-author. Will also implement a similar fix for Deno environments.

[bluwy]

Turns out supporting build.envKey is not as simple as I thought 😅 There's a lot to consider to not break things, particularly #3327.

It also looks like we can't support runtime env vars when someone does const { SECRET } = import.meta.env as it's hard to compile that without parsing JS.

Currently working on this branch which simplifies env var replacement & perf, but it still has some failing test. Once that's resolved we can start implementing the cloudflare and deno env support.

[AirBorne04]

@bluwy I did a little more digging and see now why there is an issue. I have compiled the import.meta.env.USER with the cloudflare adapter (Env compilation is handled by the env plugin though) and the replacement in the [[path]].js file was actually process.env.USER which makes total sense when we build for NodeJS, but is not available in cloudflare / netlify.
What is possible though is to actually expand the env vars in the build step.

Build	Runtime
Env Vars (Build system + User)	Env vars (User)
-	Bindings to KV, Durable, R2, D1

So for cloudflare there would be the path of adding the env vars to the SHIM like

const SHIM = `globalThis.process = {
	argv: [],
	env: ${JSON.stringify(process.env)},
};`;

This would cause all the env vars to be included in the build and bloat the file size a little (3Kb in my case) but since this is server side that should be fine. This would also eliminate the need for build.envKey implementation. And the currently promoted define via vite.
I will put a PR together as this is minimal effort, this could also be fixed in the other adapters like netlify and deno.

[bluwy]

@AirBorne04 I'm currently close to a PR too with https://github.com/withastro/astro/tree/env-key and I actually found build.envKey not needed at all too. Currently by assigning the env from cloudflare to the shimmed process.env is enough to fix it, as I now saw other integrations doing the same too.