Skip to content

Commit b9cd500

Browse files
authored
fix(fmt): verify written output before caching it as clean (#481)
1 parent c4af63d commit b9cd500

3 files changed

Lines changed: 38 additions & 13 deletions

File tree

packages/rstack/src/fmt/worker.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,14 +94,13 @@ const formatFile = async ({
9494
return { status };
9595
}
9696

97-
const cacheHash =
98-
shouldWrite && !unchanged
99-
? hashContent(result.formatted)
100-
: (contentHash ?? hashContent(result.source));
97+
// Cache only the input we actually checked. Prettier or a plugin may produce
98+
// non-idempotent output, so writing it does not prove that it is clean.
99+
// Keeping the input hash makes the next run verify the newly written content.
101100
const cacheEntry: FmtCacheEntry = [
102-
cacheHash,
101+
contentHash ?? hashContent(result.source),
103102
cache.optionsHash,
104-
shouldWrite || unchanged ? 'clean' : 'dirty',
103+
unchanged ? 'clean' : 'dirty',
105104
];
106105
return { status, cacheEntry };
107106
};

packages/rstack/tests/cli/fmt/cache.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ test('excludes the custom cache directory from formatting', () => {
130130

131131
test('uses an explicit config root cache from a subdirectory', () => {
132132
const appPath = resolveProjectPath('packages/app');
133-
writeProjectFile('packages/app/index.ts', 'const value=1');
133+
writeProjectFile('packages/app/index.ts', 'const value = 1;\n');
134134

135135
const result = runFmt(
136136
['index.ts', '--config', '../../rstack.config.ts'],

packages/rstack/tests/fmt/runnerCache.test.ts

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ test('does not cache formatting errors', async () => {
311311
});
312312
});
313313

314-
test('write persists clean results for misses and hits', async () => {
314+
test('write persists checked input results for misses and hits', async () => {
315315
await withTempProject(async (rootPath) => {
316316
const cleanPath = path.join(rootPath, 'clean.ts');
317317
const dirtyPath = path.join(rootPath, 'dirty.ts');
@@ -333,9 +333,9 @@ test('write persists clean results for misses and hits', async () => {
333333
'clean',
334334
]);
335335
expect(store.get('dirty.ts')).toEqual([
336-
createCacheHash(readFileSync(dirtyPath)),
336+
createCacheHash('const dirty=1'),
337337
expect.any(String),
338-
'clean',
338+
'dirty',
339339
]);
340340

341341
const timestamps = files.map((file) => statSync(file.path).mtimeMs);
@@ -350,7 +350,7 @@ test('write persists clean results for misses and hits', async () => {
350350
});
351351
});
352352

353-
test('write converts a dirty entry to clean', async () => {
353+
test('write keeps a dirty entry until the output is checked', async () => {
354354
await withTempProject(async (rootPath) => {
355355
const filePath = path.join(rootPath, 'index.ts');
356356
const cache = createFmtCacheContext(rootPath);
@@ -367,13 +367,39 @@ test('write converts a dirty entry to clean', async () => {
367367

368368
const store = await loadFmtCacheStore(cache.filePath, cacheNamespace);
369369
expect(store.get('index.ts')).toEqual([
370-
createCacheHash(readFileSync(filePath)),
370+
createCacheHash('const value=1'),
371371
expect.any(String),
372-
'clean',
372+
'dirty',
373373
]);
374374
await expect(run([file], 'check', cache)).resolves.toMatchObject({
375375
exitCode: 0,
376376
files: [],
377377
});
378378
});
379379
});
380+
381+
test('cached check matches uncached check after writing non-idempotent output', async () => {
382+
await withTempProject(async (rootPath) => {
383+
// Preserve these line breaks: this input needs two passes in Prettier 3.9.6.
384+
const filePath = writeProjectFile(
385+
rootPath,
386+
'example.ts',
387+
`const fetch = rs.fn<typeof globalThis.fetch>().mockImplementation(() => Promise.resolve(
388+
new Response('cached pixels', { headers: { 'content-type': 'image/webp' } }),
389+
));
390+
`,
391+
);
392+
const files = [
393+
createFmtRequest(filePath, {
394+
parser: 'typescript',
395+
singleQuote: true,
396+
trailingComma: 'all',
397+
}),
398+
];
399+
const cache = createFmtCacheContext(rootPath);
400+
401+
await run(files, 'write', cache);
402+
const uncached = await runFmtFiles({ files, mode: 'check' });
403+
await expect(run(files, 'check', cache)).resolves.toEqual(uncached);
404+
});
405+
});

0 commit comments

Comments
 (0)