Skip to content

Commit

Permalink
fix: better handling of resync and restarts in content layer (#12984)
Browse files Browse the repository at this point in the history
* fix: better handling of resync and restarts

* Always regenerate content layer on sync

* Formatting
  • Loading branch information
ascorbic authored Jan 16, 2025
1 parent 0968069 commit 2d259cf
Show file tree
Hide file tree
Showing 13 changed files with 152 additions and 21 deletions.
5 changes: 5 additions & 0 deletions .changeset/kind-horses-smile.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes a bug in dev where files would stop being watched if the Astro config file was edited
5 changes: 5 additions & 0 deletions .changeset/large-cherries-explain.md
Original file line number Diff line number Diff line change
@@ -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
18 changes: 15 additions & 3 deletions packages/astro/src/content/content-layer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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 });
}

Expand Down Expand Up @@ -79,6 +84,7 @@ export class ContentLayer {
dispose() {
this.#queue.clear();
this.#unsubscribe?.();
this.#watcher?.removeAllTrackedListeners();
}

async #getGenerateDigest() {
Expand Down Expand Up @@ -178,7 +184,7 @@ export class ContentLayer {
} = this.#settings.config;

const astroConfigDigest = safeStringify(hashableConfig);

const { digest: currentConfigDigest } = contentConfig.config;
this.#lastConfigDigest = currentConfigDigest;

Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions packages/astro/src/content/loaders/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand Down
2 changes: 2 additions & 0 deletions packages/astro/src/content/loaders/glob.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
15 changes: 4 additions & 11 deletions packages/astro/src/content/server-listeners.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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 });
});
Expand Down
9 changes: 8 additions & 1 deletion packages/astro/src/content/types-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
62 changes: 62 additions & 0 deletions packages/astro/src/content/watcher.ts
Original file line number Diff line number Diff line change
@@ -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<WatchEventName, Set<WatchEventCallback>>();

const handler: ProxyHandler<FSWatcher> = {
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;
}
5 changes: 3 additions & 2 deletions packages/astro/src/core/dev/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?? {},
Expand Down
3 changes: 3 additions & 0 deletions packages/astro/src/core/dev/restart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
10 changes: 9 additions & 1 deletion packages/astro/src/core/sync/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -50,6 +50,7 @@ export type SyncOptions = {
};
manifest: ManifestData;
command: 'build' | 'dev' | 'sync';
watcher?: FSWatcher;
};

export default async function sync(
Expand Down Expand Up @@ -120,6 +121,7 @@ export async function syncInternal({
force,
manifest,
command,
watcher,
}: SyncOptions): Promise<void> {
const isDev = command === 'dev';
if (force) {
Expand All @@ -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);
Expand All @@ -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)
Expand Down
33 changes: 32 additions & 1 deletion packages/astro/test/content-layer.test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
});
});
});
4 changes: 2 additions & 2 deletions packages/markdown/remark/src/import-plugin-default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ let cwdUrlStr: string | undefined;
export async function importPlugin(p: string): Promise<unified.Plugin> {
// 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;
}

0 comments on commit 2d259cf

Please sign in to comment.