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

Surface configuration errors to the client #5273

Merged
merged 6 commits into from
Nov 3, 2022
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/brown-clocks-press.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Surface astro.config errors to the user
76 changes: 15 additions & 61 deletions packages/astro/src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,67 +177,21 @@ async function runCommand(cmd: string, flags: yargs.Arguments) {
// by the end of this switch statement.
switch (cmd) {
case 'dev': {
async function startDevServer({ isRestart = false }: { isRestart?: boolean } = {}) {
const { watcher, stop } = await devServer(settings, { logging, telemetry, isRestart });
let restartInFlight = false;
const configFlag = resolveFlags(flags).config;
const configFlagPath = configFlag
? await resolveConfigPath({ cwd: root, flags })
: undefined;
const resolvedRoot = appendForwardSlash(resolveRoot(root));

const handleServerRestart = (logMsg: string) =>
async function (changedFile: string) {
if (restartInFlight) return;

let shouldRestart = false;

// If the config file changed, reload the config and restart the server.
shouldRestart = configFlag
? // If --config is specified, only watch changes for this file
!!configFlagPath && normalizePath(configFlagPath) === normalizePath(changedFile)
: // Otherwise, watch for any astro.config.* file changes in project root
new RegExp(
`${normalizePath(resolvedRoot)}.*astro\.config\.((mjs)|(cjs)|(js)|(ts))$`
).test(normalizePath(changedFile));

if (!shouldRestart && settings.watchFiles.length > 0) {
// If the config file didn't change, check if any of the watched files changed.
shouldRestart = settings.watchFiles.some(
(path) => normalizePath(path) === normalizePath(changedFile)
);
}

if (!shouldRestart) return;

restartInFlight = true;
console.clear();
try {
const newConfig = await openConfig({
cwd: root,
flags,
cmd,
logging,
isRestart: true,
});
info(logging, 'astro', logMsg + '\n');
let astroConfig = newConfig.astroConfig;
settings = createSettings(astroConfig, root);
await stop();
await startDevServer({ isRestart: true });
} catch (e) {
await handleConfigError(e, { cwd: root, flags, logging });
await stop();
info(logging, 'astro', 'Continuing with previous valid configuration\n');
await startDevServer({ isRestart: true });
}
};

watcher.on('change', handleServerRestart('Configuration updated. Restarting...'));
watcher.on('unlink', handleServerRestart('Configuration removed. Restarting...'));
watcher.on('add', handleServerRestart('Configuration added. Restarting...'));
}
await startDevServer({ isRestart: false });
const configFlag = resolveFlags(flags).config;
const configFlagPath = configFlag
? await resolveConfigPath({ cwd: root, flags })
: undefined;

await devServer(settings, {
configFlag,
configFlagPath,
logging,
telemetry,
handleConfigError(e) {
handleConfigError(e, { cwd: root, flags, logging });
info(logging, 'astro', 'Continuing with previous valid configuration\n');
}
});
return await new Promise(() => {}); // lives forever
}

Expand Down
11 changes: 9 additions & 2 deletions packages/astro/src/core/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,10 @@ export function resolveFlags(flags: Partial<Flags>): CLIFlags {
};
}

export function resolveRoot(cwd?: string): string {
export function resolveRoot(cwd?: string | URL): string {
if(cwd instanceof URL) {
cwd = fileURLToPath(cwd);
}
return cwd ? path.resolve(cwd) : process.cwd();
}

Expand Down Expand Up @@ -137,6 +140,7 @@ interface LoadConfigOptions {
logging: LogOptions;
/** Invalidate when reloading a previously loaded config */
isRestart?: boolean;
fsMod?: typeof fs;
}

/**
Expand Down Expand Up @@ -210,6 +214,7 @@ async function tryLoadConfig(
flags: CLIFlags,
root: string
): Promise<TryLoadConfigResult | undefined> {
const fsMod = configOptions.fsMod ?? fs;
let finallyCleanup = async () => {};
try {
let configPath = await resolveConfigPath({
Expand All @@ -224,7 +229,9 @@ async function tryLoadConfig(
root,
`.temp.${Date.now()}.config${path.extname(configPath)}`
);
await fs.promises.writeFile(tempConfigPath, await fs.promises.readFile(configPath));

const currentConfigContent = await fsMod.promises.readFile(configPath, 'utf-8');
await fs.promises.writeFile(tempConfigPath, currentConfigContent);
finallyCleanup = async () => {
try {
await fs.promises.unlink(tempConfigPath);
Expand Down
41 changes: 36 additions & 5 deletions packages/astro/src/core/dev/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import {
runHookConfigSetup,
runHookServerSetup,
runHookServerStart,
runHookServerDone
} from '../../integrations/index.js';
import { createDefaultDevSettings } from '../config/index.js';
import { createDefaultDevSettings, resolveRoot } from '../config/index.js';
import { createVite } from '../create-vite.js';
import { LogOptions } from '../logger/core.js';
import { appendForwardSlash } from '../path.js';
import { nodeLogDestination } from '../logger/node.js';
import { apply as applyPolyfill } from '../polyfill.js';

Expand All @@ -27,6 +29,10 @@ export interface Container {
settings: AstroSettings;
viteConfig: vite.InlineConfig;
viteServer: vite.ViteDevServer;
resolvedRoot: string;
configFlag: string | undefined;
configFlagPath: string | undefined;
restartInFlight: boolean; // gross
handle: (req: http.IncomingMessage, res: http.ServerResponse) => void;
close: () => Promise<void>;
}
Expand All @@ -38,6 +44,9 @@ export interface CreateContainerParams {
settings?: AstroSettings;
fs?: typeof nodeFs;
root?: string | URL;
// The string passed to --config and the resolved path
configFlag?: string;
configFlagPath?: string;
}

export async function createContainer(params: CreateContainerParams = {}): Promise<Container> {
Expand Down Expand Up @@ -83,20 +92,38 @@ export async function createContainer(params: CreateContainerParams = {}): Promi
const viteServer = await vite.createServer(viteConfig);
runHookServerSetup({ config: settings.config, server: viteServer, logging });

return {
const container: Container = {
configFlag: params.configFlag,
configFlagPath: params.configFlagPath,
fs,
logging,
resolvedRoot: appendForwardSlash(resolveRoot(params.root)),
restartInFlight: false,
settings,
viteConfig,
viteServer,

handle(req, res) {
viteServer.middlewares.handle(req, res, Function.prototype);
},
// TODO deprecate and remove
close() {
return viteServer.close();
},
return closeContainer(container);
}
};

return container;
}

async function closeContainer({
viteServer,
settings,
logging
}: Container) {
await viteServer.close();
await runHookServerDone({
config: settings.config,
logging,
});
}

export async function startContainer({
Expand All @@ -116,6 +143,10 @@ export async function startContainer({
return devServerAddressInfo;
}

export function isStarted(container: Container): boolean {
return !!container.viteServer.httpServer?.listening;
}

export async function runInContainer(
params: CreateContainerParams,
callback: (container: Container) => Promise<void> | void
Expand Down
39 changes: 26 additions & 13 deletions packages/astro/src/core/dev/dev.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,26 @@
import type { AstroTelemetry } from '@astrojs/telemetry';
import type { AddressInfo } from 'net';
import type http from 'http';
import { performance } from 'perf_hooks';
import * as vite from 'vite';
import type { AstroSettings } from '../../@types/astro';
import { runHookServerDone } from '../../integrations/index.js';
import { info, LogOptions, warn } from '../logger/core.js';
import * as msg from '../messages.js';
import { createContainer, startContainer } from './container.js';
import { startContainer } from './container.js';
import { createContainerWithAutomaticRestart } from './restart.js';

export interface DevOptions {
configFlag: string | undefined;
configFlagPath: string | undefined;
logging: LogOptions;
telemetry: AstroTelemetry;
handleConfigError: (error: Error) => void;
isRestart?: boolean;
}

export interface DevServer {
address: AddressInfo;
handle: (req: http.IncomingMessage, res: http.ServerResponse<http.IncomingMessage>) => void;
watcher: vite.FSWatcher;
stop(): Promise<void>;
}
Expand All @@ -29,14 +34,20 @@ export default async function dev(
await options.telemetry.record([]);

// Create a container which sets up the Vite server.
const container = await createContainer({
settings,
logging: options.logging,
isRestart: options.isRestart,
const restart = await createContainerWithAutomaticRestart({
flags: {},
handleConfigError: options.handleConfigError,
// eslint-disable-next-line no-console
beforeRestart: () => console.clear(),
params: {
settings,
logging: options.logging,
isRestart: options.isRestart,
}
});

// Start listening to the port
const devServerAddressInfo = await startContainer(container);
const devServerAddressInfo = await startContainer(restart.container);

const site = settings.config.site
? new URL(settings.config.base, settings.config.site)
Expand All @@ -46,7 +57,7 @@ export default async function dev(
null,
msg.serverStart({
startupTime: performance.now() - devStart,
resolvedUrls: container.viteServer.resolvedUrls || { local: [], network: [] },
resolvedUrls: restart.container.viteServer.resolvedUrls || { local: [], network: [] },
host: settings.config.server.host,
site,
isRestart: options.isRestart,
Expand All @@ -57,18 +68,20 @@ export default async function dev(
if (currentVersion.includes('-')) {
warn(options.logging, null, msg.prerelease({ currentVersion }));
}
if (container.viteConfig.server?.fs?.strict === false) {
if (restart.container.viteConfig.server?.fs?.strict === false) {
warn(options.logging, null, msg.fsStrictWarning());
}

return {
address: devServerAddressInfo,
get watcher() {
return container.viteServer.watcher;
return restart.container.viteServer.watcher;
},
handle(req, res) {
return restart.container.handle(req, res);
},
stop: async () => {
await container.close();
await runHookServerDone({ config: settings.config, logging: options.logging });
async stop() {
await restart.container.close();
},
};
}
9 changes: 8 additions & 1 deletion packages/astro/src/core/dev/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,9 @@
export { createContainer, runInContainer, startContainer } from './container.js';
export {
createContainer,
runInContainer,
startContainer
} from './container.js';
export {
createContainerWithAutomaticRestart
} from './restart.js';
export { default } from './dev.js';
Loading