From b4d5251b8fa9d22fc83bcd7e187f61489ff551eb Mon Sep 17 00:00:00 2001
From: Rob Hogan <robhogan@meta.com>
Date: Thu, 30 Jan 2025 17:25:27 -0800
Subject: [PATCH] Use async IO in DiskCacheManager

Summary:
Switch from `graceful-fs` synchronous IO in `metro-file-map`'s `DiskCacheManager` to native promise IO.

[`graceful-fs`](https://github.com/isaacs/node-graceful-fs/blob/main/graceful-fs.js) [does not patch](https://github.com/isaacs/node-graceful-fs/tree/main?tab=readme-ov-file#sync-methods) `readFileSync`/`writeFileSync`, so we're not deriving any benefit from `graceful-fs` over `fs` in this instance.

Using async IO is just good practice especially during startup when Metro is doing various things at once.

This sets up lazy writes triggered by events, in the next diff.

Modification to `server-torn-down-test` required because `fs.promises` open differently-labelled handles, but these are unreffed and don't block shutdown.

Changelog: Internal

Reviewed By: vzaidman

Differential Revision: D68722144

fbshipit-source-id: 6e070b76a650f43238719e95bdb2d13600deb3d8
---
 .../src/cache/DiskCacheManager.js             |  6 +-
 .../cache/__tests__/DiskCacheManager-test.js  | 56 ++++++++++++++++++-
 .../__tests__/server-torn-down-test.js        |  2 +
 3 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/packages/metro-file-map/src/cache/DiskCacheManager.js b/packages/metro-file-map/src/cache/DiskCacheManager.js
index 4e7fc1a0e6..16feeb59d8 100644
--- a/packages/metro-file-map/src/cache/DiskCacheManager.js
+++ b/packages/metro-file-map/src/cache/DiskCacheManager.js
@@ -18,7 +18,7 @@ import type {
 } from '../flow-types';
 
 import rootRelativeCacheKeys from '../lib/rootRelativeCacheKeys';
-import {readFileSync, writeFileSync} from 'graceful-fs';
+import {promises as fsPromises} from 'fs';
 import {tmpdir} from 'os';
 import path from 'path';
 import {deserialize, serialize} from 'v8';
@@ -67,7 +67,7 @@ export class DiskCacheManager implements CacheManager {
 
   async read(): Promise<?CacheData> {
     try {
-      return deserialize(readFileSync(this._cachePath));
+      return deserialize(await fsPromises.readFile(this._cachePath));
     } catch (e) {
       if (e?.code === 'ENOENT') {
         // Cache file not found - not considered an error.
@@ -83,7 +83,7 @@ export class DiskCacheManager implements CacheManager {
     {changed, removed}: CacheDelta,
   ): Promise<void> {
     if (changed.size > 0 || removed.size > 0) {
-      writeFileSync(this._cachePath, serialize(dataSnapshot));
+      await fsPromises.writeFile(this._cachePath, serialize(dataSnapshot));
     }
   }
 }
diff --git a/packages/metro-file-map/src/cache/__tests__/DiskCacheManager-test.js b/packages/metro-file-map/src/cache/__tests__/DiskCacheManager-test.js
index 2fc001a141..4620b8b70c 100644
--- a/packages/metro-file-map/src/cache/__tests__/DiskCacheManager-test.js
+++ b/packages/metro-file-map/src/cache/__tests__/DiskCacheManager-test.js
@@ -9,10 +9,21 @@
  * @oncall react_native
  */
 
-import type {BuildParameters} from '../../flow-types';
+import type {BuildParameters, CacheData} from '../../flow-types';
 
 import {DiskCacheManager} from '../DiskCacheManager';
 import * as path from 'path';
+import {serialize} from 'v8';
+
+const mockReadFile = jest.fn();
+const mockWriteFile = jest.fn();
+
+jest.mock('fs', () => ({
+  promises: {
+    readFile: (...args) => mockReadFile(...args),
+    writeFile: (...args) => mockWriteFile(...args),
+  },
+}));
 
 const buildParameters: BuildParameters = {
   cacheBreaker: '',
@@ -42,6 +53,10 @@ const defaultConfig = {
 };
 
 describe('cacheManager', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
   test('creates valid cache file paths', () => {
     expect(
       DiskCacheManager.getCacheFilePath(buildParameters, 'file-prefix', '/'),
@@ -158,4 +173,43 @@ describe('cacheManager', () => {
       cacheManager2.getCacheFilePath(),
     );
   });
+
+  test('reads a cache file and deserialises its contents', async () => {
+    const cacheManager = new DiskCacheManager({buildParameters}, defaultConfig);
+    mockReadFile.mockResolvedValueOnce(serialize({foo: 'bar'}));
+    const cache = await cacheManager.read();
+    expect(mockReadFile).toHaveBeenCalledWith(cacheManager.getCacheFilePath());
+    expect(cache).toEqual({foo: 'bar'});
+  });
+
+  test('serialises and writes a cache file', async () => {
+    const cacheManager = new DiskCacheManager({buildParameters}, defaultConfig);
+    const data: CacheData = {
+      clocks: new Map([['foo', 'bar']]),
+      fileSystemData: new Map(),
+      plugins: new Map(),
+    };
+    await cacheManager.write(data, {
+      changed: new Map(),
+      removed: new Set(['foo']),
+    });
+    expect(mockWriteFile).toHaveBeenCalledWith(
+      cacheManager.getCacheFilePath(),
+      serialize(data),
+    );
+  });
+
+  test('does not write when there have been no changes', async () => {
+    const cacheManager = new DiskCacheManager({buildParameters}, defaultConfig);
+    await cacheManager.write(
+      {
+        clocks: new Map([['foo', 'bar']]),
+        fileSystemData: new Map(),
+        plugins: new Map(),
+      },
+      // Empty delta
+      {changed: new Map(), removed: new Set()},
+    );
+    expect(mockWriteFile).not.toHaveBeenCalled();
+  });
 });
diff --git a/packages/metro/src/integration_tests/__tests__/server-torn-down-test.js b/packages/metro/src/integration_tests/__tests__/server-torn-down-test.js
index 2296b1c6da..a823a94841 100644
--- a/packages/metro/src/integration_tests/__tests__/server-torn-down-test.js
+++ b/packages/metro/src/integration_tests/__tests__/server-torn-down-test.js
@@ -29,6 +29,8 @@ describe('Server torn down test', () => {
           'RANDOMBYTESREQUEST',
           'Timeout',
           'TickObject',
+          'FILEHANDLE',
+          'FILEHANDLECLOSEREQ',
           'FSREQCALLBACK',
           'FSREQPROMISE',
           'FSEVENTWRAP',