From b2e17281b941458cfad97a9f4d310ade94090e9e Mon Sep 17 00:00:00 2001 From: Florian Lefebvre Date: Wed, 17 Jul 2024 17:38:32 +0200 Subject: [PATCH] feat(astro): clean sync (#11415) --- .changeset/modern-buses-check.md | 5 + packages/astro/src/actions/index.ts | 22 ++-- packages/astro/src/cli/check/index.ts | 8 +- packages/astro/src/cli/sync/index.ts | 10 +- packages/astro/src/core/build/index.ts | 11 +- packages/astro/src/core/create-vite.ts | 2 - packages/astro/src/core/dev/container.ts | 9 ++ packages/astro/src/core/index.ts | 4 +- packages/astro/src/core/sync/index.ts | 102 ++++++++++-------- .../index.ts => core/sync/setup-env-ts.ts} | 47 +++----- packages/astro/src/integrations/hooks.ts | 8 +- packages/astro/test/astro-sync.test.js | 9 +- packages/astro/test/test-utils.js | 4 +- .../collections-mixed-content-errors.test.js | 17 ++- 14 files changed, 139 insertions(+), 119 deletions(-) create mode 100644 .changeset/modern-buses-check.md rename packages/astro/src/{vite-plugin-inject-env-ts/index.ts => core/sync/setup-env-ts.ts} (68%) diff --git a/.changeset/modern-buses-check.md b/.changeset/modern-buses-check.md new file mode 100644 index 000000000000..3cf7482c1bf1 --- /dev/null +++ b/.changeset/modern-buses-check.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Refactors how `sync` works and when it's called. Fixes an issue with `astro:env` types in dev not being generated diff --git a/packages/astro/src/actions/index.ts b/packages/astro/src/actions/index.ts index e20f8647dd97..f4ab24e2d428 100644 --- a/packages/astro/src/actions/index.ts +++ b/packages/astro/src/actions/index.ts @@ -1,4 +1,4 @@ -import { mkdir, readFile, writeFile } from 'node:fs/promises'; +import fsMod from 'node:fs'; import type { Plugin as VitePlugin } from 'vite'; import type { AstroIntegration } from '../@types/astro.js'; import { ActionsWithoutServerOutputError } from '../core/errors/errors-data.js'; @@ -6,7 +6,7 @@ import { AstroError } from '../core/errors/errors.js'; import { isServerLikeOutput, viteID } from '../core/util.js'; import { ACTIONS_TYPES_FILE, RESOLVED_VIRTUAL_MODULE_ID, VIRTUAL_MODULE_ID } from './consts.js'; -export default function astroActions(): AstroIntegration { +export default function astroActions({ fs = fsMod }: { fs?: typeof fsMod }): AstroIntegration { return { name: VIRTUAL_MODULE_ID, hooks: { @@ -25,7 +25,7 @@ export default function astroActions(): AstroIntegration { define: { 'import.meta.env.ACTIONS_PATH': stringifiedActionsImport, }, - plugins: [vitePluginActions], + plugins: [vitePluginActions(fs)], }, }); @@ -43,13 +43,14 @@ export default function astroActions(): AstroIntegration { await typegen({ stringifiedActionsImport, root: params.config.root, + fs, }); }, }, }; } -const vitePluginActions: VitePlugin = { +const vitePluginActions = (fs: typeof fsMod): VitePlugin => ({ name: VIRTUAL_MODULE_ID, enforce: 'pre', resolveId(id) { @@ -60,7 +61,10 @@ const vitePluginActions: VitePlugin = { async load(id, opts) { if (id !== RESOLVED_VIRTUAL_MODULE_ID) return; - let code = await readFile(new URL('../../templates/actions.mjs', import.meta.url), 'utf-8'); + let code = await fs.promises.readFile( + new URL('../../templates/actions.mjs', import.meta.url), + 'utf-8' + ); if (opts?.ssr) { code += `\nexport * from 'astro/actions/runtime/virtual/server.js';`; } else { @@ -68,14 +72,16 @@ const vitePluginActions: VitePlugin = { } return code; }, -}; +}); async function typegen({ stringifiedActionsImport, root, + fs, }: { stringifiedActionsImport: string; root: URL; + fs: typeof fsMod; }) { const content = `declare module "astro:actions" { type Actions = typeof import(${stringifiedActionsImport})["server"]; @@ -85,6 +91,6 @@ async function typegen({ const dotAstroDir = new URL('.astro/', root); - await mkdir(dotAstroDir, { recursive: true }); - await writeFile(new URL(ACTIONS_TYPES_FILE, dotAstroDir), content); + await fs.promises.mkdir(dotAstroDir, { recursive: true }); + await fs.promises.writeFile(new URL(ACTIONS_TYPES_FILE, dotAstroDir), content); } diff --git a/packages/astro/src/cli/check/index.ts b/packages/astro/src/cli/check/index.ts index 721a0bf6911b..ff7835fdca08 100644 --- a/packages/astro/src/cli/check/index.ts +++ b/packages/astro/src/cli/check/index.ts @@ -28,10 +28,10 @@ export async function check(flags: Arguments) { // NOTE: In the future, `@astrojs/check` can expose a `before lint` hook so that this works during `astro check --watch` too. // For now, we run this once as usually `astro check --watch` is ran alongside `astro dev` which also calls `astro sync`. const { default: sync } = await import('../../core/sync/index.js'); - const inlineConfig = flagsToAstroInlineConfig(flags); - const exitCode = await sync(inlineConfig); - if (exitCode !== 0) { - process.exit(exitCode); + try { + await sync({ inlineConfig: flagsToAstroInlineConfig(flags) }); + } catch (_) { + return process.exit(1); } const { check: checker, parseArgsAsCheckConfig } = checkPackage; diff --git a/packages/astro/src/cli/sync/index.ts b/packages/astro/src/cli/sync/index.ts index 8650bf904655..6849fee70844 100644 --- a/packages/astro/src/cli/sync/index.ts +++ b/packages/astro/src/cli/sync/index.ts @@ -20,8 +20,10 @@ export async function sync({ flags }: SyncOptions) { return 0; } - const inlineConfig = flagsToAstroInlineConfig(flags); - - const exitCode = await _sync(inlineConfig); - return exitCode; + try { + await _sync({ inlineConfig: flagsToAstroInlineConfig(flags), telemetry: true }); + return 0; + } catch (_) { + return 1; + } } diff --git a/packages/astro/src/core/build/index.ts b/packages/astro/src/core/build/index.ts index d4c23b7c6dc4..7933b77f9732 100644 --- a/packages/astro/src/core/build/index.ts +++ b/packages/astro/src/core/build/index.ts @@ -143,11 +143,12 @@ class AstroBuilder { ); await runHookConfigDone({ settings: this.settings, logger: logger }); - const { syncContentCollections } = await import('../sync/index.js'); - const syncRet = await syncContentCollections(this.settings, { logger: logger, fs }); - if (syncRet !== 0) { - return process.exit(syncRet); - } + const { syncInternal } = await import('../sync/index.js'); + await syncInternal({ + settings: this.settings, + logger, + fs, + }); return { viteConfig }; } diff --git a/packages/astro/src/core/create-vite.ts b/packages/astro/src/core/create-vite.ts index bac87bf723f8..4462bb088539 100644 --- a/packages/astro/src/core/create-vite.ts +++ b/packages/astro/src/core/create-vite.ts @@ -25,7 +25,6 @@ import envVitePlugin from '../vite-plugin-env/index.js'; import vitePluginFileURL from '../vite-plugin-fileurl/index.js'; import astroHeadPlugin from '../vite-plugin-head/index.js'; import htmlVitePlugin from '../vite-plugin-html/index.js'; -import { astroInjectEnvTsPlugin } from '../vite-plugin-inject-env-ts/index.js'; import astroIntegrationsContainerPlugin from '../vite-plugin-integrations-container/index.js'; import astroLoadFallbackPlugin from '../vite-plugin-load-fallback/index.js'; import markdownVitePlugin from '../vite-plugin-markdown/index.js'; @@ -142,7 +141,6 @@ export async function createVite( astroScriptsPageSSRPlugin({ settings }), astroHeadPlugin(), astroScannerPlugin({ settings, logger }), - astroInjectEnvTsPlugin({ settings, logger, fs }), astroContentVirtualModPlugin({ fs, settings }), astroContentImportPlugin({ fs, settings, logger }), astroContentAssetPropagationPlugin({ mode, settings }), diff --git a/packages/astro/src/core/dev/container.ts b/packages/astro/src/core/dev/container.ts index 7403eae5b177..aaf6506e9d81 100644 --- a/packages/astro/src/core/dev/container.ts +++ b/packages/astro/src/core/dev/container.ts @@ -14,6 +14,7 @@ import { import { createVite } from '../create-vite.js'; import type { Logger } from '../logger/core.js'; import { apply as applyPolyfill } from '../polyfill.js'; +import { syncInternal } from '../sync/index.js'; export interface Container { fs: typeof nodeFs; @@ -90,6 +91,14 @@ export async function createContainer({ { settings, logger, mode: 'dev', command: 'dev', fs, sync: false } ); await runHookConfigDone({ settings, logger }); + await syncInternal({ + settings, + logger, + skip: { + content: true, + }, + }); + const viteServer = await vite.createServer(viteConfig); const container: Container = { diff --git a/packages/astro/src/core/index.ts b/packages/astro/src/core/index.ts index 31d868311455..e0f9f6c82412 100644 --- a/packages/astro/src/core/index.ts +++ b/packages/astro/src/core/index.ts @@ -22,5 +22,5 @@ export const build = (inlineConfig: AstroInlineConfig) => _build(inlineConfig); * * @experimental The JavaScript API is experimental */ -// Wrap `_sync` to prevent exposing the second internal options parameter -export const sync = (inlineConfig: AstroInlineConfig) => _sync(inlineConfig); +// Wrap `_sync` to prevent exposing internal options +export const sync = (inlineConfig: AstroInlineConfig) => _sync({ inlineConfig }); diff --git a/packages/astro/src/core/sync/index.ts b/packages/astro/src/core/sync/index.ts index 1e43884ac0a7..c95252953619 100644 --- a/packages/astro/src/core/sync/index.ts +++ b/packages/astro/src/core/sync/index.ts @@ -1,21 +1,15 @@ +import { dim } from 'kleur/colors'; import fsMod from 'node:fs'; import { performance } from 'node:perf_hooks'; import { fileURLToPath } from 'node:url'; -import { dim } from 'kleur/colors'; import { type HMRPayload, createServer } from 'vite'; import type { AstroConfig, AstroInlineConfig, AstroSettings } from '../../@types/astro.js'; import { getPackage } from '../../cli/install-package.js'; import { createContentTypesGenerator } from '../../content/index.js'; import { globalContentConfigObserver } from '../../content/utils.js'; import { syncAstroEnv } from '../../env/sync.js'; -import { telemetry } from '../../events/index.js'; -import { eventCliSession } from '../../events/session.js'; -import { runHookConfigSetup } from '../../integrations/hooks.js'; -import { setUpEnvTs } from '../../vite-plugin-inject-env-ts/index.js'; +import { setUpEnvTs } from './setup-env-ts.js'; import { getTimeStat } from '../build/util.js'; -import { resolveConfig } from '../config/config.js'; -import { createNodeLogger } from '../config/logging.js'; -import { createSettings } from '../config/settings.js'; import { createVite } from '../create-vite.js'; import { collectErrorMetadata } from '../errors/dev/utils.js'; import { @@ -28,46 +22,63 @@ import { import type { Logger } from '../logger/core.js'; import { formatErrorMessage } from '../messages.js'; import { ensureProcessNodeEnv } from '../util.js'; - -export type ProcessExit = 0 | 1; +import { createNodeLogger } from '../config/logging.js'; +import { resolveConfig } from '../config/config.js'; +import { createSettings } from '../config/settings.js'; +import { telemetry } from '../../events/index.js'; +import { eventCliSession } from '../../events/session.js'; +import { runHookConfigSetup } from '../../integrations/hooks.js'; export type SyncOptions = { /** * @internal only used for testing */ fs?: typeof fsMod; -}; - -export type SyncInternalOptions = SyncOptions & { logger: Logger; + settings: AstroSettings; + skip?: { + // Must be skipped in dev + content?: boolean; + }; }; type DBPackage = { typegen?: (args: Pick) => Promise; }; +export default async function sync({ + inlineConfig, + fs, + telemetry: _telemetry = false, +}: { inlineConfig: AstroInlineConfig; fs?: typeof fsMod; telemetry?: boolean }) { + ensureProcessNodeEnv('production'); + const logger = createNodeLogger(inlineConfig); + const { astroConfig, userConfig } = await resolveConfig(inlineConfig ?? {}, 'sync'); + if (_telemetry) { + telemetry.record(eventCliSession('sync', userConfig)); + } + let settings = await createSettings(astroConfig, inlineConfig.root); + settings = await runHookConfigSetup({ + command: 'build', + settings, + logger, + }); + return await syncInternal({ settings, logger, fs }); +} + /** * Generates TypeScript types for all Astro modules. This sets up a `src/env.d.ts` file for type inferencing, * and defines the `astro:content` module for the Content Collections API. * * @experimental The JavaScript API is experimental */ -export default async function sync( - inlineConfig: AstroInlineConfig, - options?: SyncOptions -): Promise { - ensureProcessNodeEnv('production'); - const logger = createNodeLogger(inlineConfig); - const { userConfig, astroConfig } = await resolveConfig(inlineConfig ?? {}, 'sync'); - telemetry.record(eventCliSession('sync', userConfig)); - - const _settings = await createSettings(astroConfig, fileURLToPath(astroConfig.root)); - - const settings = await runHookConfigSetup({ - settings: _settings, - logger: logger, - command: 'build', - }); +export async function syncInternal({ + logger, + fs = fsMod, + settings, + skip, +}: SyncOptions): Promise { + const cwd = fileURLToPath(settings.config.root); const timerStart = performance.now(); const dbPackage = await getPackage( @@ -75,26 +86,28 @@ export default async function sync( logger, { optional: true, - cwd: inlineConfig.root, + cwd, }, [] ); try { - await dbPackage?.typegen?.(astroConfig); - const exitCode = await syncContentCollections(settings, { ...options, logger }); - if (exitCode !== 0) return exitCode; - syncAstroEnv(settings, options?.fs); + await dbPackage?.typegen?.(settings.config); + if (!skip?.content) { + await syncContentCollections(settings, { fs, logger }); + } + syncAstroEnv(settings, fs); - logger.info(null, `Types generated ${dim(getTimeStat(timerStart, performance.now()))}`); - return 0; + await setUpEnvTs({ settings, logger, fs }); + logger.info('types', `Generated ${dim(getTimeStat(timerStart, performance.now()))}`); } catch (err) { const error = createSafeError(err); logger.error( - 'content', + 'types', formatErrorMessage(collectErrorMetadata(error), logger.level() === 'debug') + '\n' ); - return 1; + // Will return exit code 1 in CLI + throw error; } } @@ -112,10 +125,10 @@ export default async function sync( * @param {LogOptions} options.logging Logging options * @return {Promise} */ -export async function syncContentCollections( +async function syncContentCollections( settings: AstroSettings, - { logger, fs }: SyncInternalOptions -): Promise { + { logger, fs }: Required> +): Promise { // Needed to load content config const tempViteServer = await createServer( await createVite( @@ -143,7 +156,7 @@ export async function syncContentCollections( const contentTypesGenerator = await createContentTypesGenerator({ contentConfigObserver: globalContentConfigObserver, logger: logger, - fs: fs ?? fsMod, + fs, settings, viteServer: tempViteServer, }); @@ -159,7 +172,6 @@ export async function syncContentCollections( case 'no-content-dir': default: logger.debug('types', 'No content directory found. Skipping type generation.'); - return 0; } } } catch (e) { @@ -179,8 +191,4 @@ export async function syncContentCollections( } finally { await tempViteServer.close(); } - - await setUpEnvTs({ settings, logger, fs: fs ?? fsMod }); - - return 0; } diff --git a/packages/astro/src/vite-plugin-inject-env-ts/index.ts b/packages/astro/src/core/sync/setup-env-ts.ts similarity index 68% rename from packages/astro/src/vite-plugin-inject-env-ts/index.ts rename to packages/astro/src/core/sync/setup-env-ts.ts index 3ebecce2dd51..1363b0da8fb8 100644 --- a/packages/astro/src/vite-plugin-inject-env-ts/index.ts +++ b/packages/astro/src/core/sync/setup-env-ts.ts @@ -2,36 +2,12 @@ import type fsMod from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { bold } from 'kleur/colors'; -import { type Plugin, normalizePath } from 'vite'; -import type { AstroSettings } from '../@types/astro.js'; -import { ACTIONS_TYPES_FILE } from '../actions/consts.js'; -import { CONTENT_TYPES_FILE } from '../content/consts.js'; -import { type Logger } from '../core/logger/core.js'; -import { ENV_TYPES_FILE } from '../env/constants.js'; - -export function getEnvTsPath({ srcDir }: { srcDir: URL }) { - return new URL('env.d.ts', srcDir); -} - -export function astroInjectEnvTsPlugin({ - settings, - logger, - fs, -}: { - settings: AstroSettings; - logger: Logger; - fs: typeof fsMod; -}): Plugin { - return { - name: 'astro-inject-env-ts', - // Use `post` to ensure project setup is complete - // Ex. `.astro` types have been written - enforce: 'post', - async config() { - await setUpEnvTs({ settings, logger, fs }); - }, - }; -} +import { normalizePath } from 'vite'; +import type { AstroSettings } from '../../@types/astro.js'; +import { ACTIONS_TYPES_FILE } from '../../actions/consts.js'; +import { CONTENT_TYPES_FILE } from '../../content/consts.js'; +import { type Logger } from '../logger/core.js'; +import { ENV_TYPES_FILE } from '../../env/constants.js'; function getDotAstroTypeReference({ settings, @@ -58,7 +34,7 @@ export async function setUpEnvTs({ logger: Logger; fs: typeof fsMod; }) { - const envTsPath = getEnvTsPath(settings.config); + const envTsPath = new URL('env.d.ts', settings.config.srcDir); const envTsPathRelativetoRoot = normalizePath( path.relative(fileURLToPath(settings.config.root), fileURLToPath(envTsPath)) ); @@ -80,7 +56,8 @@ export async function setUpEnvTs({ } if (fs.existsSync(envTsPath)) { - let typesEnvContents = await fs.promises.readFile(envTsPath, 'utf-8'); + const initialEnvContents = await fs.promises.readFile(envTsPath, 'utf-8'); + let typesEnvContents = initialEnvContents for (const injectedType of injectedTypes) { if (!injectedType.meetsCondition || (await injectedType.meetsCondition?.())) { @@ -95,8 +72,10 @@ export async function setUpEnvTs({ } } - logger.info('types', `Added ${bold(envTsPathRelativetoRoot)} type declarations.`); - await fs.promises.writeFile(envTsPath, typesEnvContents, 'utf-8'); + if (initialEnvContents !== typesEnvContents) { + logger.info('types', `Updated ${bold(envTsPathRelativetoRoot)} type declarations.`); + await fs.promises.writeFile(envTsPath, typesEnvContents, 'utf-8'); + } } else { // Otherwise, inject the `env.d.ts` file let referenceDefs: string[] = []; diff --git a/packages/astro/src/integrations/hooks.ts b/packages/astro/src/integrations/hooks.ts index 0e58f7e8588f..0da9c883379d 100644 --- a/packages/astro/src/integrations/hooks.ts +++ b/packages/astro/src/integrations/hooks.ts @@ -1,4 +1,4 @@ -import fs from 'node:fs'; +import fsMod from 'node:fs'; import type { AddressInfo } from 'node:net'; import { fileURLToPath } from 'node:url'; import { bold } from 'kleur/colors'; @@ -105,11 +105,13 @@ export async function runHookConfigSetup({ command, logger, isRestart = false, + fs = fsMod }: { settings: AstroSettings; command: 'dev' | 'build' | 'preview'; logger: Logger; isRestart?: boolean; + fs?: typeof fsMod }): Promise { // An adapter is an integration, so if one is provided push it. if (settings.config.adapter) { @@ -117,7 +119,7 @@ export async function runHookConfigSetup({ } if (settings.config.experimental?.actions) { const { default: actionsIntegration } = await import('../actions/index.js'); - settings.config.integrations.push(actionsIntegration()); + settings.config.integrations.push(actionsIntegration({ fs })); } let updatedConfig: AstroConfig = { ...settings.config }; @@ -532,7 +534,7 @@ export async function runHookBuildDone({ cacheManifest, }: RunHookBuildDone) { const dir = isServerLikeOutput(config) ? config.build.client : config.outDir; - await fs.promises.mkdir(dir, { recursive: true }); + await fsMod.promises.mkdir(dir, { recursive: true }); for (const integration of config.integrations) { if (integration?.hooks?.['astro:build:done']) { diff --git a/packages/astro/test/astro-sync.test.js b/packages/astro/test/astro-sync.test.js index 11152f77b2d8..0e22a36c8d9a 100644 --- a/packages/astro/test/astro-sync.test.js +++ b/packages/astro/test/astro-sync.test.js @@ -3,6 +3,7 @@ import * as fs from 'node:fs'; import { before, describe, it } from 'node:test'; import ts from 'typescript'; import { loadFixture } from './test-utils.js'; +import { fileURLToPath } from 'node:url'; const createFixture = () => { /** @type {Awaited>} */ @@ -47,10 +48,10 @@ const createFixture = () => { }, }; - const code = await astroFixture.sync({}, { fs: fsMock }); - if (code !== 0) { - throw new Error(`Process error code ${code}`); - } + await astroFixture.sync({ + inlineConfig: { root: fileURLToPath(new URL(root, import.meta.url)) }, + fs: fsMock, + }); }, /** @param {string} path */ thenFileShouldExist(path) { diff --git a/packages/astro/test/test-utils.js b/packages/astro/test/test-utils.js index d68d64e3870c..025fe63359e6 100644 --- a/packages/astro/test/test-utils.js +++ b/packages/astro/test/test-utils.js @@ -161,9 +161,7 @@ export async function loadFixture(inlineConfig) { process.env.NODE_ENV = 'production'; return build(mergeConfig(inlineConfig, extraInlineConfig), { teardownCompiler: false }); }, - sync: async (extraInlineConfig = {}, opts) => { - return sync(mergeConfig(inlineConfig, extraInlineConfig), opts); - }, + sync, check: async (opts) => { return await check(opts); }, diff --git a/packages/astro/test/units/dev/collections-mixed-content-errors.test.js b/packages/astro/test/units/dev/collections-mixed-content-errors.test.js index 0086b51e83ac..d63e42d53323 100644 --- a/packages/astro/test/units/dev/collections-mixed-content-errors.test.js +++ b/packages/astro/test/units/dev/collections-mixed-content-errors.test.js @@ -6,8 +6,19 @@ import { createFsWithFallback } from '../test-utils.js'; const root = new URL('../../fixtures/content-mixed-errors/', import.meta.url); -async function sync({ fs, config = {} }) { - return _sync({ ...config, root: fileURLToPath(root), logLevel: 'silent' }, { fs }); +async function sync({ fs }) { + try { + await _sync({ + inlineConfig: { + root: fileURLToPath(root), + logLevel: 'silent', + }, + fs, + }); + return 0; + } catch (_) { + return 1; + } } describe('Content Collections - mixed content errors', () => { @@ -114,7 +125,7 @@ title: Post const res = await sync({ fs }); assert.equal(res, 0); } catch (e) { - expect.fail(0, 1, `Did not expect sync to throw: ${e.message}`); + assert.fail(`Did not expect sync to throw: ${e.message}`); } }); });