chore(shared-ini-file-loader): use Promises in slurpFile hash (#3546)

Author: trivikr

packages/shared-ini-file-loader/src/slurpFile.spec.ts

@@ -8,35 +8,56 @@ describe("slurpFile", () => {
   const getMockFileContents = (path: string, options = UTF8) => JSON.stringify({ path, options });
 
   beforeEach(() => {
-    (promises.readFile as jest.Mock).mockImplementation((path, options) =>
-      Promise.resolve(getMockFileContents(path, options))
-    );
+    (promises.readFile as jest.Mock).mockImplementation(async (path, options) => {
+      await new Promise((resolve) => setTimeout(resolve, 100));
+      return getMockFileContents(path, options);
+    });
   });
 
   afterEach(() => {
     jest.clearAllMocks();
   });
 
-  it("makes one readFile call for a filepath irrepsective of slurpFile calls", (done) => {
-    jest.isolateModules(async () => {
-      const { slurpFile } = require("./slurpFile");
-      const mockPath = "/mock/path";
-      const mockPathContent = getMockFileContents(mockPath);
+  describe("makes one readFile call for a filepath irrespective of slurpFile calls", () => {
+    // @ts-ignore: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/34617
+    it.each([10, 100, 1000, 10000])("parallel calls: %d ", (num: number, done: Function) => {
+      jest.isolateModules(async () => {
+        const { slurpFile } = require("./slurpFile");
+        const mockPath = "/mock/path";
+        const mockPathContent = getMockFileContents(mockPath);
+
+        expect(promises.readFile).not.toHaveBeenCalled();
+        const fileContentArr = await Promise.all(Array(num).fill(slurpFile(mockPath)));
+        expect(fileContentArr).toStrictEqual(Array(num).fill(mockPathContent));
+
+        // There is one readFile call even through slurpFile is called in parallel num times.
+        expect(promises.readFile).toHaveBeenCalledTimes(1);
+        expect(promises.readFile).toHaveBeenCalledWith(mockPath, UTF8);
+        done();
+      });
+    });
 
-      expect(promises.readFile).not.toHaveBeenCalled();
-      const fileContentArr = await Promise.all([slurpFile(mockPath), slurpFile(mockPath)]);
-      expect(fileContentArr).toStrictEqual([mockPathContent, mockPathContent]);
+    it("two parallel calls and one sequential call", (done) => {
+      jest.isolateModules(async () => {
+        const { slurpFile } = require("./slurpFile");
+        const mockPath = "/mock/path";
+        const mockPathContent = getMockFileContents(mockPath);
 
-      // There is one readFile call even through slurpFile is called in parallel twice.
-      expect(promises.readFile).toHaveBeenCalledTimes(1);
-      expect(promises.readFile).toHaveBeenCalledWith(mockPath, UTF8);
+        expect(promises.readFile).not.toHaveBeenCalled();
+        const fileContentArr = await Promise.all([slurpFile(mockPath), slurpFile(mockPath)]);
+        expect(fileContentArr).toStrictEqual([mockPathContent, mockPathContent]);
 
-      const fileContent = await slurpFile(mockPath);
-      expect(fileContent).toStrictEqual(mockPathContent);
+        // There is one readFile call even through slurpFile is called in parallel twice.
+        expect(promises.readFile).toHaveBeenCalledTimes(1);
+        expect(promises.readFile).toHaveBeenCalledWith(mockPath, UTF8);
 
-      // There is one readFile call even through slurpFile is called for the third time.
-      expect(promises.readFile).toHaveBeenCalledTimes(1);
-      done();
+        const fileContent = await slurpFile(mockPath);
+        expect(fileContent).toStrictEqual(mockPathContent);
+
+        // There is one readFile call even through slurpFile is called for the third time.
+        expect(promises.readFile).toHaveBeenCalledTimes(1);
+        done();
+      });
     });
   });
 

packages/shared-ini-file-loader/src/slurpFile.ts

@@ -3,45 +3,11 @@ import { promises as fsPromises } from "fs";
 
 const { readFile } = fsPromises;
 
-type callbacks = { resolve: Function; reject: Function };
-type FileStatus = {
-  contents: string;
-  isReading: boolean;
-  requestQueue: callbacks[];
-};
-
-const fileStatusHash: { [key: string]: FileStatus } = {};
+const filePromisesHash: { [key: string]: Promise<string> } = {};
 
-export const slurpFile = (path: string) =>
-  new Promise<string>((resolve, reject) => {
-    if (!fileStatusHash[path]) {
-      // File not read yet, set file isReading to true and read file.
-      fileStatusHash[path] = { isReading: true, contents: "", requestQueue: [] };
-      fileStatusHash[path].requestQueue.push({ resolve, reject });
-      readFile(path, "utf8")
-        .then((data) => {
-          // File read successful
-          fileStatusHash[path].isReading = false;
-          fileStatusHash[path].contents = data;
-          const { requestQueue } = fileStatusHash[path];
-          while (requestQueue.length) {
-            const { resolve } = requestQueue.pop()!;
-            resolve(data);
-          }
-        })
-        .catch((err) => {
-          // File read failed;
-          fileStatusHash[path].isReading = false;
-          const { requestQueue } = fileStatusHash[path];
-          while (requestQueue.length) {
-            const { reject } = requestQueue.pop()!;
-            reject(err);
-          }
-        });
-    } else if (fileStatusHash[path].isReading) {
-      // File currently being read. Add callbacks to the request queue.
-      fileStatusHash[path].requestQueue.push({ resolve, reject });
-    } else {
-      resolve(fileStatusHash[path].contents);
-    }
-  });
+export const slurpFile = (path: string) => {
+  if (!filePromisesHash[path]) {
+    filePromisesHash[path] = readFile(path, "utf8");
+  }
+  return filePromisesHash[path];
+};