Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor + chore cleanup #322

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Prev Previous commit
Next Next commit
Do not pass unnecessary rest args to fs functions
lukekarrys committed Oct 10, 2024
commit 604153acefd65284488cf7171eb3f1f0d3b5b37b
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 {
@@ -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 })

@@ -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) =>
@@ -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) =>
@@ -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,
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
@@ -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 }]])
})