diff --git a/packages/cli/src/server/fileWatcher.ts b/packages/cli/src/server/fileWatcher.ts index d5e2f67e5..e02b605df 100644 --- a/packages/cli/src/server/fileWatcher.ts +++ b/packages/cli/src/server/fileWatcher.ts @@ -1,4 +1,6 @@ import { watch, type FSWatcher } from "node:fs"; +import { join } from "node:path"; +import { affectsProjectSignature } from "@hyperframes/studio-server"; export type FileChangeListener = (relativePath: string) => void; @@ -42,7 +44,17 @@ export function createProjectWatcher(projectDir: string): ProjectWatcher { watcher = watch(projectDir, { recursive: true }, (_event, filename) => { if (!filename) return; const relativePath = filename.toString(); - if (!shouldWatchProjectFile(relativePath)) return; + // The reload filter excludes all of `.hyperframes/`, but two files in + // there feed the preview signature and Studio writes one of them at + // runtime — dropping those at ingest left the CLI server's ETag stale + // until restart. Admit them here and let the reload listener re-apply + // its own filter, so what triggers a browser reload is unchanged. + if ( + !shouldWatchProjectFile(relativePath) && + !affectsProjectSignature(projectDir, join(projectDir, relativePath)) + ) { + return; + } pendingPaths.add(relativePath); if (debounceTimer) clearTimeout(debounceTimer); diff --git a/packages/cli/src/server/studioServer.ts b/packages/cli/src/server/studioServer.ts index 10b455bf8..15346c94c 100644 --- a/packages/cli/src/server/studioServer.ts +++ b/packages/cli/src/server/studioServer.ts @@ -9,7 +9,11 @@ import { Hono, type Context } from "hono"; import { streamSSE } from "hono/streaming"; import { existsSync, readFileSync, writeFileSync, statSync, unlinkSync } from "node:fs"; import { resolve, join, basename } from "node:path"; -import { createProjectWatcher, type ProjectWatcher } from "./fileWatcher.js"; +import { + createProjectWatcher, + shouldWatchProjectFile, + type ProjectWatcher, +} from "./fileWatcher.js"; import { hashSignatureParts, loadRuntimeSource, @@ -32,6 +36,7 @@ import { consumeFileWriteReceipt, fileContentVersion, getMimeType, + affectsProjectSignature, type PreviewApiAdapter, thumbnailDeviceScaleFactor, type ResolvedProject, @@ -359,8 +364,10 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { const project: ResolvedProject = { id: projectId, dir: projectDir, title: projectId }; let cachedProjectSignature: string | null = null; - watcher.addListener(() => { - cachedProjectSignature = null; + watcher.addListener((changedPath) => { + if (affectsProjectSignature(projectDir, join(projectDir, changedPath))) { + cachedProjectSignature = null; + } }); const adapter: PreviewApiAdapter = { @@ -770,7 +777,11 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { .writeSSE({ event: "file-change", data: JSON.stringify(receipt ?? { path }) }) .catch(() => {}); }; - watcher.addListener(listener); + // Re-applied here because the watcher now also emits the signature + // manifest files, which must not trigger a browser reload. + watcher.addListener((changedPath) => { + if (shouldWatchProjectFile(changedPath)) listener(changedPath); + }); while (true) { await stream.sleep(30000); } diff --git a/packages/studio-server/src/helpers/projectSignature.test.ts b/packages/studio-server/src/helpers/projectSignature.test.ts new file mode 100644 index 000000000..d6d5afcdd --- /dev/null +++ b/packages/studio-server/src/helpers/projectSignature.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from "vitest"; +import { resolve } from "node:path"; +import { affectsProjectSignature } from "./projectSignature.js"; + +const PROJECT = resolve("/projects/demo"); +const affects = (relativePath: string) => + affectsProjectSignature(PROJECT, resolve(PROJECT, relativePath)); + +describe("affectsProjectSignature", () => { + it("accepts a file the signature walk collects", () => { + expect(affects("index.html")).toBe(true); + expect(affects("assets/logo.png")).toBe(true); + }); + + it("rejects the caches the walk skips", () => { + // .thumbnails is the one that matters: the thumbnail route writes a capture + // there and reads the preview on the next one, so invalidating on it throws + // the memo away on roughly every request of the workload it exists for. + expect(affects(".thumbnails/frame-0.jpg")).toBe(false); + expect(affects("node_modules/pkg/index.js")).toBe(false); + expect(affects("renders/out.mp4")).toBe(false); + }); + + it("rejects a directory event on an excluded dir itself", () => { + expect(affects(".thumbnails")).toBe(false); + }); + + it("accepts the two manifest files the signature reads back out of .hyperframes", () => { + // The reload watcher's exclusion set is character-identical to the walk's but + // drops all of .hyperframes/. Filtering with it would stop a motion-state save + // from ever invalidating — the same stale-ETag bug in a new place. + expect(affects(".hyperframes/studio-motion.json")).toBe(true); + expect(affects(".hyperframes/studio-manual-edits.json")).toBe(true); + }); + + it("rejects everything else inside .hyperframes", () => { + expect(affects(".hyperframes/cache/blob.bin")).toBe(false); + }); + + it("rejects a path outside the project", () => { + expect(affectsProjectSignature(PROJECT, resolve("/projects/other/index.html"))).toBe(false); + expect(affectsProjectSignature(PROJECT, PROJECT)).toBe(false); + }); +}); diff --git a/packages/studio-server/src/helpers/projectSignature.ts b/packages/studio-server/src/helpers/projectSignature.ts index 763082f00..051616e11 100644 --- a/packages/studio-server/src/helpers/projectSignature.ts +++ b/packages/studio-server/src/helpers/projectSignature.ts @@ -1,6 +1,6 @@ import { createHash } from "node:crypto"; import { lstatSync, readFileSync, readdirSync } from "node:fs"; -import { extname, isAbsolute, relative, resolve } from "node:path"; +import { extname, isAbsolute, relative, resolve, sep } from "node:path"; import type { ResolvedProject, StudioApiAdapter } from "../types.js"; const SIGNATURE_TEXT_EXTENSIONS = new Set([ @@ -37,6 +37,36 @@ const STUDIO_SIGNATURE_MANIFEST_PATHS = [ ".hyperframes/studio-motion.json", ] as const; +/** + * Whether a write at `changedPath` can change `createProjectSignature(projectDir)`. + * + * Owned here because it is the exact complement of what the walk below collects, + * and a second copy of that reasoning drifts the moment either set changes. A + * watcher that invalidates on everything is not merely wasteful: `.thumbnails/` + * is written by the thumbnail route on every capture, and each capture reads the + * preview, so an unfiltered watcher discards the memo on roughly every request of + * the one workload the memo exists for. + * + * Note this is not `WATCHER_EXCLUDED_DIRS`, which is character-identical but + * excludes all of `.hyperframes/` — the signature deliberately reads two manifest + * files from inside it, so filtering with that set would stop motion-state saves + * from ever invalidating. + * + * Every segment is tested, not just the parents, so a directory event on an + * excluded dir itself (`unlinkDir .thumbnails`) is filtered too. The cost is that + * a *file* literally named `dist` at the project root reads as excluded; the walk + * would collect it, so it is a false negative, and no real project has one. + */ +export function affectsProjectSignature(projectDir: string, changedPath: string): boolean { + const relativePath = relative(resolve(projectDir), resolve(changedPath)); + if (relativePath === "" || relativePath.startsWith("..") || isAbsolute(relativePath)) { + return false; + } + const segments = relativePath.split(sep); + if (STUDIO_SIGNATURE_MANIFEST_PATHS.includes(segments.join("/") as never)) return true; + return !segments.some((segment) => SIGNATURE_EXCLUDED_DIRS.has(segment)); +} + interface ProjectSignatureFile { file: string; mtimeMs: number; diff --git a/packages/studio-server/src/index.ts b/packages/studio-server/src/index.ts index 834b3459c..7802ad975 100644 --- a/packages/studio-server/src/index.ts +++ b/packages/studio-server/src/index.ts @@ -1,5 +1,5 @@ export { createStudioApi } from "./createStudioApi.js"; -export { createProjectSignature } from "./helpers/projectSignature.js"; +export { createProjectSignature, affectsProjectSignature } from "./helpers/projectSignature.js"; export type { StudioApiAdapter, ResolvedProject, diff --git a/packages/studio/vite.adapter.signature.test.ts b/packages/studio/vite.adapter.signature.test.ts new file mode 100644 index 000000000..4a34d7065 --- /dev/null +++ b/packages/studio/vite.adapter.signature.test.ts @@ -0,0 +1,122 @@ +import { describe, expect, it } from "vitest"; +import { resolve } from "node:path"; +import { createProjectSignatureCache } from "./vite.adapter"; + +const PROJECT = resolve("/projects/demo"); + +/** A compute that changes every call, so a stale read is visible as a repeat. */ +function countingCompute() { + let calls = 0; + return { + compute: () => `sig-${++calls}`, + get calls() { + return calls; + }, + }; +} + +describe("createProjectSignatureCache", () => { + it("memoises a project's signature", () => { + const source = countingCompute(); + const cache = createProjectSignatureCache({ compute: source.compute }); + + expect(cache.get(PROJECT)).toBe("sig-1"); + expect(cache.get(PROJECT)).toBe("sig-1"); + expect(source.calls).toBe(1); + }); + + it("recomputes after a file inside the project changes", () => { + const source = countingCompute(); + const cache = createProjectSignatureCache({ compute: source.compute }); + + expect(cache.get(PROJECT)).toBe("sig-1"); + cache.invalidate(resolve(PROJECT, "index.html")); + + // The preview ETag is this string. Serving "sig-1" again answers the + // browser's revalidation with a 304 and the pre-edit composition is what + // renders — which is how a thumbnail taken after an edit showed the old frame. + expect(cache.get(PROJECT)).toBe("sig-2"); + }); + + it("recomputes for an asset added or removed, not only one edited", () => { + const source = countingCompute(); + const cache = createProjectSignatureCache({ compute: source.compute }); + + cache.get(PROJECT); + cache.invalidate(resolve(PROJECT, "assets/new-clip.mp4")); + expect(cache.get(PROJECT)).toBe("sig-2"); + + cache.invalidate(resolve(PROJECT, "compositions/scene.html")); + expect(cache.get(PROJECT)).toBe("sig-3"); + }); + + it("leaves other projects alone", () => { + const signatures = new Map([ + [PROJECT, "demo"], + [resolve("/projects/other"), "other"], + ]); + let bumped = 0; + const cache = createProjectSignatureCache({ + compute: (dir) => `${signatures.get(dir)}-${bumped}`, + }); + + expect(cache.get(PROJECT)).toBe("demo-0"); + expect(cache.get(resolve("/projects/other"))).toBe("other-0"); + + bumped = 1; + cache.invalidate(resolve(PROJECT, "index.html")); + + expect(cache.get(PROJECT)).toBe("demo-1"); + expect(cache.get(resolve("/projects/other"))).toBe("other-0"); + }); + + it("asks for a project directory to be watched once, on first use", () => { + const watched: string[] = []; + const cache = createProjectSignatureCache({ + compute: () => "sig", + watch: (dir) => watched.push(dir), + }); + + cache.get(PROJECT); + cache.get(PROJECT); + cache.invalidate(resolve(PROJECT, "index.html")); + cache.get(PROJECT); + + // Re-registering on every recompute would stack duplicate chokidar entries. + expect(watched).toEqual([PROJECT]); + }); + + it("normalises the directory it is asked about", () => { + const source = countingCompute(); + const cache = createProjectSignatureCache({ compute: source.compute }); + + expect(cache.get(PROJECT)).toBe("sig-1"); + expect(cache.get(`${PROJECT}/`)).toBe("sig-1"); + expect(cache.get(resolve(PROJECT, "nested/.."))).toBe("sig-1"); + expect(source.calls).toBe(1); + }); + + it("does not invalidate on a write the signature cannot see", () => { + const source = countingCompute(); + const cache = createProjectSignatureCache({ compute: source.compute }); + + cache.get(PROJECT); + // Each thumbnail capture writes here and then reads the preview, so an + // unfiltered watcher discards the memo on roughly every request of the one + // workload the memo exists for. + cache.invalidate(resolve(PROJECT, ".thumbnails/frame-0.jpg")); + + expect(cache.get(PROJECT)).toBe("sig-1"); + expect(source.calls).toBe(1); + }); + + it("invalidates on a motion-state save, which lives under an otherwise-skipped dir", () => { + const source = countingCompute(); + const cache = createProjectSignatureCache({ compute: source.compute }); + + cache.get(PROJECT); + cache.invalidate(resolve(PROJECT, ".hyperframes/studio-motion.json")); + + expect(cache.get(PROJECT)).toBe("sig-2"); + }); +}); diff --git a/packages/studio/vite.adapter.ts b/packages/studio/vite.adapter.ts index 148dfbcaa..d34346251 100644 --- a/packages/studio/vite.adapter.ts +++ b/packages/studio/vite.adapter.ts @@ -19,13 +19,14 @@ import { type BackgroundRemovalRender, createBackgroundRemovalJob, createProjectSignature, + affectsProjectSignature, } from "@hyperframes/studio-server"; import type { RegistryItem } from "@hyperframes/core/registry"; import { createRetryingModuleLoader, ensureProducerDist } from "./vite.producer"; import { createStudioDevRenderBodyScripts } from "./vite.studioMotion"; import { generateThumbnail, findSystemChrome } from "./vite.browser"; -export function isPathWithin(parentDir: string, childPath: string): boolean { +function isPathWithin(parentDir: string, childPath: string): boolean { const childRelativePath = relative(resolve(parentDir), resolve(childPath)); return ( childRelativePath === "" || @@ -37,7 +38,65 @@ export function resolveViteAutoProxy(value: string | undefined): boolean { return value !== "false"; } -export function createViteAdapter(dataDir: string, server: ViteDevServer): StudioApiAdapter { +/** + * The preview ETag's cache, and the one thing allowed to clear it. + * + * The signature walks the whole project directory, so it is memoised per project + * directory. (The content hash underneath is already gated behind a stat-only + * fingerprint, so what this memo saves is the walk, not the hashing — worth + * knowing before deciding how aggressive invalidation is allowed to be.) + * Getting the invalidation wrong is not a + * performance bug: the preview answers a revalidation with 304 and the browser + * keeps serving the pre-edit composition, which is how a thumbnail regenerated + * after an edit can still show the old frame. + * + * `watch` is called the first time a project dir is seen, so whoever owns the + * watcher can start following it. It must be a watcher that actually sees + * project writes: Vite's own is configured to ignore them. + */ +export interface ProjectSignatureCache { + get(projectDir: string): string; + /** Drop the signature of whichever project contains `changedPath`. */ + invalidate(changedPath: string): void; +} + +export function createProjectSignatureCache({ + compute = createProjectSignature, + watch, +}: { + compute?: (projectDir: string) => string; + watch?: (projectDir: string) => void; +} = {}): ProjectSignatureCache { + const signatures = new Map(); + const watched = new Set(); + return { + get(projectDir) { + const key = resolve(projectDir); + const cached = signatures.get(key); + if (cached !== undefined) return cached; + if (!watched.has(key)) { + watched.add(key); + watch?.(key); + } + const signature = compute(key); + signatures.set(key, signature); + return signature; + }, + invalidate(changedPath) { + // Filtered here rather than at the watcher so no caller can wire up a + // subscription that forgets to: the cache owns what can change its value. + for (const projectDir of signatures.keys()) { + if (affectsProjectSignature(projectDir, changedPath)) signatures.delete(projectDir); + } + }, + }; +} + +export function createViteAdapter( + dataDir: string, + server: ViteDevServer, + signatureCache: ProjectSignatureCache, +): StudioApiAdapter { let _bundler: | (( dir: string, @@ -63,13 +122,6 @@ export function createViteAdapter(dataDir: string, server: ViteDevServer): Studi }>) | null = null; - const projectSignatureCache = new Map(); - server.watcher.on("all", (_event, file) => { - for (const projectDir of projectSignatureCache.keys()) { - if (isPathWithin(projectDir, file)) projectSignatureCache.delete(projectDir); - } - }); - const getBundler = async () => { if (!_bundler) { try { @@ -192,17 +244,7 @@ export function createViteAdapter(dataDir: string, server: ViteDevServer): Studi }, getProjectSignature(projectDir: string): string { - const cacheKey = resolve(projectDir); - const cached = projectSignatureCache.get(cacheKey); - if (cached) return cached; - // Project dirs are symlinked from anywhere on disk (often outside the - // studio package), so Vite's default watch roots don't cover them. - // Without this, the signature cache never invalidates for external - // projects and the preview ETag serves stale 304s after edits. - server.watcher.add(cacheKey); - const signature = createProjectSignature(cacheKey); - projectSignatureCache.set(cacheKey, signature); - return signature; + return signatureCache.get(projectDir); }, async lint(html: string, opts?: { filePath?: string }) { diff --git a/packages/studio/vite.config.ts b/packages/studio/vite.config.ts index da670ed85..649e3a2ec 100644 --- a/packages/studio/vite.config.ts +++ b/packages/studio/vite.config.ts @@ -4,7 +4,7 @@ import { readFileSync, readdirSync, existsSync, lstatSync, realpathSync } from " import { join, resolve } from "node:path"; import { readNodeRequestBody } from "./vite.request-body.js"; import { watch } from "chokidar"; -import { createViteAdapter } from "./vite.adapter"; +import { createProjectSignatureCache, createViteAdapter } from "./vite.adapter"; import { previewConfigPayload } from "./vite.preview-config"; async function loadRuntimeSourceForDev( @@ -64,6 +64,44 @@ function devProjectApi(): Plugin { return { name: "studio-dev-api", configureServer(server): void { + // Watch project directories on a watcher of our own. Vite's is told to + // ignore them (see `server.watch.ignored`), because it answers an html + // change with a full page reload; this one only announces the change and + // lets Studio decide what to do with it. + const realProjectPaths: string[] = []; + try { + for (const entry of readdirSync(dataDir, { withFileTypes: true })) { + const full = join(dataDir, entry.name); + try { + realProjectPaths.push(lstatSync(full).isSymbolicLink() ? realpathSync(full) : full); + } catch { + /* skip broken symlinks */ + } + } + } catch { + /* dataDir doesn't exist yet */ + } + + const projectWatcher = watch(realProjectPaths, { + ignoreInitial: true, + // A project write is a whole-file replace; wait for it to settle so a + // half-written composition is never announced. + awaitWriteFinish: { stabilityThreshold: 40, pollInterval: 10 }, + }); + + // This watcher, and not Vite's, is what clears the preview signature. + // Vite's ignores `data/projects/**`, so subscribing the cache to it left + // the ETag frozen for the life of the dev server: the preview answered + // every revalidation with 304 and thumbnails regenerated after an edit + // still rendered the pre-edit composition. Every event type counts, since + // an added or deleted asset changes the signature as surely as an edit. + const signatureCache = createProjectSignatureCache({ + watch: (projectDir) => void projectWatcher.add(projectDir), + }); + for (const event of ["add", "change", "unlink", "addDir", "unlinkDir"] as const) { + projectWatcher.on(event, (filePath: string) => signatureCache.invalidate(filePath)); + } + let _api: { fetch: (req: Request) => Promise } | null = null; let _studioServerModule: { createStudioApi: (adapter: ReturnType) => { @@ -79,7 +117,7 @@ function devProjectApi(): Plugin { if (!_api) { const mod = await server.ssrLoadModule("@hyperframes/studio-server"); _studioServerModule = mod as typeof _studioServerModule; - const adapter = createViteAdapter(dataDir, server); + const adapter = createViteAdapter(dataDir, server, signatureCache); _api = mod.createStudioApi(adapter); } return _api; @@ -153,30 +191,6 @@ function devProjectApi(): Plugin { } }); - // Watch project directories on a watcher of our own. Vite's is told to - // ignore them (see `server.watch.ignored`), because it answers an html - // change with a full page reload; this one only announces the change and - // lets Studio decide what to do with it. - const realProjectPaths: string[] = []; - try { - for (const entry of readdirSync(dataDir, { withFileTypes: true })) { - const full = join(dataDir, entry.name); - try { - realProjectPaths.push(lstatSync(full).isSymbolicLink() ? realpathSync(full) : full); - } catch { - /* skip broken symlinks */ - } - } - } catch { - /* dataDir doesn't exist yet */ - } - - const projectWatcher = watch(realProjectPaths, { - ignoreInitial: true, - // A project write is a whole-file replace; wait for it to settle so a - // half-written composition is never announced. - awaitWriteFinish: { stabilityThreshold: 40, pollInterval: 10 }, - }); projectWatcher.on("change", (filePath: string) => { if ( !filePath.endsWith(".html") &&