feat(fmt): cache write results (#227)

Author: chenjiahan

packages/rstack/src/fmt/runner.ts

@@ -183,7 +183,7 @@ const runFmtFiles = async ({
 }: RunFmtFilesOptions): Promise<FmtRunResult> => {
   const shouldWrite = mode === 'write';
   let runCache: RunCache | undefined;
-  if (files.length > 0 && cache && !shouldWrite) {
+  if (files.length > 0 && cache) {
     runCache = {
       store: await loadFmtCacheStore(cache.filePath, cacheNamespace),
       resolveKey: createCacheKeyResolver(cache.rootPath),

packages/rstack/src/fmt/types.ts

@@ -99,7 +99,7 @@ interface RunFmtFilesOptions {
   mode: FmtMode;
   /** Maximum number of formatting workers. */
   maxWorkers?: number;
-  /** Internal persistent cache context. Currently used only by check and list modes. */
+  /** Internal persistent cache context. */
   cache?: FmtCacheContext;
 }
 

packages/rstack/src/fmt/worker.ts

@@ -11,7 +11,7 @@ interface FormatFileTask {
   cache?: FmtFileCache;
 }
 
-const hashContent = (content: Uint8Array): string =>
+const hashContent = (content: string | Uint8Array): string =>
   createHash('sha256').update(content).digest('hex');
 
 /**
@@ -25,10 +25,9 @@ const formatFile = async ({
 }: FormatFileTask): Promise<FmtWorkerResult> => {
   let source: string | undefined;
   let contentHash: string | undefined;
-  const fileCache = shouldWrite ? undefined : cache;
 
-  const readSource = (): string => {
-    if (!fileCache) {
+  const readSource = (shouldHash = !shouldWrite): string => {
+    if (!cache || !shouldHash) {
       return readFileSync(file.path, 'utf8');
     }
 
@@ -37,10 +36,10 @@ const formatFile = async ({
     return content.toString('utf8');
   };
 
-  if (fileCache?.entry && fileCache.entry[1] === fileCache.optionsHash) {
-    source = readSource();
-    const { entry } = fileCache;
-    if (entry[0] === contentHash) {
+  if (cache?.entry && cache.entry[1] === cache.optionsHash) {
+    source = readSource(true);
+    const { entry } = cache;
+    if (entry[0] === contentHash && (!shouldWrite || entry[2] === 'clean')) {
       return { status: entry[2] === 'clean' ? 'unchanged' : 'changed' };
     }
   }
@@ -58,14 +57,18 @@ const formatFile = async ({
   }
 
   const status = unchanged ? 'unchanged' : 'changed';
-  if (!fileCache || contentHash === undefined) {
+  if (!cache) {
     return { status };
   }
 
+  const cacheHash =
+    shouldWrite && !unchanged
+      ? hashContent(result.formatted)
+      : (contentHash ?? hashContent(result.source));
   const cacheEntry: FmtCacheEntry = [
-    contentHash,
-    fileCache.optionsHash,
-    unchanged ? 'clean' : 'dirty',
+    cacheHash,
+    cache.optionsHash,
+    shouldWrite || unchanged ? 'clean' : 'dirty',
   ];
   return { status, cacheEntry };
 };

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

@@ -1,4 +1,4 @@
-import { existsSync, readFileSync, statSync, utimesSync, writeFileSync } from 'node:fs';
+import { readFileSync, statSync, utimesSync, writeFileSync } from 'node:fs';
 import path from 'node:path';
 import { expect, test } from 'rstack/test';
 import { cacheNamespace, createOptionsHasher, sha256 } from '../../src/fmt/cacheIdentity.ts';
@@ -160,17 +160,67 @@ test('does not cache formatting errors', async () => {
   });
 });
 
-test('does not apply the cache in write mode yet', async () => {
+test('write persists clean results for misses and hits', async () => {
+  await withTempProject(async (rootPath) => {
+    const cleanPath = path.join(rootPath, 'clean.ts');
+    const dirtyPath = path.join(rootPath, 'dirty.ts');
+    const cache = createCache(rootPath);
+    writeFileSync(cleanPath, 'const clean = 1;\n');
+    writeFileSync(dirtyPath, 'const dirty=1');
+
+    const files = [createRequest(cleanPath), createRequest(dirtyPath)];
+    await expect(run(files, 'write', cache)).resolves.toMatchObject({
+      exitCode: 0,
+      files: [{ path: dirtyPath, status: 'written' }],
+      processedFileCount: 2,
+    });
+
+    const store = await loadFmtCacheStore(cache.filePath, cacheNamespace);
+    expect(store.get('clean.ts')).toEqual([
+      sha256(readFileSync(cleanPath)),
+      expect.any(String),
+      'clean',
+    ]);
+    expect(store.get('dirty.ts')).toEqual([
+      sha256(readFileSync(dirtyPath)),
+      expect.any(String),
+      'clean',
+    ]);
+
+    const timestamps = files.map((file) => statSync(file.path).mtimeMs);
+    await expect(run(files, 'write', cache)).resolves.toMatchObject({
+      exitCode: 0,
+      files: [],
+      processedFileCount: 2,
+    });
+    expect(files.map((file) => statSync(file.path).mtimeMs)).toEqual(timestamps);
+  });
+});
+
+test('write converts a dirty entry to clean', async () => {
   await withTempProject(async (rootPath) => {
     const filePath = path.join(rootPath, 'index.ts');
     const cache = createCache(rootPath);
+    const file = createRequest(filePath);
     writeFileSync(filePath, 'const value=1');
 
-    await expect(run([createRequest(filePath)], 'write', cache)).resolves.toMatchObject({
+    await run([file], 'check', cache);
+
+    await expect(run([file], 'write', cache)).resolves.toMatchObject({
       exitCode: 0,
       files: [{ path: filePath, status: 'written' }],
     });
     expect(readFileSync(filePath, 'utf8')).toBe('const value = 1;\n');
-    expect(existsSync(cache.filePath)).toBe(false);
+
+    const store = await loadFmtCacheStore(cache.filePath, cacheNamespace);
+    expect(store.get('index.ts')).toEqual([
+      sha256(readFileSync(filePath)),
+      expect.any(String),
+      'clean',
+    ]);
+    await expect(run([file], 'check', cache)).resolves.toMatchObject({
+      exitCode: 0,
+      files: [],
+    });
   });
 });

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

@@ -51,9 +51,10 @@ test('returns cached states before resolving the parser', async () => {
     const contentHash = sha256(source);
     const optionsHash = 'options';
 
-    for (const [state, status] of [
-      ['clean', 'unchanged'],
-      ['dirty', 'changed'],
+    for (const [state, shouldWrite, status] of [
+      ['clean', false, 'unchanged'],
+      ['dirty', false, 'changed'],
+      ['clean', true, 'unchanged'],
     ] as const) {
       await expect(
         formatFile({
@@ -63,7 +64,7 @@ test('returns cached states before resolving the parser', async () => {
               parser: 'unknown-parser',
             },
           },
-          shouldWrite: false,
+          shouldWrite,
           cache: {
             entry: [contentHash, optionsHash, state],
             optionsHash,