diff options
| -rw-r--r-- | .changeset/brown-clocks-press.md | 5 | ||||
| -rw-r--r-- | packages/astro/src/cli/index.ts | 76 | ||||
| -rw-r--r-- | packages/astro/src/core/config/config.ts | 11 | ||||
| -rw-r--r-- | packages/astro/src/core/dev/container.ts | 41 | ||||
| -rw-r--r-- | packages/astro/src/core/dev/dev.ts | 39 | ||||
| -rw-r--r-- | packages/astro/src/core/dev/index.ts | 9 | ||||
| -rw-r--r-- | packages/astro/src/core/dev/restart.ts | 197 | ||||
| -rw-r--r-- | packages/astro/test/units/dev/restart.test.js | 68 | ||||
| -rw-r--r-- | packages/astro/test/units/test-utils.js | 16 |
9 files changed, 379 insertions, 83 deletions
diff --git a/.changeset/brown-clocks-press.md b/.changeset/brown-clocks-press.md new file mode 100644 index 000000000..72da3e649 --- /dev/null +++ b/.changeset/brown-clocks-press.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Surface astro.config errors to the user diff --git a/packages/astro/src/cli/index.ts b/packages/astro/src/cli/index.ts index 954628d68..5f9cb2452 100644 --- a/packages/astro/src/cli/index.ts +++ b/packages/astro/src/cli/index.ts @@ -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 } diff --git a/packages/astro/src/core/config/config.ts b/packages/astro/src/core/config/config.ts index 2e71a94c8..ec96bb8c5 100644 --- a/packages/astro/src/core/config/config.ts +++ b/packages/astro/src/core/config/config.ts @@ -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(); } @@ -137,6 +140,7 @@ interface LoadConfigOptions { logging: LogOptions; /** Invalidate when reloading a previously loaded config */ isRestart?: boolean; + fsMod?: typeof fs; } /** @@ -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({ @@ -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); diff --git a/packages/astro/src/core/dev/container.ts b/packages/astro/src/core/dev/container.ts index 5626aaf6d..e98cbfc4c 100644 --- a/packages/astro/src/core/dev/container.ts +++ b/packages/astro/src/core/dev/container.ts @@ -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'; @@ -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>; } @@ -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> { @@ -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({ @@ -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 diff --git a/packages/astro/src/core/dev/dev.ts b/packages/astro/src/core/dev/dev.ts index 78d25e9a7..5868424b1 100644 --- a/packages/astro/src/core/dev/dev.ts +++ b/packages/astro/src/core/dev/dev.ts @@ -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>; } @@ -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) @@ -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, @@ -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(); }, }; } diff --git a/packages/astro/src/core/dev/index.ts b/packages/astro/src/core/dev/index.ts index 52366b0bf..70247b659 100644 --- a/packages/astro/src/core/dev/index.ts +++ b/packages/astro/src/core/dev/index.ts @@ -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'; diff --git a/packages/astro/src/core/dev/restart.ts b/packages/astro/src/core/dev/restart.ts new file mode 100644 index 000000000..515d226d6 --- /dev/null +++ b/packages/astro/src/core/dev/restart.ts @@ -0,0 +1,197 @@ +import type { AstroSettings } from '../../@types/astro'; +import type { Container, CreateContainerParams } from './container'; +import * as vite from 'vite'; +import { createSettings, openConfig } from '../config/index.js'; +import { info } from '../logger/core.js'; +import { createContainer, isStarted, startContainer } from './container.js'; +import { createSafeError } from '../errors/index.js'; + +async function createRestartedContainer(container: Container, settings: AstroSettings): Promise<Container> { + const { + logging, + fs, + resolvedRoot, + configFlag, + configFlagPath + } = container; + const needsStart = isStarted(container); + const newContainer = await createContainer({ + isRestart: true, + logging, + settings, + fs, + root: resolvedRoot, + configFlag, + configFlagPath, + }); + + if(needsStart) { + await startContainer(newContainer); + } + + return newContainer; +} + +export function shouldRestartContainer({ + settings, + configFlag, + configFlagPath, + restartInFlight +}: Container, changedFile: string): boolean { + if(restartInFlight) return false; + + let shouldRestart = false; + + // If the config file changed, reload the config and restart the server. + if(configFlag) { + if(!!configFlagPath) { + shouldRestart = vite.normalizePath(configFlagPath) === vite.normalizePath(changedFile); + } + } + // Otherwise, watch for any astro.config.* file changes in project root + else { + const exp = new RegExp(`.*astro\.config\.((mjs)|(cjs)|(js)|(ts))$`); + const normalizedChangedFile = vite.normalizePath(changedFile); + shouldRestart = exp.test(normalizedChangedFile); + } + + 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) => vite.normalizePath(path) === vite.normalizePath(changedFile) + ); + } + + return shouldRestart; +} + +interface RestartContainerParams { + container: Container; + flags: any; + logMsg: string; + handleConfigError: (err: Error) => Promise<void> | void; + beforeRestart?: () => void; +} + +export async function restartContainer({ + container, + flags, + logMsg, + handleConfigError, + beforeRestart +}: RestartContainerParams): Promise<{ container: Container, error: Error | null }> { + const { + logging, + close, + resolvedRoot, + settings: existingSettings + } = container; + container.restartInFlight = true; + + //console.clear(); // TODO move this + if(beforeRestart) { + beforeRestart() + } + try { + const newConfig = await openConfig({ + cwd: resolvedRoot, + flags, + cmd: 'dev', + logging, + isRestart: true, + fsMod: container.fs, + }); + info(logging, 'astro', logMsg + '\n'); + let astroConfig = newConfig.astroConfig; + const settings = createSettings(astroConfig, resolvedRoot); + await close(); + return { + container: await createRestartedContainer(container, settings), + error: null + }; + } catch (_err) { + const error = createSafeError(_err); + await handleConfigError(error); + await close(); + info(logging, 'astro', 'Continuing with previous valid configuration\n'); + return { + container: await createRestartedContainer(container, existingSettings), + error + }; + } +} + +export interface CreateContainerWithAutomaticRestart { + flags: any; + params: CreateContainerParams; + handleConfigError?: (error: Error) => void | Promise<void>; + beforeRestart?: () => void; +} + +interface Restart { + container: Container; + restarted: () => Promise<Error | null>; +} + +export async function createContainerWithAutomaticRestart({ + flags, + handleConfigError = (_e: Error) => {}, + beforeRestart, + params +}: CreateContainerWithAutomaticRestart): Promise<Restart> { + const initialContainer = await createContainer(params); + let resolveRestart: (value: Error | null) => void; + let restartComplete = new Promise<Error | null>(resolve => { + resolveRestart = resolve; + }); + + let restart: Restart = { + container: initialContainer, + restarted() { + return restartComplete; + } + }; + + function handleServerRestart(logMsg: string) { + // eslint-disable-next-line @typescript-eslint/no-shadow + const container = restart.container; + return async function(changedFile: string) { + if(shouldRestartContainer(container, changedFile)) { + const { container: newContainer, error } = await restartContainer({ + beforeRestart, + container, + flags, + logMsg, + async handleConfigError(err) { + // Send an error message to the client if one is connected. + await handleConfigError(err); + container.viteServer.ws.send({ + type: 'error', + err: { + message: err.message, + stack: err.stack || '' + } + }); + } + }); + restart.container = newContainer; + // Add new watches because this is a new container with a new Vite server + addWatches(); + resolveRestart(error); + restartComplete = new Promise<Error | null>(resolve => { + resolveRestart = resolve; + }); + } + } + } + + // Set up watches + function addWatches() { + const watcher = restart.container.viteServer.watcher; + watcher.on('change', handleServerRestart('Configuration updated. Restarting...')); + watcher.on('unlink', handleServerRestart('Configuration removed. Restarting...')); + watcher.on('add', handleServerRestart('Configuration added. Restarting...')); + } + addWatches(); + return restart; +} diff --git a/packages/astro/test/units/dev/restart.test.js b/packages/astro/test/units/dev/restart.test.js new file mode 100644 index 000000000..10907a4f6 --- /dev/null +++ b/packages/astro/test/units/dev/restart.test.js @@ -0,0 +1,68 @@ +import { expect } from 'chai'; +import * as cheerio from 'cheerio'; + +import { createContainerWithAutomaticRestart, runInContainer } from '../../../dist/core/dev/index.js'; +import { createFs, createRequestAndResponse } from '../test-utils.js'; + +const root = new URL('../../fixtures/alias/', import.meta.url); + +describe('dev container restarts', () => { + it('Surfaces config errors on restarts', async () => { + const fs = createFs( + { + '/src/pages/index.astro': ` + <html> + <head><title>Test</title></head> + <body> + <h1>Test</h1> + </body> + </html> + `, + '/astro.config.mjs': ` + + ` + }, + root + ); + + let restart = await createContainerWithAutomaticRestart({ + params: { fs, root } + }); + + try { + let r = createRequestAndResponse({ + method: 'GET', + url: '/', + }); + restart.container.handle(r.req, r.res); + let html = await r.text(); + const $ = cheerio.load(html); + expect(r.res.statusCode).to.equal(200); + expect($('h1')).to.have.a.lengthOf(1); + + // Create an error + let restartComplete = restart.restarted(); + fs.writeFileFromRootSync('/astro.config.mjs', 'const foo = bar'); + + // Vite watches the real filesystem, so we have to mock this part. It's not so bad. + restart.container.viteServer.watcher.emit('change', fs.getFullyResolvedPath('/astro.config.mjs')); + + // Wait for the restart to finish + let hmrError = await restartComplete; + expect(hmrError).to.not.be.a('undefined'); + + // Do it a second time to make sure we are still watching + + restartComplete = restart.restarted(); + fs.writeFileFromRootSync('/astro.config.mjs', 'const foo = bar2'); + + // Vite watches the real filesystem, so we have to mock this part. It's not so bad. + restart.container.viteServer.watcher.emit('change', fs.getFullyResolvedPath('/astro.config.mjs')); + + hmrError = await restartComplete; + expect(hmrError).to.not.be.a('undefined'); + } finally { + await restart.container.close(); + } + }); +}); diff --git a/packages/astro/test/units/test-utils.js b/packages/astro/test/units/test-utils.js index 4440b7a6f..45e1e50a9 100644 --- a/packages/astro/test/units/test-utils.js +++ b/packages/astro/test/units/test-utils.js @@ -6,12 +6,26 @@ import npath from 'path'; import { unixify } from './correct-path.js'; class MyVolume extends Volume { + #root = ''; + constructor(root) { + super(); + this.#root = root; + } + + getFullyResolvedPath(pth) { + return npath.posix.join(this.#root, pth); + } + existsSync(p) { if (p instanceof URL) { p = fileURLToPath(p); } return super.existsSync(p); } + + writeFileFromRootSync(pth, ...rest) { + return super.writeFileSync(this.getFullyResolvedPath(pth), ...rest); + } } export function createFs(json, root) { @@ -25,7 +39,7 @@ export function createFs(json, root) { structure[fullpath] = value; } - const fs = new MyVolume(); + const fs = new MyVolume(root); fs.fromJSON(structure); return fs; } |