fix(fslib): fix FileHandle implementations

.yarn/versions/a0e6ef2e.yml

@@ -0,0 +1,37 @@
+releases:
+  "@yarnpkg/builder": patch
+  "@yarnpkg/cli": patch
+  "@yarnpkg/core": patch
+  "@yarnpkg/doctor": patch
+  "@yarnpkg/esbuild-plugin-pnp": patch
+  "@yarnpkg/fslib": minor
+  "@yarnpkg/json-proxy": patch
+  "@yarnpkg/nm": patch
+  "@yarnpkg/plugin-compat": patch
+  "@yarnpkg/plugin-constraints": patch
+  "@yarnpkg/plugin-dlx": patch
+  "@yarnpkg/plugin-essentials": patch
+  "@yarnpkg/plugin-exec": patch
+  "@yarnpkg/plugin-file": patch
+  "@yarnpkg/plugin-git": patch
+  "@yarnpkg/plugin-github": patch
+  "@yarnpkg/plugin-http": patch
+  "@yarnpkg/plugin-init": patch
+  "@yarnpkg/plugin-interactive-tools": patch
+  "@yarnpkg/plugin-link": patch
+  "@yarnpkg/plugin-nm": patch
+  "@yarnpkg/plugin-npm": patch
+  "@yarnpkg/plugin-npm-cli": patch
+  "@yarnpkg/plugin-pack": patch
+  "@yarnpkg/plugin-patch": patch
+  "@yarnpkg/plugin-pnp": patch
+  "@yarnpkg/plugin-pnpm": patch
+  "@yarnpkg/plugin-stage": patch
+  "@yarnpkg/plugin-typescript": patch
+  "@yarnpkg/plugin-version": patch
+  "@yarnpkg/plugin-workspace-tools": patch
+  "@yarnpkg/pnp": patch
+  "@yarnpkg/pnpify": patch
+  "@yarnpkg/sdks": patch
+  "@yarnpkg/shell": patch
+  vscode-zipfs: patch

packages/yarnpkg-fslib/sources/patchFs.ts

@@ -70,18 +70,20 @@ const ASYNC_IMPLEMENTATIONS = new Set([
   `writeSync`,
 ]);
 
