From e6ff381e5b6ad1dc0df3164e90a458361aa99970 Mon Sep 17 00:00:00 2001 From: Daniel Bannert Date: Tue, 20 Oct 2026 20:44:16 +0200 Subject: [PATCH] fix(vis): stop config cache from dropping plugin hooks and going stale The root config cache stored a JSON copy of the merged config, so function values were lost on a cache hit: `plugins[].hooks`/`setup` and `release.plugins` hooks (e.g. `willPublish`) silently stopped running after the first invocation. The key also ignored env vars and imported helpers, so env-dependent configs stayed stale. The chain is executed before the lookup anyway, so the cache saved only the merge; remove it. Co-Authored-By: Claude Opus 5.5 --- .../vis/__tests__/config/config.test.ts | 83 +++++++--------- .../__tests__/config/extends-chain.test.ts | 6 +- .../release-ci-release-generate.test.ts | 6 +- packages/tooling/vis/src/config/config.ts | 97 +------------------ 4 files changed, 46 insertions(+), 146 deletions(-) diff --git a/packages/tooling/vis/__tests__/config/config.test.ts b/packages/tooling/vis/__tests__/config/config.test.ts index 99fcf1ad..20e65bb3 100644 --- a/packages/tooling/vis/__tests__/config/config.test.ts +++ b/packages/tooling/vis/__tests__/config/config.test.ts @@ -1,4 +1,4 @@ -import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "@visulima/path"; @@ -235,12 +235,11 @@ export default config; }); }); -describe("config cache", () => { +describe("config loading (no stale cache)", () => { let tmpDir: string; beforeEach(() => { - tmpDir = mkdtempSync(join(tmpdir(), "vis-cache-test-")); - // Create node_modules so find-cache-dir can resolve + tmpDir = mkdtempSync(join(tmpdir(), "vis-config-test-")); mkdirSync(join(tmpDir, "node_modules"), { recursive: true }); }); @@ -248,71 +247,63 @@ describe("config cache", () => { rmSync(tmpDir, { force: true, recursive: true }); }); - it("should create a cache file after first load", async () => { - expect.assertions(2); - - writeFileSync(join(tmpDir, "vis.config.mjs"), "export default { update: { target: \"patch\" } };"); + it("should keep plugin functions on repeated loads", async () => { + expect.assertions(4); - const config = await loadVisConfig(tmpDir); + writeFileSync(join(tmpDir, "vis.config.ts"), "export default { plugins: [{ name: \"marker\", hooks: { \"task:before\": () => {} }, setup: () => {} }] };"); - expect(config.update?.target).toBe("patch"); + const first = await loadVisConfig(tmpDir); - const cachePath = join(tmpDir, "node_modules", ".cache", "vis", "vis-config-cache.json"); - const cacheExists = (() => { - try { - readFileSync(cachePath); + expect(first.plugins?.[0]?.setup).toBeTypeOf("function"); + expect(first.plugins?.[0]?.hooks?.["task:before"]).toBeTypeOf("function"); - return true; - } catch { - return false; - } - })(); + const second = await loadVisConfig(tmpDir); - expect(cacheExists).toBe(true); + expect(second.plugins?.[0]?.setup).toBeTypeOf("function"); + expect(second.plugins?.[0]?.hooks?.["task:before"]).toBeTypeOf("function"); }); - it("should return cached config on second load without recompiling", async () => { + it("should reflect env var changes on repeated loads", async () => { expect.assertions(2); - writeFileSync(join(tmpDir, "vis.config.mjs"), "export default { update: { target: \"patch\" } };"); + writeFileSync(join(tmpDir, "vis.config.ts"), "export default { tasks: { build: { cache: process.env.VIS_TEST_FLAG === \"on\" } } };"); - const first = await loadVisConfig(tmpDir); - const second = await loadVisConfig(tmpDir); + const originalValue = process.env.VIS_TEST_FLAG; - expect(first.update?.target).toBe("patch"); - expect(second.update?.target).toBe("patch"); + try { + process.env.VIS_TEST_FLAG = "off"; + + const off = await loadVisConfig(tmpDir); + + expect(off.tasks?.build?.cache).toBe(false); + + process.env.VIS_TEST_FLAG = "on"; + + const on = await loadVisConfig(tmpDir); + + expect(on.tasks?.build?.cache).toBe(true); + } finally { + if (originalValue === undefined) { + delete process.env.VIS_TEST_FLAG; + } else { + process.env.VIS_TEST_FLAG = originalValue; + } + } }); - it("should invalidate cache when config file changes", async () => { + it("should pick up edits to the config file on reload", async () => { expect.assertions(2); - writeFileSync(join(tmpDir, "vis.config.mjs"), "export default { update: { target: \"patch\" } };"); + writeFileSync(join(tmpDir, "vis.config.ts"), "export default { update: { target: \"patch\" } };"); const first = await loadVisConfig(tmpDir); expect(first.update?.target).toBe("patch"); - // Modify the config file - writeFileSync(join(tmpDir, "vis.config.mjs"), "export default { update: { target: \"latest\" } };"); + writeFileSync(join(tmpDir, "vis.config.ts"), "export default { update: { target: \"latest\" } };"); const second = await loadVisConfig(tmpDir); expect(second.update?.target).toBe("latest"); }); - - it("should handle corrupt cache gracefully", async () => { - expect.assertions(1); - - writeFileSync(join(tmpDir, "vis.config.mjs"), "export default { update: { target: \"minor\" } };"); - - // Write corrupt cache - const cacheDir = join(tmpDir, "node_modules", ".cache", "vis"); - - mkdirSync(cacheDir, { recursive: true }); - writeFileSync(join(cacheDir, "vis-config-cache.json"), "not valid json{{{"); - - const config = await loadVisConfig(tmpDir); - - expect(config.update?.target).toBe("minor"); - }); }); diff --git a/packages/tooling/vis/__tests__/config/extends-chain.test.ts b/packages/tooling/vis/__tests__/config/extends-chain.test.ts index 36628d99..ebc3b23f 100644 --- a/packages/tooling/vis/__tests__/config/extends-chain.test.ts +++ b/packages/tooling/vis/__tests__/config/extends-chain.test.ts @@ -20,9 +20,9 @@ describe("vis.config.ts extends chain", () => { beforeEach(() => { scratch = mkdtempSync(join(realpathSync(tmpdir()), "vis-extends-")); - // node_modules has to exist so getConfigCachePath finds a writable - // location — without it the loader falls back to findCacheDirSync - // which traverses upward and may resolve outside the fixture. + // node_modules has to exist so vis's caches (vis.task overlays) land + // in the fixture — without it findCacheDirSync traverses upward and + // may resolve outside the fixture. mkdirSync(join(scratch, "node_modules"), { recursive: true }); writeFileSync(join(scratch, "package.json"), JSON.stringify({ name: "fixture-root", private: true })); }); diff --git a/packages/tooling/vis/__tests__/release/commands/release-ci-release-generate.test.ts b/packages/tooling/vis/__tests__/release/commands/release-ci-release-generate.test.ts index 66848178..3f07399e 100644 --- a/packages/tooling/vis/__tests__/release/commands/release-ci-release-generate.test.ts +++ b/packages/tooling/vis/__tests__/release/commands/release-ci-release-generate.test.ts @@ -60,9 +60,9 @@ const setupFixture = (): { cwd: string; rootSha: string } => { `${JSON.stringify({ name: "fixture-root", packageManager: "pnpm@10.32.1", private: true, version: "0.0.0", workspaces: ["packages/*"] }, null, 4)}\n`, ); writeFileSync(join(cwd, "pnpm-workspace.yaml"), "packages:\n - 'packages/*'\n"); - // `loadVisConfig` writes `node_modules/.cache/vis/vis-config-cache.json`; - // in a real repo that path is gitignored, so the fixture must be too or - // the clean-tree guard would fire on the tool's own cache. + // vis writes its caches under `node_modules/.cache/vis`; in a real repo + // that path is gitignored, so the fixture must be too or the clean-tree + // guard would fire on the tool's own cache. writeFileSync(join(cwd, ".gitignore"), "node_modules\n"); mkdirSync(join(cwd, "packages", "a", "src"), { recursive: true }); writeFileSync(join(cwd, "packages", "a", "package.json"), `${JSON.stringify({ name: "@scope/a", version: "1.0.0" }, null, 4)}\n`); diff --git a/packages/tooling/vis/src/config/config.ts b/packages/tooling/vis/src/config/config.ts index 16d8e1aa..cdf74d56 100644 --- a/packages/tooling/vis/src/config/config.ts +++ b/packages/tooling/vis/src/config/config.ts @@ -152,79 +152,8 @@ const findVisTaskConfigFile = (projectDirectory: string): string | undefined => return undefined; }; -interface ConfigCache { - config: VisConfig; - hash: string; -} - const hashFileContents = (filePath: string): string => createHash("sha256").update(fsReadFileSync(filePath)).digest("hex"); -/** - * Composite hash for a chain of config files. Sorting by absolute path - * makes the result independent of resolution order — diamond imports - * (A and B both extend C) collapse to one entry. - */ -const hashConfigChain = (paths: ReadonlyArray): string => { - const hasher = createHash("sha256"); - const sorted = [...paths].sort(); - - for (const path of sorted) { - hasher.update(path); - hasher.update(":"); - hasher.update(hashFileContents(path)); - hasher.update("\n"); - } - - return hasher.digest("hex"); -}; - -const getConfigCachePath = (workspaceRoot: string): string | undefined => { - // First try: use node_modules/.cache/vis directly if node_modules exists - // in the workspace root. This avoids findCacheDirSync traversing to a - // parent project's node_modules when the workspace root lacks package.json. - const nodeModulesDir = join(workspaceRoot, "node_modules"); - - if (isAccessibleSync(nodeModulesDir)) { - const directCacheDir = join(nodeModulesDir, ".cache", "vis"); - - ensureDirSync(directCacheDir); - - return join(directCacheDir, "vis-config-cache.json"); - } - - // Fallback: standard cache dir resolution - const cacheDir = findCacheDirSync("vis", { create: true, cwd: workspaceRoot }); - - return cacheDir ? join(cacheDir, "vis-config-cache.json") : undefined; -}; - -const readConfigCache = (cachePath: string, hash: string): VisConfig | undefined => { - if (!isAccessibleSync(cachePath)) { - return undefined; - } - - try { - const cache = readJsonSync(cachePath) as unknown as ConfigCache; - - if (cache.hash === hash) { - return cache.config; - } - } catch { - // Corrupt cache — ignore - } - - return undefined; -}; - -const writeConfigCache = (cachePath: string, hash: string, config: VisConfig): void => { - try { - ensureDirSync(dirname(cachePath)); - writeJsonSync(cachePath, { config, hash } satisfies ConfigCache); - } catch { - // Non-critical - } -}; - const normalizeExtends = (value: VisConfig["extends"]): string[] => { if (value === undefined) { return []; @@ -428,9 +357,7 @@ const resolveConfigChain = async ( * Load the vis configuration from a `vis.config.ts` (or `.js`, `.mjs`, `.cjs`, `.mts`, `.cts`) file. * * Resolves the entire `extends` chain, post-order, and folds it into a - * single merged config (extends first, root last — child wins). The - * cache key covers every file in the chain, so editing any extended - * file invalidates the cache. + * single merged config (extends first, root last — child wins). * * Falls back to secure defaults if no config file is found. * @param workspaceRoot The workspace root directory to search for the config file. @@ -466,30 +393,13 @@ const loadVisConfig = async (workspaceRoot: string, options?: { explicitConfigPa await resolveConfigChain(rootConfigPath, [], inFlight, loaded, order); - const chainHash = hashConfigChain(order); - const cachePath = getConfigCachePath(workspaceRoot); - - if (cachePath) { - const cached = readConfigCache(cachePath, chainHash); - - if (cached) { - return cached; - } - } - let merged: VisConfig = {}; for (const path of order) { merged = mergeVisConfigs(merged, loaded.get(path)!); } - const finalConfig = applyDefaults(merged); - - if (cachePath) { - writeConfigCache(cachePath, chainHash, finalConfig); - } - - return finalConfig; + return applyDefaults(merged); }; interface VisTaskConfigCache { @@ -549,8 +459,7 @@ const writeVisTaskCache = (cachePath: string, hash: string, config: VisTaskConfi * Returns `undefined` when no overlay file exists. Otherwise compiles * the file via the oxc TS loader and caches the result under * `node_modules/.cache/vis/task-configs/<project>.json`, keyed by the - * file's content hash. Editing one project's overlay does not invalidate - * the root config cache. + * file's content hash. * * Errors thrown by the file are wrapped in `VisConfigLoadError` so the * source path is reported instead of an opaque workspace.ts failure.