Support environment variables in Cloudflare and Netlify Edge functions

[bluwy]

Changes

Fix #5105
Fix #5234

• Refactor vite-plugin-env. Does lesser work and should be more performant.
• Handle netlify edge functions environment variables similar to Deno integration.
• Handle cloudflare environment variables by assigning request env to global process.env.

Note: From the linked issue, we discussed of the possibility of build.envKey, but I later found it's not needed as we can assign platform env vars to a shimmed process.env and it should be enough. Currently the Deno integration does this so there's a precedence to follow. Also one less configuration 😬

Note 2: Thanks @AirBorne04 for the discussion and ideas that reached here.

Testing

Existing env var test should pass. Added new tests for Cloudflare and Netlify Edge.

Cloudflare and Netlify Edge tests are previously skipped and I can't seem to get it passing for CI. However it was working when testing locally.

Docs

Updated docs in Cloudflare integration README.

[changeset-bot]

🦋 Changeset detected

Latest commit: b30d076

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[AirBorne04]

hey @bluwy , i love it!

Code

I think for the Cloudflare adapter there are multiple process.env shims in place now, which should probably be cleaned up?

Is this still needed?

astro/packages/integrations/cloudflare/src/shim.ts

Lines 1 to 4 in 3976513

	(globalThis as any).process = {
	argv: [],
	env: {},
	};

This is run at build time so it could be used to map all build env vars into the process env shim

astro/packages/integrations/cloudflare/src/index.ts

Lines 30 to 33 in 3976513

	const SHIM = `globalThis.process = {
	argv: [],
	env: {},
	};`;

astro/packages/integrations/cloudflare/src/index.ts

Line 105 in 3976513

js: SHIM,

If it is changed to env: JSON.stringify(${process.env}) making the guard/proxy obsolete (this is not 100% the same as the request env vars though which can only be added later)

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

Having said that, i think it is fine either way (guard/proxy or build vars) probably the proxy is a bit more straight forward to understand for the user and creates less headaches for maintance.

Docs

For the docs i think the Cloudflare adapter docs for Environment vars need to be adjusted.

[natemoo-re]

Great work! LGTM.

[natemoo-re]

Greatly appreciate these refactors! I knew this plugin was messy and never had time to come back and clean it up. 🎉

[natemoo-re]

WHAT! I totally missed this. That's very smart.

[bluwy]

It's slightly leaning towards a hack that should be fixed in Vite 4 (hopefully) 😬

[bluwy]

I think for the Cloudflare adapter there are multiple process.env shims in place now, which should probably be cleaned up?

Nice find! Yeah we should clean them up.

If it is changed to env: JSON.stringify(${process.env}) making the guard/proxy obsolete (this is not 100% the same as the request env vars though which can only be added later)

Interesting fix, I wondered why you did that before. I'm not really sure about this yet as it brings up the bundle size by default, but we can revisit this idea if the Proxy is confusing for users.

For the docs i think the Cloudflare adapter docs for Environment vars need to be adjusted.

Yes 👍 This should be updated too. Thanks for the review!

[sarah11918]

Thanks for this, @bluwy!

I've left a couple of comments for your consideration regarding the README. You can consider this reviewed by Team Docs now! 🥳

[sarah11918]

Final LGTM approval from me! 🚀