Skip to content

Commit

Permalink
Surface configuration errors to the client (#5273)
Browse files Browse the repository at this point in the history 
* Surface configuration errors to the client

* Actually start the container on restart

* Add beforeRestart to clear the console

* Some minor changes, restarted() returns an Error maybe

* Refactor testing code

* Adding a changeset
  • Loading branch information
@matthewp
matthewp committed Nov 3, 2022
1 parent a558cf3 commit c7b9b14
Show file tree
Hide file tree
Showing 9 changed files with 379 additions and 83 deletions.
5 changes: 5 additions & 0 deletions .changeset/brown-clocks-press.md
View file
@@ -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
View file
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
View file
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
View file
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
View file
@@ -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
View file
@@ -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';

0 comments on commit c7b9b14

Please sign in to comment.