-const FILEHANDLE_IMPLEMENTATIONS = new Set([
-  `appendFilePromise`,
-  `chmodPromise`,
-  `chownPromise`,
-  `closePromise`,
-  `readPromise`,
-  `readFilePromise`,
-  `statPromise`,
-  `truncatePromise`,
-  `utimesPromise`,
-  `writePromise`,
-  `writeFilePromise`,
+/**
+ * List of `FileHandle` methods
+ * Set `shouldProvidePath` for methods that expect `path` as the first argument, otherwise `fd` will be passed
+ */
+const FILEHANDLE_IMPLEMENTATIONS = new Set<[string, {shouldProvidePath: boolean}]>([
+  [`appendFilePromise`, {shouldProvidePath: false}],
+  [`chmodPromise`, {shouldProvidePath: true}],
+  [`chownPromise`, {shouldProvidePath: true}],
+  [`closePromise`, {shouldProvidePath: false}],
+  [`readFilePromise`, {shouldProvidePath: false}],
+  [`statPromise`, {shouldProvidePath: true}],
+  [`truncatePromise`, {shouldProvidePath: true}],
+  [`utimesPromise`, {shouldProvidePath: true}],
+  [`writeFilePromise`, {shouldProvidePath: false}],
 ]);
 
 //#region readSync types
@@ -335,26 +337,43 @@ export function patchFs(patchedFs: typeof fs, fakeFs: FakeFS<NativePath>): void
       }
 
       class FileHandle {
-        constructor(public fd: number) {
+        public _path: unknown;
+        constructor(public fd: number, _path: unknown) {
+          Object.defineProperty(this, `_path`, {
+            value: _path,
+            enumerable: false,
+            writable: false,
+            configurable: false,
+          });
         }
       }
 
-      for (const fnName of FILEHANDLE_IMPLEMENTATIONS) {
+      for (const [fnName, {shouldProvidePath}] of FILEHANDLE_IMPLEMENTATIONS) {
         const origName = fnName.replace(/Promise$/, ``);
 
         const fakeImpl: Function = (fakeFs as any)[fnName];
         if (typeof fakeImpl === `undefined`)
           continue;
 
         setupFn(FileHandle.prototype, origName, function (this: FileHandle, ...args: Array<any>) {
-          return fakeImpl.call(fakeFs, this.fd, ...args);
+          return fakeImpl.call(fakeFs, !shouldProvidePath ? this.fd : this._path, ...args);
         });
       }
 
-      setupFn(patchedFsPromises, `open`, async (...args: Array<any>) => {
+      setupFn(FileHandle.prototype, `read`, function (this: FileHandle, ...args: Array<any>) {
         // @ts-expect-error
-        const fd = await fakeFs.openPromise(...args);
-        return new FileHandle(fd);
+        return patchedFs.read[promisify.custom](this.fd, ...args);
+      });
+
+      setupFn(FileHandle.prototype, `write`, function (this: FileHandle, ...args: Array<any>) {
+        // @ts-expect-error
+        return patchedFs.write[promisify.custom](this.fd, ...args);
+      });
+
+      setupFn(patchedFsPromises, `open`, async (path: unknown, ...args: Array<any>) => {
+        // @ts-expect-error
+        const fd = await fakeFs.openPromise(path, ...args);
+        return new FileHandle(fd, path);
       });
 
       // `fs.promises.realpath` doesn't have a `native` property
@@ -363,15 +382,22 @@ export function patchFs(patchedFs: typeof fs, fakeFs: FakeFS<NativePath>): void
 
   /** util.promisify implementations */
   {
-    // Override the promisified version of `fs.read` to return an object as per
+    // Override the promisified versions of `fs.read` and `fs.write` to return an object as per
     // https://github.com/nodejs/node/blob/dc79f3f37caf6f25b8efee4623bec31e2c20f595/lib/fs.js#L559-L560
     // and
+    // https://github.com/nodejs/node/blob/dc79f3f37caf6f25b8efee4623bec31e2c20f595/lib/fs.js#L690-L691
+    // and
     // https://github.com/nodejs/node/blob/ba684805b6c0eded76e5cd89ee00328ac7a59365/lib/internal/util.js#L293
     // @ts-expect-error
-    patchedFs.read[promisify.custom] = async (p: number, buffer: Buffer, ...args: Array<any>) => {
-      const res = fakeFs.readPromise(p, buffer, ...args);
+    patchedFs.read[promisify.custom] = async (fd: number, buffer: Buffer, ...args: Array<any>) => {
+      const res = fakeFs.readPromise(fd, buffer, ...args);
       return {bytesRead: await res, buffer};
     };
+    // @ts-expect-error
+    patchedFs.write[promisify.custom] = async (fd: number, buffer: Buffer, ...args: Array<any>) => {
+      const res = fakeFs.writePromise(fd, buffer, ...args);
+      return {bytesWritten: await res, buffer};
+    };
   }
 }
 

packages/yarnpkg-fslib/tests/patchedFs.test.ts

@@ -63,6 +63,24 @@ describe(`patchedFs`, () => {
     expect(Buffer.isBuffer(result.buffer)).toBeTruthy();
   });
 
+  it(`matches the util.promisify return shape of node: fs.write`, async () => {
+    const patchedFs = extendFs(fs, new PosixFS(new NodeFS()));
+    const patchedFsWriteAsync = promisify(patchedFs.write);
+
+    const tmpdir = npath.fromPortablePath(xfs.mktempSync());
+
+    const file = npath.join(tmpdir, `file.txt`);
+
+    const fd = fs.openSync(file, `w`);
+
+    const bufferFs = Buffer.alloc(16);
+
+    const result = await patchedFsWriteAsync(fd, bufferFs, 0, 16, 0);
+
+    expect(typeof result.bytesWritten).toBe(`number`);
+    expect(Buffer.isBuffer(result.buffer)).toBeTruthy();
+  });
+
   it(`should support URL instances`, () => {
     const patchedFs = extendFs(fs, new PosixFS(new NodeFS()));
 
@@ -239,4 +257,66 @@ describe(`patchedFs`, () => {
       patchedFs.closeSync(fd);
     }
   });
+
+  it(`should support FileHandle implementation: read`, async() => {
+    const patchedFs = extendFs(fs, new PosixFS(new NodeFS()));
+
+    const handle = await patchedFs.promises.open(__filename, `r`);
+
+    const buffer = Buffer.alloc(42);
+    try {
+      const {bytesRead} = await handle.read(buffer, 0, buffer.byteLength);
+
+      expect(bytesRead).toEqual(buffer.byteLength);
+    } finally {
+      await handle.close();
+    }
+  });
+
+  it(`should support FileHandle implementation: write`, async() => {
+    const patchedFs = extendFs(fs, new PosixFS(new NodeFS()));
+
+    const tmpdir = npath.fromPortablePath(xfs.mktempSync());
+
+    const file = npath.join(tmpdir, `file.txt`);
+
+    const handle = await patchedFs.promises.open(file, `w`);
+
+    const buffer = Buffer.from(`hello`);
+    try {
+      const {bytesWritten} = await handle.write(buffer);
+
+      expect(bytesWritten).toEqual(buffer.byteLength);
+    } finally {
+      await handle.close();
+    }
+  });
+
+  it(`should support FileHandle implementations for methods that accept a file descriptor`, async() => {
+    const patchedFs = extendFs(fs, new PosixFS(new NodeFS()));
+
+    const handle = await patchedFs.promises.open(__filename, `r`);
+
+    try {
+      const contents = await handle.readFile();
+
+      expect(contents.byteLength).toBeGreaterThan(0);
+    } finally {
+      await handle.close();
+    }
+  });
+
+  it(`should support FileHandle implementations for methods that accept a path`, async() => {
+    const patchedFs = extendFs(fs, new PosixFS(new NodeFS()));
+
+    const handle = await patchedFs.promises.open(__filename, `r`);
+
+    try {
+      const stat = await handle.stat();
+
+      expect(stat.size).toBeGreaterThan(0);
+    } finally {
+      await handle.close();
+    }
+  });
 });