Skip to content

Commit

Permalink
Refactor and improve Astro config loading flow (#7879)
Browse files Browse the repository at this point in the history
  • Loading branch information
bluwy authored Aug 1, 2023
1 parent 9e22038 commit ebf7ebb
Show file tree
Hide file tree
Showing 57 changed files with 737 additions and 822 deletions.
5 changes: 5 additions & 0 deletions .changeset/twenty-oranges-poke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Refactor and improve Astro config loading flow
7 changes: 2 additions & 5 deletions packages/astro/e2e/errors.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect } from '@playwright/test';
import { getErrorOverlayContent, silentLogging, testFactory } from './test-utils.js';
import { getErrorOverlayContent, testFactory } from './test-utils.js';

const test = testFactory({
root: './fixtures/errors/',
Expand All @@ -12,10 +12,7 @@ const test = testFactory({
let devServer;

test.beforeAll(async ({ astro }) => {
devServer = await astro.startDevServer({
// Only test the error overlay, don't print to console
logging: silentLogging,
});
devServer = await astro.startDevServer();
});

test.afterAll(async ({ astro }) => {
Expand Down
3 changes: 2 additions & 1 deletion packages/astro/e2e/test-utils.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { expect, test as testBase } from '@playwright/test';
import fs from 'node:fs/promises';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { loadFixture as baseLoadFixture } from '../test/test-utils.js';

export const isWindows = process.platform === 'win32';
Expand All @@ -24,7 +25,7 @@ export function loadFixture(inlineConfig) {
// without this, the main `loadFixture` helper will resolve relative to `packages/astro/test`
return baseLoadFixture({
...inlineConfig,
root: new URL(inlineConfig.root, import.meta.url).toString(),
root: fileURLToPath(new URL(inlineConfig.root, import.meta.url)),
server: {
port: testFileToPort.get(path.basename(inlineConfig.root)),
},
Expand Down
12 changes: 11 additions & 1 deletion packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import type { PageBuildData } from '../core/build/types';
import type { AstroConfigSchema } from '../core/config';
import type { AstroTimer } from '../core/config/timer';
import type { AstroCookies } from '../core/cookies';
import type { LogOptions } from '../core/logger/core';
import type { LogOptions, LoggerLevel } from '../core/logger/core';
import type { AstroComponentFactory, AstroComponentInstance } from '../runtime/server';
import type { SUPPORTED_MARKDOWN_FILE_EXTENSIONS } from './../core/constants.js';
export type {
Expand Down Expand Up @@ -1331,6 +1331,16 @@ export interface AstroConfig extends z.output<typeof AstroConfigSchema> {
// TypeScript still confirms zod validation matches this type.
integrations: AstroIntegration[];
}
export interface AstroInlineConfig extends AstroUserConfig, AstroInlineOnlyConfig {}
export interface AstroInlineOnlyConfig {
configFile?: string | false;
mode?: RuntimeMode;
logLevel?: LoggerLevel;
/**
* @internal for testing only
*/
logging?: LogOptions;
}

export type ContentEntryModule = {
id: string;
Expand Down
16 changes: 11 additions & 5 deletions packages/astro/src/cli/add/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import ora from 'ora';
import preferredPM from 'preferred-pm';
import prompts from 'prompts';
import type yargs from 'yargs-parser';
import { loadTSConfig, resolveConfigPath } from '../../core/config/index.js';
import { loadTSConfig, resolveConfigPath, resolveRoot } from '../../core/config/index.js';
import {
defaultTSConfig,
presets,
Expand All @@ -23,12 +23,12 @@ import { appendForwardSlash } from '../../core/path.js';
import { apply as applyPolyfill } from '../../core/polyfill.js';
import { parseNpmName } from '../../core/util.js';
import { eventCliSession, telemetry } from '../../events/index.js';
import { createLoggingFromFlags } from '../flags.js';
import { generate, parse, t, visit } from './babel.js';
import { ensureImport } from './imports.js';
import { wrapDefaultExport } from './wrapper.js';

interface AddOptions {
logging: LogOptions;
flags: yargs.Arguments;
}

Expand Down Expand Up @@ -86,7 +86,7 @@ async function getRegistry(): Promise<string> {
}
}

export async function add(names: string[], { flags, logging }: AddOptions) {
export async function add(names: string[], { flags }: AddOptions) {
telemetry.record(eventCliSession('add'));
applyPolyfill();
if (flags.help || names.length === 0) {
Expand Down Expand Up @@ -130,10 +130,12 @@ export async function add(names: string[], { flags, logging }: AddOptions) {

// Some packages might have a common alias! We normalize those here.
const cwd = flags.root;
const logging = createLoggingFromFlags(flags);
const integrationNames = names.map((name) => (ALIASES.has(name) ? ALIASES.get(name)! : name));
const integrations = await validateIntegrations(integrationNames);
let installResult = await tryToInstallIntegrations({ integrations, cwd, flags, logging });
const root = pathToFileURL(cwd ? path.resolve(cwd) : process.cwd());
const rootPath = resolveRoot(cwd);
const root = pathToFileURL(rootPath);
// Append forward slash to compute relative paths
root.href = appendForwardSlash(root.href);

Expand Down Expand Up @@ -199,7 +201,11 @@ export async function add(names: string[], { flags, logging }: AddOptions) {
}
}

const rawConfigPath = await resolveConfigPath({ cwd, flags, fs: fsMod });
const rawConfigPath = await resolveConfigPath({
root: rootPath,
configFile: flags.config,
fs: fsMod,
});
let configURL = rawConfigPath ? pathToFileURL(rawConfigPath) : undefined;

if (configURL) {
Expand Down
30 changes: 20 additions & 10 deletions packages/astro/src/cli/build/index.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,31 @@
import type yargs from 'yargs-parser';
import _build from '../../core/build/index.js';
import type { LogOptions } from '../../core/logger/core.js';
import { loadSettings } from '../load-settings.js';
import { printHelp } from '../../core/messages.js';
import { flagsToAstroInlineConfig } from '../flags.js';

interface BuildOptions {
flags: yargs.Arguments;
logging: LogOptions;
}

export async function build({ flags, logging }: BuildOptions) {
const settings = await loadSettings({ cmd: 'build', flags, logging });
if (!settings) return;
export async function build({ flags }: BuildOptions) {
if (flags?.help || flags?.h) {
printHelp({
commandName: 'astro build',
usage: '[...flags]',
tables: {
Flags: [
['--drafts', `Include Markdown draft pages in the build.`],
['--help (-h)', 'See all available flags.'],
],
},
description: `Builds your site for deployment.`,
});
return;
}

await _build(settings, {
flags,
logging,
const inlineConfig = flagsToAstroInlineConfig(flags);

await _build(inlineConfig, {
teardownCompiler: true,
mode: flags.mode,
});
}
46 changes: 28 additions & 18 deletions packages/astro/src/cli/check/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,16 @@ import { fileURLToPath, pathToFileURL } from 'node:url';
import ora from 'ora';
import type { Arguments as Flags } from 'yargs-parser';
import type { AstroSettings } from '../../@types/astro';
import { resolveConfig } from '../../core/config/config.js';
import { createNodeLogging } from '../../core/config/logging.js';
import { createSettings } from '../../core/config/settings.js';
import type { LogOptions } from '../../core/logger/core.js';
import { debug, info } from '../../core/logger/core.js';
import { printHelp } from '../../core/messages.js';
import type { ProcessExit, SyncOptions } from '../../core/sync';
import { loadSettings } from '../load-settings.js';
import type { syncInternal } from '../../core/sync';
import { eventCliSession, telemetry } from '../../events/index.js';
import { runHookConfigSetup } from '../../integrations/index.js';
import { flagsToAstroInlineConfig } from '../flags.js';
import { printDiagnostic } from './print.js';

type DiagnosticResult = {
Expand All @@ -31,11 +36,6 @@ export type CheckPayload = {
* Flags passed via CLI
*/
flags: Flags;

/**
* Logging options
*/
logging: LogOptions;
};

type CheckFlags = {
Expand Down Expand Up @@ -77,9 +77,8 @@ const ASTRO_GLOB_PATTERN = '**/*.astro';
*
* @param {CheckPayload} options Options passed {@link AstroChecker}
* @param {Flags} options.flags Flags coming from the CLI
* @param {LogOptions} options.logging Logging options
*/
export async function check({ logging, flags }: CheckPayload): Promise<AstroChecker | undefined> {
export async function check({ flags }: CheckPayload): Promise<AstroChecker | undefined> {
if (flags.help || flags.h) {
printHelp({
commandName: 'astro check',
Expand All @@ -95,8 +94,12 @@ export async function check({ logging, flags }: CheckPayload): Promise<AstroChec
return;
}

const settings = await loadSettings({ cmd: 'check', flags, logging });
if (!settings) return;
// Load settings
const inlineConfig = flagsToAstroInlineConfig(flags);
const logging = createNodeLogging(inlineConfig);
const { userConfig, astroConfig } = await resolveConfig(inlineConfig, 'check');
telemetry.record(eventCliSession('check', userConfig, flags));
const settings = createSettings(astroConfig, fileURLToPath(astroConfig.root));

const checkFlags = parseFlags(flags);
if (checkFlags.watch) {
Expand All @@ -105,7 +108,7 @@ export async function check({ logging, flags }: CheckPayload): Promise<AstroChec
info(logging, 'check', 'Checking files');
}

const { syncCli } = await import('../../core/sync/index.js');
const { syncInternal } = await import('../../core/sync/index.js');
const root = settings.config.root;
const require = createRequire(import.meta.url);
const diagnosticChecker = new AstroCheck(
Expand All @@ -116,7 +119,7 @@ export async function check({ logging, flags }: CheckPayload): Promise<AstroChec
);

return new AstroChecker({
syncCli,
syncInternal,
settings,
fileSystem: fs,
logging,
Expand All @@ -130,7 +133,7 @@ type CheckerConstructor = {

isWatchMode: boolean;

syncCli: (settings: AstroSettings, options: SyncOptions) => Promise<ProcessExit>;
syncInternal: typeof syncInternal;

settings: Readonly<AstroSettings>;

Expand All @@ -148,7 +151,7 @@ type CheckerConstructor = {
export class AstroChecker {
readonly #diagnosticsChecker: AstroCheck;
readonly #shouldWatch: boolean;
readonly #syncCli: (settings: AstroSettings, opts: SyncOptions) => Promise<ProcessExit>;
readonly #syncInternal: CheckerConstructor['syncInternal'];

readonly #settings: AstroSettings;

Expand All @@ -162,14 +165,14 @@ export class AstroChecker {
constructor({
diagnosticChecker,
isWatchMode,
syncCli,
syncInternal,
settings,
fileSystem,
logging,
}: CheckerConstructor) {
this.#diagnosticsChecker = diagnosticChecker;
this.#shouldWatch = isWatchMode;
this.#syncCli = syncCli;
this.#syncInternal = syncInternal;
this.#logging = logging;
this.#settings = settings;
this.#fs = fileSystem;
Expand Down Expand Up @@ -223,7 +226,14 @@ export class AstroChecker {
* @param openDocuments Whether the operation should open all `.astro` files
*/
async #checkAllFiles(openDocuments: boolean): Promise<CheckResult> {
const processExit = await this.#syncCli(this.#settings, {
// Run `astro:config:setup` before syncing to initialize integrations.
// We do this manually as we're calling `syncInternal` directly.
const syncSettings = await runHookConfigSetup({
settings: this.#settings,
logging: this.#logging,
command: 'build',
});
const processExit = await this.#syncInternal(syncSettings, {
logging: this.#logging,
fs: this.#fs,
});
Expand Down
46 changes: 25 additions & 21 deletions packages/astro/src/cli/dev/index.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,35 @@
import fs from 'node:fs';
import { cyan } from 'kleur/colors';
import type yargs from 'yargs-parser';
import { resolveConfigPath, resolveFlags } from '../../core/config/index.js';
import devServer from '../../core/dev/index.js';
import { info, type LogOptions } from '../../core/logger/core.js';
import { handleConfigError, loadSettings } from '../load-settings.js';
import { printHelp } from '../../core/messages.js';
import { flagsToAstroInlineConfig } from '../flags.js';

interface DevOptions {
flags: yargs.Arguments;
logging: LogOptions;
}

export async function dev({ flags, logging }: DevOptions) {
const settings = await loadSettings({ cmd: 'dev', flags, logging });
if (!settings) return;
export async function dev({ flags }: DevOptions) {
if (flags.help || flags.h) {
printHelp({
commandName: 'astro dev',
usage: '[...flags]',
tables: {
Flags: [
['--port', `Specify which port to run on. Defaults to 3000.`],
['--host', `Listen on all addresses, including LAN and public addresses.`],
['--host <custom-address>', `Expose on a network IP address at <custom-address>`],
['--open', 'Automatically open the app in the browser on server start'],
['--help (-h)', 'See all available flags.'],
],
},
description: `Check ${cyan(
'https://docs.astro.build/en/reference/cli-reference/#astro-dev'
)} for more information.`,
});
return;
}

const root = flags.root;
const configFlag = resolveFlags(flags).config;
const configFlagPath = configFlag ? await resolveConfigPath({ cwd: root, flags, fs }) : undefined;
const inlineConfig = flagsToAstroInlineConfig(flags);

return await devServer(settings, {
configFlag,
configFlagPath,
flags,
logging,
handleConfigError(e) {
handleConfigError(e, { cmd: 'dev', cwd: root, flags, logging });
info(logging, 'astro', 'Continuing with previous valid configuration\n');
},
});
return await devServer(inlineConfig);
}
Loading

0 comments on commit ebf7ebb

Please sign in to comment.