Skip to content

Commit b4d5251

Browse files
robhoganfacebook-github-bot
authored andcommitted
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
1 parent 71b8a54 commit b4d5251

File tree

3 files changed

+60
-4
lines changed

3 files changed

+60
-4
lines changed

packages/metro-file-map/src/cache/DiskCacheManager.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import type {
1818
} from '../flow-types';
1919

2020
import rootRelativeCacheKeys from '../lib/rootRelativeCacheKeys';
21-
import {readFileSync, writeFileSync} from 'graceful-fs';
21+
import {promises as fsPromises} from 'fs';
2222
import {tmpdir} from 'os';
2323
import path from 'path';
2424
import {deserialize, serialize} from 'v8';
@@ -67,7 +67,7 @@ export class DiskCacheManager implements CacheManager {
6767

6868
async read(): Promise<?CacheData> {
6969
try {
70-
return deserialize(readFileSync(this._cachePath));
70+
return deserialize(await fsPromises.readFile(this._cachePath));
7171
} catch (e) {
7272
if (e?.code === 'ENOENT') {
7373
// Cache file not found - not considered an error.
@@ -83,7 +83,7 @@ export class DiskCacheManager implements CacheManager {
8383
{changed, removed}: CacheDelta,
8484
): Promise<void> {
8585
if (changed.size > 0 || removed.size > 0) {
86-
writeFileSync(this._cachePath, serialize(dataSnapshot));
86+
await fsPromises.writeFile(this._cachePath, serialize(dataSnapshot));
8787
}
8888
}
8989
}

packages/metro-file-map/src/cache/__tests__/DiskCacheManager-test.js

+55-1
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,21 @@
99
* @oncall react_native
1010
*/
1111

12-
import type {BuildParameters} from '../../flow-types';
12+
import type {BuildParameters, CacheData} from '../../flow-types';
1313

1414
import {DiskCacheManager} from '../DiskCacheManager';
1515
import * as path from 'path';
16+
import {serialize} from 'v8';
17+
18+
const mockReadFile = jest.fn();
19+
const mockWriteFile = jest.fn();
20+
21+
jest.mock('fs', () => ({
22+
promises: {
23+
readFile: (...args) => mockReadFile(...args),
24+
writeFile: (...args) => mockWriteFile(...args),
25+
},
26+
}));
1627

1728
const buildParameters: BuildParameters = {
1829
cacheBreaker: '',
@@ -42,6 +53,10 @@ const defaultConfig = {
4253
};
4354

4455
describe('cacheManager', () => {
56+
beforeEach(() => {
57+
jest.clearAllMocks();
58+
});
59+
4560
test('creates valid cache file paths', () => {
4661
expect(
4762
DiskCacheManager.getCacheFilePath(buildParameters, 'file-prefix', '/'),
@@ -158,4 +173,43 @@ describe('cacheManager', () => {
158173
cacheManager2.getCacheFilePath(),
159174
);
160175
});
176+
177+
test('reads a cache file and deserialises its contents', async () => {
178+
const cacheManager = new DiskCacheManager({buildParameters}, defaultConfig);
179+
mockReadFile.mockResolvedValueOnce(serialize({foo: 'bar'}));
180+
const cache = await cacheManager.read();
181+
expect(mockReadFile).toHaveBeenCalledWith(cacheManager.getCacheFilePath());
182+
expect(cache).toEqual({foo: 'bar'});
183+
});
184+
185+
test('serialises and writes a cache file', async () => {
186+
const cacheManager = new DiskCacheManager({buildParameters}, defaultConfig);
187+
const data: CacheData = {
188+
clocks: new Map([['foo', 'bar']]),
189+
fileSystemData: new Map(),
190+
plugins: new Map(),
191+
};
192+
await cacheManager.write(data, {
193+
changed: new Map(),
194+
removed: new Set(['foo']),
195+
});
196+
expect(mockWriteFile).toHaveBeenCalledWith(
197+
cacheManager.getCacheFilePath(),
198+
serialize(data),
199+
);
200+
});
201+
202+
test('does not write when there have been no changes', async () => {
203+
const cacheManager = new DiskCacheManager({buildParameters}, defaultConfig);
204+
await cacheManager.write(
205+
{
206+
clocks: new Map([['foo', 'bar']]),
207+
fileSystemData: new Map(),
208+
plugins: new Map(),
209+
},
210+
// Empty delta
211+
{changed: new Map(), removed: new Set()},
212+
);
213+
expect(mockWriteFile).not.toHaveBeenCalled();
214+
});
161215
});

packages/metro/src/integration_tests/__tests__/server-torn-down-test.js

+2
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ describe('Server torn down test', () => {
2929
'RANDOMBYTESREQUEST',
3030
'Timeout',
3131
'TickObject',
32+
'FILEHANDLE',
33+
'FILEHANDLECLOSEREQ',
3234
'FSREQCALLBACK',
3335
'FSREQPROMISE',
3436
'FSEVENTWRAP',

0 commit comments

Comments
 (0)