From c6c0e60eecab0d069aede2ce5c3ee3cb42c3ba0d Mon Sep 17 00:00:00 2001 From: neverland Date: Mon, 14 Sep 2026 13:57:05 +0800 Subject: [PATCH] fix(fmt): verify written output before caching it as clean --- packages/rstack/src/fmt/worker.ts | 11 +++--- packages/rstack/tests/cli/fmt/cache.test.ts | 2 +- packages/rstack/tests/fmt/runnerCache.test.ts | 38 ++++++++++++++++--- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/packages/rstack/src/fmt/worker.ts b/packages/rstack/src/fmt/worker.ts index 40d7a764..74252679 100644 --- a/packages/rstack/src/fmt/worker.ts +++ b/packages/rstack/src/fmt/worker.ts @@ -94,14 +94,13 @@ const formatFile = async ({ return { status }; } - const cacheHash = - shouldWrite && !unchanged - ? hashContent(result.formatted) - : (contentHash ?? hashContent(result.source)); + // Cache only the input we actually checked. Prettier or a plugin may produce + // non-idempotent output, so writing it does not prove that it is clean. + // Keeping the input hash makes the next run verify the newly written content. const cacheEntry: FmtCacheEntry = [ - cacheHash, + contentHash ?? hashContent(result.source), cache.optionsHash, - shouldWrite || unchanged ? 'clean' : 'dirty', + unchanged ? 'clean' : 'dirty', ]; return { status, cacheEntry }; }; diff --git a/packages/rstack/tests/cli/fmt/cache.test.ts b/packages/rstack/tests/cli/fmt/cache.test.ts index d5f98a3c..3f380dd6 100644 --- a/packages/rstack/tests/cli/fmt/cache.test.ts +++ b/packages/rstack/tests/cli/fmt/cache.test.ts @@ -130,7 +130,7 @@ test('excludes the custom cache directory from formatting', () => { test('uses an explicit config root cache from a subdirectory', () => { const appPath = resolveProjectPath('packages/app'); - writeProjectFile('packages/app/index.ts', 'const value=1'); + writeProjectFile('packages/app/index.ts', 'const value = 1;\n'); const result = runFmt( ['index.ts', '--config', '../../rstack.config.ts'], diff --git a/packages/rstack/tests/fmt/runnerCache.test.ts b/packages/rstack/tests/fmt/runnerCache.test.ts index 8426d5e1..429a8773 100644 --- a/packages/rstack/tests/fmt/runnerCache.test.ts +++ b/packages/rstack/tests/fmt/runnerCache.test.ts @@ -311,7 +311,7 @@ test('does not cache formatting errors', async () => { }); }); -test('write persists clean results for misses and hits', async () => { +test('write persists checked input results for misses and hits', async () => { await withTempProject(async (rootPath) => { const cleanPath = path.join(rootPath, 'clean.ts'); const dirtyPath = path.join(rootPath, 'dirty.ts'); @@ -333,9 +333,9 @@ test('write persists clean results for misses and hits', async () => { 'clean', ]); expect(store.get('dirty.ts')).toEqual([ - createCacheHash(readFileSync(dirtyPath)), + createCacheHash('const dirty=1'), expect.any(String), - 'clean', + 'dirty', ]); const timestamps = files.map((file) => statSync(file.path).mtimeMs); @@ -350,7 +350,7 @@ test('write persists clean results for misses and hits', async () => { }); }); -test('write converts a dirty entry to clean', async () => { +test('write keeps a dirty entry until the output is checked', async () => { await withTempProject(async (rootPath) => { const filePath = path.join(rootPath, 'index.ts'); const cache = createFmtCacheContext(rootPath); @@ -367,9 +367,9 @@ test('write converts a dirty entry to clean', async () => { const store = await loadFmtCacheStore(cache.filePath, cacheNamespace); expect(store.get('index.ts')).toEqual([ - createCacheHash(readFileSync(filePath)), + createCacheHash('const value=1'), expect.any(String), - 'clean', + 'dirty', ]); await expect(run([file], 'check', cache)).resolves.toMatchObject({ exitCode: 0, @@ -377,3 +377,29 @@ test('write converts a dirty entry to clean', async () => { }); }); }); + +test('cached check matches uncached check after writing non-idempotent output', async () => { + await withTempProject(async (rootPath) => { + // Preserve these line breaks: this input needs two passes in Prettier 3.9.6. + const filePath = writeProjectFile( + rootPath, + 'example.ts', + `const fetch = rs.fn().mockImplementation(() => Promise.resolve( + new Response('cached pixels', { headers: { 'content-type': 'image/webp' } }), +)); +`, + ); + const files = [ + createFmtRequest(filePath, { + parser: 'typescript', + singleQuote: true, + trailingComma: 'all', + }), + ]; + const cache = createFmtCacheContext(rootPath); + + await run(files, 'write', cache); + const uncached = await runFmtFiles({ files, mode: 'check' }); + await expect(run(files, 'check', cache)).resolves.toEqual(uncached); + }); +});