Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor and improve Astro config loading flow #7879

Merged
merged 17 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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