From 2d259cf4abf27a4f0a067bedb32d0459c4fce507 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Thu, 16 Jan 2025 09:05:40 +0000 Subject: [PATCH] fix: better handling of resync and restarts in content layer (#12984) * fix: better handling of resync and restarts * Always regenerate content layer on sync * Formatting --- .changeset/kind-horses-smile.md | 5 ++ .changeset/large-cherries-explain.md | 5 ++ packages/astro/src/content/content-layer.ts | 18 +++++- packages/astro/src/content/loaders/file.ts | 2 + packages/astro/src/content/loaders/glob.ts | 2 + .../astro/src/content/server-listeners.ts | 15 ++--- packages/astro/src/content/types-generator.ts | 9 ++- packages/astro/src/content/watcher.ts | 62 +++++++++++++++++++ packages/astro/src/core/dev/container.ts | 5 +- packages/astro/src/core/dev/restart.ts | 3 + packages/astro/src/core/sync/index.ts | 10 ++- packages/astro/test/content-layer.test.js | 33 +++++++++- .../remark/src/import-plugin-default.ts | 4 +- 13 files changed, 152 insertions(+), 21 deletions(-) create mode 100644 .changeset/kind-horses-smile.md create mode 100644 .changeset/large-cherries-explain.md create mode 100644 packages/astro/src/content/watcher.ts diff --git a/.changeset/kind-horses-smile.md b/.changeset/kind-horses-smile.md new file mode 100644 index 000000000000..720557b2b047 --- /dev/null +++ b/.changeset/kind-horses-smile.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes a bug in dev where files would stop being watched if the Astro config file was edited diff --git a/.changeset/large-cherries-explain.md b/.changeset/large-cherries-explain.md new file mode 100644 index 000000000000..118e4d5945ba --- /dev/null +++ b/.changeset/large-cherries-explain.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes a bug where the content layer would use an outdated version of the Astro config if it was edited in dev diff --git a/packages/astro/src/content/content-layer.ts b/packages/astro/src/content/content-layer.ts index 865da268cc22..6ff356675271 100644 --- a/packages/astro/src/content/content-layer.ts +++ b/packages/astro/src/content/content-layer.ts @@ -22,6 +22,8 @@ import { globalContentConfigObserver, safeStringify, } from './utils.js'; +import { type WrappedWatcher, createWatcherWrapper } from './watcher.js'; +import type { AstroConfig } from '../types/public/index.js'; export interface ContentLayerOptions { store: MutableDataStore; @@ -30,11 +32,12 @@ export interface ContentLayerOptions { watcher?: FSWatcher; } + export class ContentLayer { #logger: Logger; #store: MutableDataStore; #settings: AstroSettings; - #watcher?: FSWatcher; + #watcher?: WrappedWatcher; #lastConfigDigest?: string; #unsubscribe?: () => void; @@ -49,7 +52,9 @@ export class ContentLayer { this.#logger = logger; this.#store = store; this.#settings = settings; - this.#watcher = watcher; + if (watcher) { + this.#watcher = createWatcherWrapper(watcher); + } this.#queue = new PQueue({ concurrency: 1 }); } @@ -79,6 +84,7 @@ export class ContentLayer { dispose() { this.#queue.clear(); this.#unsubscribe?.(); + this.#watcher?.removeAllTrackedListeners(); } async #getGenerateDigest() { @@ -178,7 +184,7 @@ export class ContentLayer { } = this.#settings.config; const astroConfigDigest = safeStringify(hashableConfig); - + const { digest: currentConfigDigest } = contentConfig.config; this.#lastConfigDigest = currentConfigDigest; @@ -213,6 +219,12 @@ export class ContentLayer { if (astroConfigDigest) { await this.#store.metaStore().set('astro-config-digest', astroConfigDigest); } + + if (!options?.loaders?.length) { + // Remove all listeners before syncing, as they will be re-added by the loaders, but not if this is a selective sync + this.#watcher?.removeAllTrackedListeners(); + } + await Promise.all( Object.entries(contentConfig.config.collections).map(async ([name, collection]) => { if (collection.type !== CONTENT_LAYER_TYPE) { diff --git a/packages/astro/src/content/loaders/file.ts b/packages/astro/src/content/loaders/file.ts index d109f95b6994..c37998a762ec 100644 --- a/packages/astro/src/content/loaders/file.ts +++ b/packages/astro/src/content/loaders/file.ts @@ -101,6 +101,8 @@ export function file(fileName: string, options?: FileOptions): Loader { await syncData(filePath, context); + watcher?.add(filePath); + watcher?.on('change', async (changedPath) => { if (changedPath === filePath) { logger.info(`Reloading data from ${fileName}`); diff --git a/packages/astro/src/content/loaders/glob.ts b/packages/astro/src/content/loaders/glob.ts index 706fd9c0d42d..7299c7ca3019 100644 --- a/packages/astro/src/content/loaders/glob.ts +++ b/packages/astro/src/content/loaders/glob.ts @@ -318,6 +318,8 @@ export function glob(globOptions: GlobOptions): Loader { return; } + watcher.add(filePath); + const matchesGlob = (entry: string) => !entry.startsWith('../') && micromatch.isMatch(entry, globOptions.pattern); diff --git a/packages/astro/src/content/server-listeners.ts b/packages/astro/src/content/server-listeners.ts index 33ab2c8959ee..492a5a5160cf 100644 --- a/packages/astro/src/content/server-listeners.ts +++ b/packages/astro/src/content/server-listeners.ts @@ -25,14 +25,7 @@ export async function attachContentServerListeners({ }: ContentServerListenerParams) { const contentPaths = getContentPaths(settings.config, fs); if (!settings.config.legacy?.collections) { - const contentGenerator = await createContentTypesGenerator({ - fs, - settings, - logger, - viteServer, - contentConfigObserver: globalContentConfigObserver, - }); - await contentGenerator.init(); + await attachListeners(); } else if (fs.existsSync(contentPaths.contentDir)) { logger.debug( 'content', @@ -71,9 +64,9 @@ export async function attachContentServerListeners({ viteServer.watcher.on('addDir', (entry) => contentGenerator.queueEvent({ name: 'addDir', entry }), ); - viteServer.watcher.on('change', (entry) => - contentGenerator.queueEvent({ name: 'change', entry }), - ); + viteServer.watcher.on('change', (entry) => { + contentGenerator.queueEvent({ name: 'change', entry }); + }); viteServer.watcher.on('unlink', (entry) => { contentGenerator.queueEvent({ name: 'unlink', entry }); }); diff --git a/packages/astro/src/content/types-generator.ts b/packages/astro/src/content/types-generator.ts index fe76400bba30..92a83367cfee 100644 --- a/packages/astro/src/content/types-generator.ts +++ b/packages/astro/src/content/types-generator.ts @@ -292,7 +292,14 @@ export async function createContentTypesGenerator({ entry: pathToFileURL(rawEvent.entry), name: rawEvent.name, }; - if (!event.entry.pathname.startsWith(contentPaths.contentDir.pathname)) return; + + if (settings.config.legacy.collections) { + if (!event.entry.pathname.startsWith(contentPaths.contentDir.pathname)) { + return; + } + } else if (contentPaths.config.url.pathname !== event.entry.pathname) { + return; + } events.push(event); diff --git a/packages/astro/src/content/watcher.ts b/packages/astro/src/content/watcher.ts new file mode 100644 index 000000000000..a1ae0284e4ec --- /dev/null +++ b/packages/astro/src/content/watcher.ts @@ -0,0 +1,62 @@ +import type { FSWatcher } from 'vite'; + +type WatchEventName = 'add' | 'addDir' | 'change' | 'unlink' | 'unlinkDir'; +type WatchEventCallback = (path: string) => void; + +export type WrappedWatcher = FSWatcher & { + removeAllTrackedListeners(): void; +}; + +// This lets us use the standard Vite FSWatcher, but also track all listeners added by the content loaders +// We do this so we can remove them all when we re-sync. +export function createWatcherWrapper(watcher: FSWatcher): WrappedWatcher { + const listeners = new Map>(); + + const handler: ProxyHandler = { + get(target, prop, receiver) { + // Intercept the 'on' method and track the listener + if (prop === 'on') { + return function (event: WatchEventName, callback: WatchEventCallback) { + if (!listeners.has(event)) { + listeners.set(event, new Set()); + } + + // Track the listener + listeners.get(event)!.add(callback); + + // Call the original method + return Reflect.get(target, prop, receiver).call(target, event, callback); + }; + } + + // Intercept the 'off' method + if (prop === 'off') { + return function (event: WatchEventName, callback: WatchEventCallback) { + // Remove from our tracking + listeners.get(event)?.delete(callback); + + // Call the original method + return Reflect.get(target, prop, receiver).call(target, event, callback); + }; + } + + // Adds a function to remove all listeners added by us + if (prop === 'removeAllTrackedListeners') { + return function () { + for (const [event, callbacks] of listeners.entries()) { + for (const callback of callbacks) { + target.off(event, callback); + } + callbacks.clear(); + } + listeners.clear(); + }; + } + + // Return original method/property for everything else + return Reflect.get(target, prop, receiver); + }, + }; + + return new Proxy(watcher, handler) as WrappedWatcher; +} diff --git a/packages/astro/src/core/dev/container.ts b/packages/astro/src/core/dev/container.ts index e82f65a0b980..7425a1a3bdb3 100644 --- a/packages/astro/src/core/dev/container.ts +++ b/packages/astro/src/core/dev/container.ts @@ -108,21 +108,22 @@ export async function createContainer({ ssrManifest: devSSRManifest, }, ); + const viteServer = await vite.createServer(viteConfig); await syncInternal({ settings, mode, logger, skip: { - content: true, + content: !isRestart, cleanup: true, }, force: inlineConfig?.force, manifest, command: 'dev', + watcher: viteServer.watcher, }); - const viteServer = await vite.createServer(viteConfig); const container: Container = { inlineConfig: inlineConfig ?? {}, diff --git a/packages/astro/src/core/dev/restart.ts b/packages/astro/src/core/dev/restart.ts index d6c0d0513d77..e2baa9a38fe9 100644 --- a/packages/astro/src/core/dev/restart.ts +++ b/packages/astro/src/core/dev/restart.ts @@ -13,6 +13,7 @@ import { createSafeError } from '../errors/index.js'; import { formatErrorMessage } from '../messages.js'; import type { Container } from './container.js'; import { createContainer, startContainer } from './container.js'; +import { attachContentServerListeners } from '../../content/server-listeners.js'; async function createRestartedContainer( container: Container, @@ -147,6 +148,8 @@ export async function createContainerWithAutomaticRestart({ // Restart success. Add new watches because this is a new container with a new Vite server restart.container = result; setupContainer(); + await attachContentServerListeners(restart.container); + if (server) { // Vite expects the resolved URLs to be available server.resolvedUrls = result.viteServer.resolvedUrls; diff --git a/packages/astro/src/core/sync/index.ts b/packages/astro/src/core/sync/index.ts index de531cf10184..e73d87e922e3 100644 --- a/packages/astro/src/core/sync/index.ts +++ b/packages/astro/src/core/sync/index.ts @@ -3,7 +3,7 @@ import { dirname, relative } from 'node:path'; import { performance } from 'node:perf_hooks'; import { fileURLToPath } from 'node:url'; import { dim } from 'kleur/colors'; -import { type HMRPayload, createServer } from 'vite'; +import { type FSWatcher, type HMRPayload, createServer } from 'vite'; import { CONTENT_TYPES_FILE } from '../../content/consts.js'; import { getDataStoreFile, globalContentLayer } from '../../content/content-layer.js'; import { createContentTypesGenerator } from '../../content/index.js'; @@ -50,6 +50,7 @@ export type SyncOptions = { }; manifest: ManifestData; command: 'build' | 'dev' | 'sync'; + watcher?: FSWatcher; }; export default async function sync( @@ -120,6 +121,7 @@ export async function syncInternal({ force, manifest, command, + watcher, }: SyncOptions): Promise { const isDev = command === 'dev'; if (force) { @@ -131,6 +133,7 @@ export async function syncInternal({ if (!skip?.content) { await syncContentCollections(settings, { mode, fs, logger, manifest }); settings.timer.start('Sync content layer'); + let store: MutableDataStore | undefined; try { const dataStoreFile = getDataStoreFile(settings, isDev); @@ -142,11 +145,16 @@ export async function syncInternal({ logger.error('content', 'Failed to load content store'); return; } + const contentLayer = globalContentLayer.init({ settings, logger, store, + watcher, }); + if (watcher) { + contentLayer.watchContentConfig(); + } await contentLayer.sync(); if (!skip?.cleanup) { // Free up memory (usually in builds since we only need to use this once) diff --git a/packages/astro/test/content-layer.test.js b/packages/astro/test/content-layer.test.js index 002c097fcdc3..83385c6d2482 100644 --- a/packages/astro/test/content-layer.test.js +++ b/packages/astro/test/content-layer.test.js @@ -1,5 +1,6 @@ import assert from 'node:assert/strict'; import { promises as fs, existsSync } from 'node:fs'; +import { setTimeout } from 'node:timers/promises'; import { sep } from 'node:path'; import { sep as posixSep } from 'node:path/posix'; import { Writable } from 'node:stream'; @@ -323,7 +324,7 @@ describe('Content Layer', () => { devServer = await fixture.startDevServer({ force: true, logger: new Logger({ - level: 'warn', + level: 'info', dest: new Writable({ objectMode: true, write(event, _, callback) { @@ -524,5 +525,35 @@ describe('Content Layer', () => { await fs.rename(newPath, oldPath); } }); + + it('still updates collection when data file is changed after server has restarted via config change', async () => { + await fixture.editFile('astro.config.mjs', (prev) => + prev.replace("'Astro content layer'", "'Astro content layer edited'"), + ); + logs.length = 0; + + // Give time for the server to restart + await setTimeout(5000); + + const rawJsonResponse = await fixture.fetch('/collections.json'); + const initialJson = devalue.parse(await rawJsonResponse.text()); + assert.equal(initialJson.jsonLoader[0].data.temperament.includes('Bouncy'), false); + + await fixture.editFile('/src/data/dogs.json', (prev) => { + const data = JSON.parse(prev); + data[0].temperament.push('Bouncy'); + return JSON.stringify(data, null, 2); + }); + + await fixture.onNextDataStoreChange(); + const updatedJsonResponse = await fixture.fetch('/collections.json'); + const updated = devalue.parse(await updatedJsonResponse.text()); + assert.ok(updated.jsonLoader[0].data.temperament.includes('Bouncy')); + logs.length = 0; + + await fixture.resetAllFiles(); + // Give time for the server to restart again + await setTimeout(5000); + }); }); }); diff --git a/packages/markdown/remark/src/import-plugin-default.ts b/packages/markdown/remark/src/import-plugin-default.ts index 1b6778f753ff..2b898aa9376d 100644 --- a/packages/markdown/remark/src/import-plugin-default.ts +++ b/packages/markdown/remark/src/import-plugin-default.ts @@ -10,13 +10,13 @@ let cwdUrlStr: string | undefined; export async function importPlugin(p: string): Promise { // Try import from this package first try { - const importResult = await import(p); + const importResult = await import(/* @vite-ignore */ p); return importResult.default; } catch {} // Try import from user project cwdUrlStr ??= pathToFileURL(path.join(process.cwd(), 'package.json')).toString(); const resolved = importMetaResolve(p, cwdUrlStr); - const importResult = await import(resolved); + const importResult = await import(/* @vite-ignore */ resolved); return importResult.default; }