refactor: switch to native fs.promises

PR-URL: #284
Credit: @SukkaW, @pmmmwh, @lukekarrys
Close: #284
Close: #314
Reviewed-by: @isaacs

Author: SukkaW

src/fs.ts

@@ -1,9 +1,9 @@
-// promisify ourselves, because older nodes don't have fs.promises
-
-import fs, { Dirent } from 'fs'
-import { readdirSync as rdSync } from 'fs'
+import fs, { Dirent, readdirSync as rdSync } from 'fs'
+import fsPromises from 'fs/promises'
 
 // sync ones just take the sync version from node
+// readdir forces withFileTypes: true
+
 export {
   chmodSync,
   mkdirSync,
@@ -18,64 +18,15 @@ export {
 export const readdirSync = (path: fs.PathLike): Dirent[] =>
   rdSync(path, { withFileTypes: true })
 
-// unrolled for better inlining, this seems to get better performance
-// than something like:
-// const makeCb = (res, rej) => (er, ...d) => er ? rej(er) : res(...d)
-// which would be a bit cleaner.
-
-const chmod = (path: fs.PathLike, mode: fs.Mode): Promise<void> =>
-  new Promise((res, rej) => fs.chmod(path, mode, er => (er ? rej(er) : res())))
-
-const mkdir = (
-  path: fs.PathLike,
-  options?:
-    | fs.Mode
-    | (fs.MakeDirectoryOptions & { recursive?: boolean | null })
-    | null,
-): Promise<string | undefined> =>
-  new Promise((res, rej) =>
-    fs.mkdir(path, options, (er, made) => (er ? rej(er) : res(made))),
-  )
-
-const readdir = (path: fs.PathLike): Promise<Dirent[]> =>
-  new Promise<Dirent[]>((res, rej) =>
-    fs.readdir(path, { withFileTypes: true }, (er, data) =>
-      er ? rej(er) : res(data),
-    ),
-  )
-
-const rename = (oldPath: fs.PathLike, newPath: fs.PathLike): Promise<void> =>
-  new Promise((res, rej) =>
-    fs.rename(oldPath, newPath, er => (er ? rej(er) : res())),
-  )
-
-const rm = (path: fs.PathLike, options: fs.RmOptions): Promise<void> =>
-  new Promise((res, rej) => fs.rm(path, options, er => (er ? rej(er) : res())))
-
-const rmdir = (path: fs.PathLike): Promise<void> =>
-  new Promise((res, rej) => fs.rmdir(path, er => (er ? rej(er) : res())))
-
-const stat = (path: fs.PathLike): Promise<fs.Stats> =>
-  new Promise((res, rej) =>
-    fs.stat(path, (er, data) => (er ? rej(er) : res(data))),
-  )
-
-const lstat = (path: fs.PathLike): Promise<fs.Stats> =>
-  new Promise((res, rej) =>
-    fs.lstat(path, (er, data) => (er ? rej(er) : res(data))),
-  )
-
-const unlink = (path: fs.PathLike): Promise<void> =>
-  new Promise((res, rej) => fs.unlink(path, er => (er ? rej(er) : res())))
-
 export const promises = {
-  chmod,
-  mkdir,
-  readdir,
-  rename,
-  rm,
-  rmdir,
-  stat,
-  lstat,
-  unlink,
+  chmod: fsPromises.chmod,
+  mkdir: fsPromises.mkdir,
+  readdir: (path: fs.PathLike) =>
+    fsPromises.readdir(path, { withFileTypes: true }),
+  rename: fsPromises.rename,
+  rm: fsPromises.rm,
+  rmdir: fsPromises.rmdir,
+  stat: fsPromises.stat,
+  lstat: fsPromises.lstat,
+  unlink: fsPromises.unlink,
 }

test/fs.ts

@@ -5,34 +5,49 @@ import t, { Test } from 'tap'
 // and that when the cb returns an error, the promised version fails,
 // and when the cb returns data, the promisified version resolves to it.
 import realFS from 'fs'
+import realFSP from 'fs/promises'
 import * as fs from '../src/fs.js'
 import { useNative } from '../src/use-native.js'
 
 type MockCb = (e: Error | null, m?: string) => void
 type MockFsCb = Record<string, (cb: MockCb) => void>
 type MockFsPromise = Record<string, () => Promise<void>>
 
-const mockFs = async (t: Test, mock: MockFsCb) =>
+const mockFs = async (t: Test, fs: MockFsCb = {}, fsp: MockFsPromise = {}) =>
   (await t.mockImport('../src/fs.js', {
-    fs: t.createMock(realFS, mock),
+    fs: t.createMock(realFS, fs),
+    'fs/promises': t.createMock(realFSP, fsp),
   })) as typeof import('../src/fs.js')
 
 const mockFSMethodPass =
   (method: string) =>
   (...args: unknown[]) => {
     process.nextTick(() => (args.at(-1) as MockCb)(null, method))
   }
+const mockFSPromiseMethodPass =
+  (_method: string) =>
+  () => new Promise<void>((resolve, _reject) => {
+    resolve()
+  })
 const mockFSMethodFail =
-  () =>
+  (_: string) =>
   (...args: unknown[]) => {
     process.nextTick(() => (args.at(-1) as MockCb)(new Error('oops')))
   }
+const mockFSPromiseMethodFail =
+  (_method: string) =>
+  () => new Promise<void>((_resolve, reject) => {
+    reject(new Error('oops'))
+  })
 
 t.type(fs.promises, Object)
 const mockFSPass: MockFsCb = {}
 const mockFSFail: MockFsCb = {}
+const mockFSPromisesFail: MockFsPromise = {}
+const mockFSPromisesPass: MockFsPromise = {}
 
 for (const method of Object.keys(fs.promises)) {
+
   // of course fs.rm is missing when we shouldn't use native :)
   // also, readdirSync is clubbed to always return file types
   if (method !== 'rm' || useNative()) {
@@ -52,7 +67,9 @@ for (const method of Object.keys(fs.promises)) {
 
   // set up our pass/fails for the next tests
   mockFSPass[method] = mockFSMethodPass(method)
-  mockFSFail[method] = mockFSMethodFail()
+  mockFSPromisesPass[method] = mockFSPromiseMethodPass(method)
+  mockFSFail[method] = mockFSMethodFail(method)
+  mockFSPromisesFail[method] = mockFSPromiseMethodFail(method)
 }
 
 // doesn't have any sync versions that aren't promisified
@@ -69,18 +86,17 @@ for (const method of Object.keys(fs)) {
 }
 
 t.test('passing resolves promise', async t => {
-  const fs = await mockFs(t, mockFSPass)
+  const fs = await mockFs(t, mockFSPass, mockFSPromisesPass)
   for (const [m, fn] of Object.entries(
     fs.promises as unknown as MockFsPromise,
   )) {
-    const expected =
-      ['chmod', 'rename', 'rm', 'rmdir', 'unlink'].includes(m) ? undefined : m
-    t.same(await fn(), expected, `got expected value for ${m}`)
+    t.same(await fn(), undefined, `${m} is a Promise<void> method`)
   }
 })
 
 t.test('failing rejects promise', async t => {
-  const fs = await mockFs(t, mockFSFail)
+  const fs = await mockFs(t, mockFSFail, mockFSPromisesFail)
+
   for (const [m, fn] of Object.entries(
     fs.promises as unknown as MockFsPromise,
   )) {