From 41121fbe00e144d4d93835811e1c4349664d9003 Mon Sep 17 00:00:00 2001 From: Florian Lefebvre Date: Tue, 25 Jun 2024 14:51:55 +0200 Subject: [PATCH] fix(astro): astro sync and astro:env (#11326) --- .changeset/dirty-ducks-play.md | 5 + packages/astro/src/config/index.ts | 2 +- packages/astro/src/core/build/index.ts | 2 +- packages/astro/src/core/create-vite.ts | 5 +- packages/astro/src/core/dev/container.ts | 2 +- packages/astro/src/core/sync/index.ts | 4 +- packages/astro/src/env/sync.ts | 30 +++++ packages/astro/src/env/vite-plugin-env.ts | 108 ++++-------------- packages/astro/test/astro-sync.test.js | 16 ++- .../astro.config.mjs | 13 +++ .../astro-env-required-public/package.json | 8 ++ .../astro-env-required-public/tsconfig.json | 3 + pnpm-lock.yaml | 6 + 13 files changed, 113 insertions(+), 91 deletions(-) create mode 100644 .changeset/dirty-ducks-play.md create mode 100644 packages/astro/src/env/sync.ts create mode 100644 packages/astro/test/fixtures/astro-env-required-public/astro.config.mjs create mode 100644 packages/astro/test/fixtures/astro-env-required-public/package.json create mode 100644 packages/astro/test/fixtures/astro-env-required-public/tsconfig.json diff --git a/.changeset/dirty-ducks-play.md b/.changeset/dirty-ducks-play.md new file mode 100644 index 0000000000..62523832e5 --- /dev/null +++ b/.changeset/dirty-ducks-play.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes a case where running `astro sync` when using the experimental `astro:env` feature would fail if environment variables were missing diff --git a/packages/astro/src/config/index.ts b/packages/astro/src/config/index.ts index 0a047c84da..fb97288a6b 100644 --- a/packages/astro/src/config/index.ts +++ b/packages/astro/src/config/index.ts @@ -48,7 +48,7 @@ export function getViteConfig( astroContentListenPlugin({ settings, logger, fs }), ], }, - { settings, logger, mode } + { settings, logger, mode, sync: false } ); await runHookConfigDone({ settings, logger }); return mergeConfig(viteConfig, userViteConfig); diff --git a/packages/astro/src/core/build/index.ts b/packages/astro/src/core/build/index.ts index 50085d5813..d4c23b7c6d 100644 --- a/packages/astro/src/core/build/index.ts +++ b/packages/astro/src/core/build/index.ts @@ -139,7 +139,7 @@ class AstroBuilder { middlewareMode: true, }, }, - { settings: this.settings, logger: this.logger, mode: 'build', command: 'build' } + { settings: this.settings, logger: this.logger, mode: 'build', command: 'build', sync: false } ); await runHookConfigDone({ settings: this.settings, logger: logger }); diff --git a/packages/astro/src/core/create-vite.ts b/packages/astro/src/core/create-vite.ts index 1627dde2cc..e07150c39d 100644 --- a/packages/astro/src/core/create-vite.ts +++ b/packages/astro/src/core/create-vite.ts @@ -47,6 +47,7 @@ interface CreateViteOptions { // will be undefined when using `getViteConfig` command?: 'dev' | 'build'; fs?: typeof nodeFs; + sync: boolean; } const ALWAYS_NOEXTERNAL = [ @@ -74,7 +75,7 @@ const ONLY_DEV_EXTERNAL = [ /** Return a base vite config as a common starting point for all Vite commands. */ export async function createVite( commandConfig: vite.InlineConfig, - { settings, logger, mode, command, fs = nodeFs }: CreateViteOptions + { settings, logger, mode, command, fs = nodeFs, sync }: CreateViteOptions ): Promise { const astroPkgsConfig = await crawlFrameworkPkgs({ root: fileURLToPath(settings.config.root), @@ -137,7 +138,7 @@ export async function createVite( // the build to run very slow as the filewatcher is triggered often. mode !== 'build' && vitePluginAstroServer({ settings, logger, fs }), envVitePlugin({ settings }), - astroEnv({ settings, mode, fs }), + astroEnv({ settings, mode, fs, sync }), markdownVitePlugin({ settings, logger }), htmlVitePlugin(), mdxVitePlugin(), diff --git a/packages/astro/src/core/dev/container.ts b/packages/astro/src/core/dev/container.ts index 58962de0f7..0102a87cd0 100644 --- a/packages/astro/src/core/dev/container.ts +++ b/packages/astro/src/core/dev/container.ts @@ -74,7 +74,7 @@ export async function createContainer({ include: rendererClientEntries, }, }, - { settings, logger, mode: 'dev', command: 'dev', fs } + { settings, logger, mode: 'dev', command: 'dev', fs, sync: false } ); await runHookConfigDone({ settings, logger }); const viteServer = await vite.createServer(viteConfig); diff --git a/packages/astro/src/core/sync/index.ts b/packages/astro/src/core/sync/index.ts index 0b4bd1af46..0649f07b57 100644 --- a/packages/astro/src/core/sync/index.ts +++ b/packages/astro/src/core/sync/index.ts @@ -27,6 +27,7 @@ import { import type { Logger } from '../logger/core.js'; import { formatErrorMessage } from '../messages.js'; import { ensureProcessNodeEnv } from '../util.js'; +import { syncAstroEnv } from '../../env/sync.js'; export type ProcessExit = 0 | 1; @@ -83,6 +84,7 @@ export default async function sync( await dbPackage?.typegen?.(astroConfig); const exitCode = await syncContentCollections(settings, { ...options, logger }); if (exitCode !== 0) return exitCode; + syncAstroEnv(settings, options?.fs); logger.info(null, `Types generated ${dim(getTimeStat(timerStart, performance.now()))}`); return 0; @@ -123,7 +125,7 @@ export async function syncContentCollections( ssr: { external: [] }, logLevel: 'silent', }, - { settings, logger, mode: 'build', command: 'build', fs } + { settings, logger, mode: 'build', command: 'build', fs, sync: true } ) ); diff --git a/packages/astro/src/env/sync.ts b/packages/astro/src/env/sync.ts new file mode 100644 index 0000000000..eb7cc939e0 --- /dev/null +++ b/packages/astro/src/env/sync.ts @@ -0,0 +1,30 @@ +import type { AstroSettings } from '../@types/astro.js'; +import fsMod from 'node:fs'; +import { getEnvFieldType } from './validators.js'; +import { ENV_TYPES_FILE, TYPES_TEMPLATE_URL } from './constants.js'; + +export function syncAstroEnv(settings: AstroSettings, fs = fsMod) { + if (!settings.config.experimental.env) { + return; + } + + const schema = settings.config.experimental.env.schema ?? {}; + + let client = ''; + let server = ''; + + for (const [key, options] of Object.entries(schema)) { + const str = `export const ${key}: ${getEnvFieldType(options)}; \n`; + if (options.context === 'client') { + client += str; + } else { + server += str; + } + } + + const template = fs.readFileSync(TYPES_TEMPLATE_URL, 'utf-8'); + const dts = template.replace('// @@CLIENT@@', client).replace('// @@SERVER@@', server); + + fs.mkdirSync(settings.dotAstroDir, { recursive: true }); + fs.writeFileSync(new URL(ENV_TYPES_FILE, settings.dotAstroDir), dts, 'utf-8'); +} diff --git a/packages/astro/src/env/vite-plugin-env.ts b/packages/astro/src/env/vite-plugin-env.ts index 7ca5e4b0a0..92e5cea6d7 100644 --- a/packages/astro/src/env/vite-plugin-env.ts +++ b/packages/astro/src/env/vite-plugin-env.ts @@ -1,17 +1,15 @@ import type fsMod from 'node:fs'; import { fileURLToPath } from 'node:url'; -import { type Plugin, loadEnv } from 'vite'; +import { loadEnv, type Plugin } from 'vite'; import type { AstroSettings } from '../@types/astro.js'; import { AstroError, AstroErrorData } from '../core/errors/index.js'; import { - ENV_TYPES_FILE, MODULE_TEMPLATE_URL, - TYPES_TEMPLATE_URL, VIRTUAL_MODULES_IDS, VIRTUAL_MODULES_IDS_VALUES, } from './constants.js'; import type { EnvSchema } from './schema.js'; -import { getEnvFieldType, validateEnvVariable } from './validators.js'; +import { validateEnvVariable } from './validators.js'; // TODO: reminders for when astro:env comes out of experimental // Types should always be generated (like in types/content.d.ts). That means the client module will be empty @@ -23,14 +21,16 @@ interface AstroEnvVirtualModPluginParams { settings: AstroSettings; mode: 'dev' | 'build' | string; fs: typeof fsMod; + sync: boolean; } export function astroEnv({ settings, mode, fs, + sync, }: AstroEnvVirtualModPluginParams): Plugin | undefined { - if (!settings.config.experimental.env) { + if (!settings.config.experimental.env || sync) { return; } const schema = settings.config.experimental.env.schema ?? {}; @@ -54,23 +54,10 @@ export function astroEnv({ const validatedVariables = validatePublicVariables({ schema, loadedEnv }); - const clientTemplates = getClientTemplates({ validatedVariables }); - const serverTemplates = getServerTemplates({ validatedVariables, schema, fs }); - templates = { - client: clientTemplates.module, - server: serverTemplates.module, + ...getTemplates(schema, fs, validatedVariables), internal: `export const schema = ${JSON.stringify(schema)};`, }; - generateDts({ - settings, - fs, - content: getDts({ - fs, - client: clientTemplates.types, - server: serverTemplates.types, - }), - }); }, buildEnd() { templates = null; @@ -104,19 +91,6 @@ function resolveVirtualModuleId(id: T): `\0${T}` { return `\0${id}`; } -function generateDts({ - content, - settings, - fs, -}: { - content: string; - settings: AstroSettings; - fs: typeof fsMod; -}) { - fs.mkdirSync(settings.dotAstroDir, { recursive: true }); - fs.writeFileSync(new URL(ENV_TYPES_FILE, settings.dotAstroDir), content, 'utf-8'); -} - function validatePublicVariables({ schema, loadedEnv, @@ -152,55 +126,22 @@ function validatePublicVariables({ return valid; } -function getDts({ - client, - server, - fs, -}: { - client: string; - server: string; - fs: typeof fsMod; -}) { - const template = fs.readFileSync(TYPES_TEMPLATE_URL, 'utf-8'); - - return template.replace('// @@CLIENT@@', client).replace('// @@SERVER@@', server); -} - -function getClientTemplates({ - validatedVariables, -}: { - validatedVariables: ReturnType; -}) { - let module = ''; - let types = ''; - - for (const { key, type, value } of validatedVariables.filter((e) => e.context === 'client')) { - module += `export const ${key} = ${JSON.stringify(value)};`; - types += `export const ${key}: ${type}; \n`; - } - - return { - module, - types, - }; -} - -function getServerTemplates({ - validatedVariables, - schema, - fs, -}: { - validatedVariables: ReturnType; - schema: EnvSchema; - fs: typeof fsMod; -}) { - let module = fs.readFileSync(MODULE_TEMPLATE_URL, 'utf-8'); - let types = ''; +function getTemplates( + schema: EnvSchema, + fs: typeof fsMod, + validatedVariables: ReturnType +) { + let client = ''; + let server = fs.readFileSync(MODULE_TEMPLATE_URL, 'utf-8'); let onSetGetEnv = ''; - for (const { key, type, value } of validatedVariables.filter((e) => e.context === 'server')) { - module += `export const ${key} = ${JSON.stringify(value)};`; - types += `export const ${key}: ${type}; \n`; + for (const { key, value, context } of validatedVariables) { + const str = `export const ${key} = ${JSON.stringify(value)};`; + if (context === 'client') { + client += str; + } else { + server += str; + } } for (const [key, options] of Object.entries(schema)) { @@ -208,15 +149,14 @@ function getServerTemplates({ continue; } - types += `export const ${key}: ${getEnvFieldType(options)}; \n`; - module += `export let ${key} = _internalGetSecret(${JSON.stringify(key)});\n`; + server += `export let ${key} = _internalGetSecret(${JSON.stringify(key)});\n`; onSetGetEnv += `${key} = reset ? undefined : _internalGetSecret(${JSON.stringify(key)});\n`; } - module = module.replace('// @@ON_SET_GET_ENV@@', onSetGetEnv); + server = server.replace('// @@ON_SET_GET_ENV@@', onSetGetEnv); return { - module, - types, + client, + server, }; } diff --git a/packages/astro/test/astro-sync.test.js b/packages/astro/test/astro-sync.test.js index 85f6551e66..11152f77b2 100644 --- a/packages/astro/test/astro-sync.test.js +++ b/packages/astro/test/astro-sync.test.js @@ -47,7 +47,10 @@ const createFixture = () => { }, }; - await astroFixture.sync({}, { fs: fsMock }); + const code = await astroFixture.sync({}, { fs: fsMock }); + if (code !== 0) { + throw new Error(`Process error code ${code}`); + } }, /** @param {string} path */ thenFileShouldExist(path) { @@ -164,6 +167,17 @@ describe('astro sync', () => { `/// ` ); }); + + it('Does not throw if a public variable is required', async () => { + let error = null; + try { + await fixture.whenSyncing('./fixtures/astro-env-required-public/'); + } catch (e) { + error = e; + } + + assert.equal(error, null, 'Syncing should not throw astro:env validation errors'); + }); }); describe('Astro Actions', () => { diff --git a/packages/astro/test/fixtures/astro-env-required-public/astro.config.mjs b/packages/astro/test/fixtures/astro-env-required-public/astro.config.mjs new file mode 100644 index 0000000000..c668e1e21f --- /dev/null +++ b/packages/astro/test/fixtures/astro-env-required-public/astro.config.mjs @@ -0,0 +1,13 @@ +import { defineConfig, envField } from 'astro/config'; + +// https://astro.build/config +export default defineConfig({ + experimental: { + env: { + schema: { + FOO: envField.string({ context: "client", access: "public" }), + BAR: envField.number({ context: "server", access: "public" }), + } + } + } +}); diff --git a/packages/astro/test/fixtures/astro-env-required-public/package.json b/packages/astro/test/fixtures/astro-env-required-public/package.json new file mode 100644 index 0000000000..9e6372807c --- /dev/null +++ b/packages/astro/test/fixtures/astro-env-required-public/package.json @@ -0,0 +1,8 @@ +{ + "name": "@test/astro-env-required-public", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/astro-env-required-public/tsconfig.json b/packages/astro/test/fixtures/astro-env-required-public/tsconfig.json new file mode 100644 index 0000000000..d78f81ec4e --- /dev/null +++ b/packages/astro/test/fixtures/astro-env-required-public/tsconfig.json @@ -0,0 +1,3 @@ +{ + "extends": "astro/tsconfigs/base" +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 289bf40e73..0c646b5c5d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2121,6 +2121,12 @@ importers: specifier: workspace:* version: link:../../.. + packages/astro/test/fixtures/astro-env-required-public: + dependencies: + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/astro-env-server-fail: dependencies: astro: