Skip to content

Commit

Permalink
Do not pass unnecessary rest args to fs functions
Browse files Browse the repository at this point in the history
  • Loading branch information
lukekarrys committed Oct 10, 2024
1 parent 5ba463f commit 943c1e5
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 48 deletions.
23 changes: 6 additions & 17 deletions src/fs.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// promisify ourselves, because older nodes don't have fs.promises

import fs, { Dirent } from 'fs'
import { readdirSync as rdSync } from 'fs'

// sync ones just take the sync version from node
export {
Expand All @@ -14,7 +15,6 @@ export {
unlinkSync,
} from 'fs'

import { readdirSync as rdSync } from 'fs'
export const readdirSync = (path: fs.PathLike): Dirent[] =>
rdSync(path, { withFileTypes: true })

Expand All @@ -24,16 +24,13 @@ export const readdirSync = (path: fs.PathLike): Dirent[] =>
// 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, ...d: any[]) => (er ? rej(er) : res(...d))),
)
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 })
| undefined
| null,
): Promise<string | undefined> =>
new Promise((res, rej) =>
Expand All @@ -49,20 +46,14 @@ const readdir = (path: fs.PathLike): Promise<Dirent[]> =>

const rename = (oldPath: fs.PathLike, newPath: fs.PathLike): Promise<void> =>
new Promise((res, rej) =>
fs.rename(oldPath, newPath, (er, ...d: any[]) =>
er ? rej(er) : res(...d),
),
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, ...d: any[]) => (er ? rej(er) : res(...d))),
)
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, ...d: any[]) => (er ? rej(er) : res(...d))),
)
new Promise((res, rej) => fs.rmdir(path, er => (er ? rej(er) : res())))

const stat = (path: fs.PathLike): Promise<fs.Stats> =>
new Promise((res, rej) =>
Expand All @@ -75,9 +66,7 @@ const lstat = (path: fs.PathLike): Promise<fs.Stats> =>
)

const unlink = (path: fs.PathLike): Promise<void> =>
new Promise((res, rej) =>
fs.unlink(path, (er, ...d: any[]) => (er ? rej(er) : res(...d))),
)
new Promise((res, rej) => fs.unlink(path, er => (er ? rej(er) : res())))

export const promises = {
chmod,
Expand Down
70 changes: 39 additions & 31 deletions test/fs.ts
Original file line number Diff line number Diff line change
@@ -1,55 +1,58 @@
import t from 'tap'
import t, { Test } from 'tap'

// verify that every function in the root is *Sync, and every
// function is fs.promises is the promisified version of fs[method],
// 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 * as fs from '../dist/esm/fs.js'
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) =>
(await t.mockImport('../src/fs.js', {
fs: t.createMock(realFS, mock),
})) as typeof import('../src/fs.js')

const mockFSMethodPass =
(method: string) =>
(...args: any[]) => {
const cb = args.pop()
process.nextTick(() => cb(null, method, 1, 2, 3))
(...args: unknown[]) => {
process.nextTick(() => (args.at(-1) as MockCb)(null, method))
}
const mockFSMethodFail =
(method: string) =>
(...args: any[]) => {
const cb = args.pop()
process.nextTick(() => cb(new Error('oops'), method, 1, 2, 3))
() =>
(...args: unknown[]) => {
process.nextTick(() => (args.at(-1) as MockCb)(new Error('oops')))
}

import { useNative } from '../dist/esm/use-native.js'
t.type(fs.promises, Object)
const mockFSPass: Record<string, (...a: any[]) => any> = {}
const mockFSFail: Record<string, (...a: any[]) => any> = {}
const mockFSPass: MockFsCb = {}
const mockFSFail: MockFsCb = {}

for (const method of Object.keys(
fs.promises as Record<string, (...a: any[]) => any>,
)) {
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()) {
t.type(
(realFS as { [k: string]: any })[method],
(realFS as unknown as MockFsCb)[method],
Function,
`real fs.${method} is a function`,
)
if (method !== 'readdir') {
t.equal(
(fs as { [k: string]: any })[`${method}Sync`],
(realFS as unknown as { [k: string]: (...a: any[]) => any })[
`${method}Sync`
],
(fs as unknown as MockFsCb)[`${method}Sync`],
(realFS as unknown as MockFsCb)[`${method}Sync`],
`has ${method}Sync`,
)
}
}

// set up our pass/fails for the next tests
mockFSPass[method] = mockFSMethodPass(method)
mockFSFail[method] = mockFSMethodFail(method)
mockFSFail[method] = mockFSMethodFail()
}

// doesn't have any sync versions that aren't promisified
Expand All @@ -59,30 +62,35 @@ for (const method of Object.keys(fs)) {
}
const m = method.replace(/Sync$/, '')
t.type(
(fs.promises as { [k: string]: (...a: any[]) => any })[m],
(fs.promises as unknown as MockFsCb)[m],
Function,
`fs.promises.${m} is a function`,
)
}

t.test('passing resolves promise', async t => {
const fs = (await t.mockImport('../src/fs.js', {
fs: { ...realFS, ...mockFSPass },
})) as typeof import('../src/fs.js')
const fs = await mockFs(t, mockFSPass)
for (const [m, fn] of Object.entries(
fs.promises as { [k: string]: (...a: any) => Promise<any> },
fs.promises as unknown as MockFsPromise,
)) {
t.same(await fn(), m, `got expected value for ${m}`)
const expected =
['chmod', 'rename', 'rm', 'rmdir', 'unlink'].includes(m) ? undefined : m
t.same(await fn(), expected, `got expected value for ${m}`)
}
})

t.test('failing rejects promise', async t => {
const fs = (await t.mockImport('../src/fs.js', {
fs: { ...realFS, ...mockFSFail },
})) as typeof import('../src/fs.js')
const fs = await mockFs(t, mockFSFail)
for (const [m, fn] of Object.entries(
fs.promises as { [k: string]: (...a: any[]) => Promise<any> },
fs.promises as unknown as MockFsPromise,
)) {
t.rejects(fn(), { message: 'oops' }, `got expected value for ${m}`)
}
})

t.test('readdirSync', async t => {
const args: unknown[][] = []
const fs = await mockFs(t, { readdirSync: (...a) => args.push(a) })
fs.readdirSync('x')
t.strictSame(args, [['x', { withFileTypes: true }]])
})

0 comments on commit 943c1e5

Please sign in to comment.