diff options
17 files changed, 132 insertions, 96 deletions
diff --git a/.changeset/cold-needles-heal.md b/.changeset/cold-needles-heal.md new file mode 100644 index 000000000..c8a8a4ef8 --- /dev/null +++ b/.changeset/cold-needles-heal.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Improve environment variable handling performance diff --git a/.changeset/hungry-coats-mix.md b/.changeset/hungry-coats-mix.md new file mode 100644 index 000000000..afab5229e --- /dev/null +++ b/.changeset/hungry-coats-mix.md @@ -0,0 +1,5 @@ +--- +'@astrojs/cloudflare': patch +--- + +Fix environment variables usage in worker output and warn if environment variables are accessedd too early diff --git a/.changeset/orange-melons-roll.md b/.changeset/orange-melons-roll.md new file mode 100644 index 000000000..7cba7f40e --- /dev/null +++ b/.changeset/orange-melons-roll.md @@ -0,0 +1,5 @@ +--- +'@astrojs/netlify': patch +--- + +Fix environment variables usage in edge functions diff --git a/packages/astro/src/vite-plugin-env/index.ts b/packages/astro/src/vite-plugin-env/index.ts index 08332d717..fdde4b59e 100644 --- a/packages/astro/src/vite-plugin-env/index.ts +++ b/packages/astro/src/vite-plugin-env/index.ts @@ -8,41 +8,44 @@ interface EnvPluginOptions { settings: AstroSettings; } -function getPrivateEnv(viteConfig: vite.ResolvedConfig, astroConfig: AstroConfig) { +function getPrivateEnv( + viteConfig: vite.ResolvedConfig, + astroConfig: AstroConfig +): Record<string, string> { let envPrefixes: string[] = ['PUBLIC_']; if (viteConfig.envPrefix) { envPrefixes = Array.isArray(viteConfig.envPrefix) ? viteConfig.envPrefix : [viteConfig.envPrefix]; } + + // Loads environment variables from `.env` files and `process.env` const fullEnv = loadEnv( viteConfig.mode, viteConfig.envDir ?? fileURLToPath(astroConfig.root), '' ); - const privateKeys = Object.keys(fullEnv).filter((key) => { - // don't inject `PUBLIC_` variables, Vite handles that for us - for (const envPrefix of envPrefixes) { - if (key.startsWith(envPrefix)) return false; - } - // Otherwise, this is a private variable defined in an `.env` file - return true; - }); - if (privateKeys.length === 0) { - return null; + const privateEnv: Record<string, string> = {}; + for (const key in fullEnv) { + // Ignore public env var + if (envPrefixes.every((prefix) => !key.startsWith(prefix))) { + if (typeof process.env[key] !== 'undefined') { + privateEnv[key] = `process.env.${key}`; + } else { + privateEnv[key] = JSON.stringify(fullEnv[key]); + } + } } - return Object.fromEntries( - privateKeys.map((key) => { - if (typeof process.env[key] !== 'undefined') return [key, `process.env.${key}`]; - return [key, JSON.stringify(fullEnv[key])]; - }) - ); + privateEnv.SITE = astroConfig.site ? `'${astroConfig.site}'` : 'undefined'; + privateEnv.SSR = JSON.stringify(true); + privateEnv.BASE_URL = astroConfig.base ? `'${astroConfig.base}'` : 'undefined'; + return privateEnv; } function getReferencedPrivateKeys(source: string, privateEnv: Record<string, any>): Set<string> { const references = new Set<string>(); - for (const key of Object.keys(privateEnv)) { + for (const key in privateEnv) { if (source.includes(key)) { references.add(key); } @@ -51,91 +54,70 @@ function getReferencedPrivateKeys(source: string, privateEnv: Record<string, any } export default function envVitePlugin({ settings }: EnvPluginOptions): vite.PluginOption { - let privateEnv: Record<string, any> | null; - let config: vite.ResolvedConfig; - let replacements: Record<string, string>; - let pattern: RegExp | undefined; + let privateEnv: Record<string, string>; + let viteConfig: vite.ResolvedConfig; const { config: astroConfig } = settings; return { name: 'astro:vite-plugin-env', enforce: 'pre', configResolved(resolvedConfig) { - config = resolvedConfig; + viteConfig = resolvedConfig; }, async transform(source, id, options) { - const ssr = options?.ssr === true; - - if (!ssr) { - return; - } - - if (!source.includes('import.meta') || !/\benv\b/.test(source)) { + if (!options?.ssr || !source.includes('import.meta.env')) { return; } - if (typeof privateEnv === 'undefined') { - privateEnv = getPrivateEnv(config, astroConfig); - if (privateEnv) { - privateEnv.SITE = astroConfig.site ? `'${astroConfig.site}'` : 'undefined'; - privateEnv.SSR = JSON.stringify(true); - privateEnv.BASE_URL = astroConfig.base ? `'${astroConfig.base}'` : undefined; - const entries = Object.entries(privateEnv).map(([key, value]) => [ - `import.meta.env.${key}`, - value, - ]); - replacements = Object.fromEntries(entries); - // These additional replacements are needed to match Vite - replacements = Object.assign(replacements, { - 'import.meta.env.SITE': astroConfig.site ? `'${astroConfig.site}'` : 'undefined', - 'import.meta.env.SSR': JSON.stringify(true), - 'import.meta.env.BASE_URL': astroConfig.base ? `'${astroConfig.base}'` : undefined, - // This catches destructed `import.meta.env` calls, - // BUT we only want to inject private keys referenced in the file. - // We overwrite this value on a per-file basis. - 'import.meta.env': `({})`, - }); - pattern = new RegExp( - // Do not allow preceding '.', but do allow preceding '...' for spread operations - '(?<!(?<!\\.\\.)\\.)\\b(' + - Object.keys(replacements) - .map((str) => { - return str.replace(/[-[\]/{}()*+?.\\^$|]/g, '\\$&'); - }) - .join('|') + - // prevent trailing assignments - ')\\b(?!\\s*?=[^=])', - 'g' - ); - } - } - - if (!privateEnv || !pattern) return; - const references = getReferencedPrivateKeys(source, privateEnv); - if (references.size === 0) return; - // Find matches for *private* env and do our own replacement. - const s = new MagicString(source); + let s: MagicString | undefined; + const pattern = new RegExp( + // Do not allow preceding '.', but do allow preceding '...' for spread operations + '(?<!(?<!\\.\\.)\\.)\\b(' + + // Captures `import.meta.env.*` calls and replace with `privateEnv` + `import\\.meta\\.env\\.(.+?)` + + '|' + + // This catches destructed `import.meta.env` calls, + // BUT we only want to inject private keys referenced in the file. + // We overwrite this value on a per-file basis. + 'import\\.meta\\.env' + + // prevent trailing assignments + ')\\b(?!\\s*?=[^=])', + 'g' + ); + let references: Set<string>; let match: RegExpExecArray | null; while ((match = pattern.exec(source))) { - const start = match.index; - const end = start + match[0].length; - let replacement = '' + replacements[match[1]]; + let replacement: string | undefined; // If we match exactly `import.meta.env`, define _only_ referenced private variables if (match[0] === 'import.meta.env') { + privateEnv ??= getPrivateEnv(viteConfig, astroConfig); + references ??= getReferencedPrivateKeys(source, privateEnv); replacement = `(Object.assign(import.meta.env,{`; for (const key of references.values()) { replacement += `${key}:${privateEnv[key]},`; } replacement += '}))'; } - s.overwrite(start, end, replacement); + // If we match `import.meta.env.*`, replace with private env + else if (match[2]) { + privateEnv ??= getPrivateEnv(viteConfig, astroConfig); + replacement = privateEnv[match[2]]; + } + if (replacement) { + const start = match.index; + const end = start + match[0].length; + s ??= new MagicString(source); + s.overwrite(start, end, replacement); + } } - return { - code: s.toString(), - map: s.generateMap(), - }; + if (s) { + return { + code: s.toString(), + map: s.generateMap(), + }; + } }, }; } diff --git a/packages/integrations/cloudflare/README.md b/packages/integrations/cloudflare/README.md index 21891c053..0a8012b1c 100644 --- a/packages/integrations/cloudflare/README.md +++ b/packages/integrations/cloudflare/README.md @@ -92,16 +92,18 @@ To do this: ## Environment Variables -As Cloudflare Pages Functions [provides environment variables differently](https://developers.cloudflare.com/pages/platform/functions/#adding-environment-variables-locally), private environment variables needs to be set through [`vite.define`](https://vitejs.dev/config/shared-options.html#define) to work in builds. +As Cloudflare Pages Functions [provides environment variables per request](https://developers.cloudflare.com/pages/platform/functions/#adding-environment-variables-locally), you can only access private environment variables when a request has happened. Usually, this means moving environment variable access inside a function. ```js -// astro.config.mjs -export default { - vite: { - define: { - 'process.env.MY_SECRET': JSON.stringify(process.env.MY_SECRET), - }, - }, +// pages/[id].json.js + +export function get({ params }) { + // Access environment variables per request inside a function + const serverUrl = import.meta.env.SERVER_URL; + const result = await fetch(serverUrl + "/user/" + params.id); + return { + body: await result.text(), + }; } ``` diff --git a/packages/integrations/cloudflare/src/server.advanced.ts b/packages/integrations/cloudflare/src/server.advanced.ts index ac040c9b2..cb83dd994 100644 --- a/packages/integrations/cloudflare/src/server.advanced.ts +++ b/packages/integrations/cloudflare/src/server.advanced.ts @@ -1,7 +1,8 @@ -import './shim.js'; - import type { SSRManifest } from 'astro'; import { App } from 'astro/app'; +import { getProcessEnvProxy } from './util.js'; + +process.env = getProcessEnvProxy(); type Env = { ASSETS: { fetch: (req: Request) => Promise<Response> }; @@ -12,6 +13,8 @@ export function createExports(manifest: SSRManifest) { const app = new App(manifest, false); const fetch = async (request: Request, env: Env, context: any) => { + process.env = env as any; + const { origin, pathname } = new URL(request.url); // static assets diff --git a/packages/integrations/cloudflare/src/server.directory.ts b/packages/integrations/cloudflare/src/server.directory.ts index e7463b84c..321f37e18 100644 --- a/packages/integrations/cloudflare/src/server.directory.ts +++ b/packages/integrations/cloudflare/src/server.directory.ts @@ -1,7 +1,8 @@ -import './shim.js'; - import type { SSRManifest } from 'astro'; import { App } from 'astro/app'; +import { getProcessEnvProxy } from './util.js'; + +process.env = getProcessEnvProxy(); export function createExports(manifest: SSRManifest) { const app = new App(manifest, false); @@ -14,6 +15,8 @@ export function createExports(manifest: SSRManifest) { request: Request; next: (request: Request) => void; } & Record<string, unknown>) => { + process.env = runtimeEnv.env as any; + const { origin, pathname } = new URL(request.url); // static assets if (manifest.assets.has(pathname)) { diff --git a/packages/integrations/cloudflare/src/shim.ts b/packages/integrations/cloudflare/src/shim.ts deleted file mode 100644 index 1a4a6ee9b..000000000 --- a/packages/integrations/cloudflare/src/shim.ts +++ /dev/null @@ -1,4 +0,0 @@ -(globalThis as any).process = { - argv: [], - env: {}, -}; diff --git a/packages/integrations/cloudflare/src/util.ts b/packages/integrations/cloudflare/src/util.ts new file mode 100644 index 000000000..a44780863 --- /dev/null +++ b/packages/integrations/cloudflare/src/util.ts @@ -0,0 +1,16 @@ +export function getProcessEnvProxy() { + return new Proxy( + {}, + { + get: (target, prop) => { + console.warn( + // NOTE: \0 prevents Vite replacement + `Unable to access \`import.meta\0.env.${prop.toString()}\` on initialization ` + + `as the Cloudflare platform only provides the environment variables per request. ` + + `Please move the environment variable access inside a function ` + + `that's only called after a request has been received.` + ); + }, + } + ); +} diff --git a/packages/integrations/cloudflare/test/basics.test.js b/packages/integrations/cloudflare/test/basics.test.js index 7127a3f68..a6229bbb2 100644 --- a/packages/integrations/cloudflare/test/basics.test.js +++ b/packages/integrations/cloudflare/test/basics.test.js @@ -24,6 +24,7 @@ describe.skip('Basic app', () => { let html = await res.text(); let $ = cheerio.load(html); expect($('h1').text()).to.equal('Testing'); + expect($('#env').text()).to.equal('secret'); } finally { stop(); } diff --git a/packages/integrations/cloudflare/test/fixtures/basics/astro.config.mjs b/packages/integrations/cloudflare/test/fixtures/basics/astro.config.mjs index bf47a0a33..105247b1b 100644 --- a/packages/integrations/cloudflare/test/fixtures/basics/astro.config.mjs +++ b/packages/integrations/cloudflare/test/fixtures/basics/astro.config.mjs @@ -1,6 +1,9 @@ import { defineConfig } from 'astro/config'; import cloudflare from '@astrojs/cloudflare'; +// test env var +process.env.SECRET_STUFF = 'secret' + export default defineConfig({ adapter: cloudflare(), output: 'server', diff --git a/packages/integrations/cloudflare/test/fixtures/basics/src/pages/index.astro b/packages/integrations/cloudflare/test/fixtures/basics/src/pages/index.astro index 9c077e2a3..8d372399c 100644 --- a/packages/integrations/cloudflare/test/fixtures/basics/src/pages/index.astro +++ b/packages/integrations/cloudflare/test/fixtures/basics/src/pages/index.astro @@ -4,5 +4,6 @@ </head> <body> <h1>Testing</h1> + <div id="env">{import.meta.env.SECRET_STUFF}</div> </body> </html> diff --git a/packages/integrations/cloudflare/test/wrangler.toml b/packages/integrations/cloudflare/test/wrangler.toml new file mode 100644 index 000000000..6e2d864b0 --- /dev/null +++ b/packages/integrations/cloudflare/test/wrangler.toml @@ -0,0 +1,4 @@ +# for tests only + +[vars] +SECRET_STUFF = "secret" diff --git a/packages/integrations/netlify/src/integration-edge-functions.ts b/packages/integrations/netlify/src/integration-edge-functions.ts index f9e5f449d..8e16e0595 100644 --- a/packages/integrations/netlify/src/integration-edge-functions.ts +++ b/packages/integrations/netlify/src/integration-edge-functions.ts @@ -14,7 +14,7 @@ interface BuildConfig { const SHIM = `globalThis.process = { argv: [], - env: {}, + env: Deno.env.toObject(), };`; export function getAdapter(): AstroAdapter { diff --git a/packages/integrations/netlify/test/edge-functions/edge-basic.test.ts b/packages/integrations/netlify/test/edge-functions/edge-basic.test.ts index 5cce36c7f..8283fe579 100644 --- a/packages/integrations/netlify/test/edge-functions/edge-basic.test.ts +++ b/packages/integrations/netlify/test/edge-functions/edge-basic.test.ts @@ -4,6 +4,9 @@ import { runBuild } from './test-utils.ts'; import { assertEquals, assert, DOMParser } from './deps.ts'; // @ts-ignore +Deno.env.set('SECRET_STUFF', 'secret'); + +// @ts-ignore Deno.test({ // TODO: debug why build cannot be found in "await import" ignore: true, @@ -23,6 +26,9 @@ Deno.test({ const div = doc.querySelector('#react'); assert(div, 'div exists'); + const envDiv = doc.querySelector('#env'); + assertEquals(envDiv?.innerText, 'secret'); + await close(); }, }); diff --git a/packages/integrations/netlify/test/edge-functions/fixtures/edge-basic/astro.config.mjs b/packages/integrations/netlify/test/edge-functions/fixtures/edge-basic/astro.config.mjs index a08e8e89d..310088c88 100644 --- a/packages/integrations/netlify/test/edge-functions/fixtures/edge-basic/astro.config.mjs +++ b/packages/integrations/netlify/test/edge-functions/fixtures/edge-basic/astro.config.mjs @@ -2,6 +2,9 @@ import { defineConfig } from 'astro/config'; import { netlifyEdgeFunctions } from '@astrojs/netlify'; import react from "@astrojs/react"; +// test env var +process.env.SECRET_STUFF = 'secret' + export default defineConfig({ adapter: netlifyEdgeFunctions({ dist: new URL('./dist/', import.meta.url), diff --git a/packages/integrations/netlify/test/edge-functions/fixtures/edge-basic/src/pages/index.astro b/packages/integrations/netlify/test/edge-functions/fixtures/edge-basic/src/pages/index.astro index 80d2eed75..1247ba8f6 100644 --- a/packages/integrations/netlify/test/edge-functions/fixtures/edge-basic/src/pages/index.astro +++ b/packages/integrations/netlify/test/edge-functions/fixtures/edge-basic/src/pages/index.astro @@ -10,5 +10,6 @@ import ReactComponent from '../components/React.jsx'; <li><a href="/two/">Two</a></li> </ul> <ReactComponent /> + <div id="env">{import.meta.env.SECRET_STUFF}</div> </body> </html> |