Skip to content

Commit

Permalink
fix: prefer fs/promises over promisify (#7399)
Browse files Browse the repository at this point in the history
  • Loading branch information
lukekarrys authored Apr 24, 2024
1 parent 03958c3 commit 78447d7
Show file tree
Hide file tree
Showing 19 changed files with 105 additions and 131 deletions.
7 changes: 1 addition & 6 deletions lib/commands/doctor.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
const cacache = require('cacache')
const fs = require('fs')
const { access, lstat, readdir, constants: { R_OK, W_OK, X_OK } } = require('fs/promises')
const fetch = require('make-fetch-happen')
const which = require('which')
const pacote = require('pacote')
const { resolve } = require('path')
const semver = require('semver')
const { promisify } = require('util')
const { log, output } = require('proc-log')
const ping = require('../utils/ping.js')
const { defaults } = require('@npmcli/config/lib/definitions')
const lstat = promisify(fs.lstat)
const readdir = promisify(fs.readdir)
const access = promisify(fs.access)
const { R_OK, W_OK, X_OK } = fs.constants

const maskLabel = mask => {
const label = []
Expand Down
26 changes: 8 additions & 18 deletions lib/commands/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// open the package folder in the $EDITOR

const { resolve } = require('path')
const fs = require('graceful-fs')
const { lstat } = require('fs/promises')
const cp = require('child_process')
const completion = require('../utils/completion/installed-shallow.js')
const BaseCommand = require('../base-command.js')
Expand Down Expand Up @@ -50,25 +50,15 @@ class Edit extends BaseCommand {
const path = splitPackageNames(args[0])
const dir = resolve(this.npm.dir, path)

// graceful-fs does not promisify
await lstat(dir)
await new Promise((res, rej) => {
fs.lstat(dir, (err) => {
if (err) {
return rej(err)
const [bin, ...spawnArgs] = this.npm.config.get('editor').split(/\s+/)
const editor = cp.spawn(bin, [...spawnArgs, dir], { stdio: 'inherit' })
editor.on('exit', async (code) => {
if (code) {
return rej(new Error(`editor process exited with code: ${code}`))
}
const [bin, ...spawnArgs] = this.npm.config.get('editor').split(/\s+/)
const editor = cp.spawn(bin, [...spawnArgs, dir], { stdio: 'inherit' })
editor.on('exit', async (code) => {
if (code) {
return rej(new Error(`editor process exited with code: ${code}`))
}
try {
await this.npm.exec('rebuild', [dir])
} catch (execErr) {
rej(execErr)
}
res()
})
await this.npm.exec('rebuild', [dir]).then(res).catch(rej)
})
})
}
Expand Down
7 changes: 3 additions & 4 deletions lib/commands/init.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const fs = require('fs')
const { statSync } = require('fs')
const { relative, resolve } = require('path')
const { mkdir } = require('fs/promises')
const initJson = require('init-package-json')
Expand All @@ -8,11 +8,10 @@ const mapWorkspaces = require('@npmcli/map-workspaces')
const PackageJson = require('@npmcli/package-json')
const { log, output } = require('proc-log')
const updateWorkspaces = require('../workspaces/update-workspaces.js')
const BaseCommand = require('../base-command.js')

const posixPath = p => p.split('\\').join('/')

const BaseCommand = require('../base-command.js')

class Init extends BaseCommand {
static description = 'Create a package.json file'
static params = [
Expand Down Expand Up @@ -197,7 +196,7 @@ class Init extends BaseCommand {
// mapWorkspaces, so we're just going to avoid touching the
// top-level package.json
try {
fs.statSync(resolve(workspacePath, 'package.json'))
statSync(resolve(workspacePath, 'package.json'))
} catch (err) {
return
}
Expand Down
8 changes: 2 additions & 6 deletions lib/commands/link.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
const fs = require('fs')
const util = require('util')
const readdir = util.promisify(fs.readdir)
const { readdir } = require('fs/promises')
const { resolve } = require('path')

const npa = require('npm-package-arg')
const pkgJson = require('@npmcli/package-json')
const semver = require('semver')

const reifyFinish = require('../utils/reify-finish.js')

const ArboristWorkspaceCmd = require('../arborist-cmd.js')

class Link extends ArboristWorkspaceCmd {
static description = 'Symlink a package folder'
static name = 'link'
Expand Down
3 changes: 2 additions & 1 deletion lib/commands/shrinkwrap.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
const { resolve, basename } = require('path')
const { unlink } = require('fs').promises
const { unlink } = require('fs/promises')
const { log } = require('proc-log')
const BaseCommand = require('../base-command.js')

class Shrinkwrap extends BaseCommand {
static description = 'Lock down dependency versions for publication'
static name = 'shrinkwrap'
Expand Down
5 changes: 2 additions & 3 deletions lib/commands/view.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
const columns = require('cli-columns')
const fs = require('fs')
const { readFile } = require('fs/promises')
const jsonParse = require('json-parse-even-better-errors')
const { log, output } = require('proc-log')
const npa = require('npm-package-arg')
const { resolve } = require('path')
const formatBytes = require('../utils/format-bytes.js')
const relativeDate = require('tiny-relative-date')
const semver = require('semver')
const { inspect, promisify } = require('util')
const { inspect } = require('util')
const { packument } = require('pacote')

const readFile = promisify(fs.readFile)
const readJson = async file => jsonParse(await readFile(file, 'utf8'))

const Queryable = require('../utils/queryable.js')
Expand Down
2 changes: 1 addition & 1 deletion lib/utils/reify-finish.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const reifyOutput = require('./reify-output.js')
const ini = require('ini')
const { writeFile } = require('fs').promises
const { writeFile } = require('fs/promises')
const { resolve } = require('path')

const reifyFinish = async (npm, arb) => {
Expand Down
10 changes: 5 additions & 5 deletions tap-snapshots/test/lib/commands/doctor.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -731,11 +731,11 @@ Object {
"warn": Array [
String(
doctor getGitPath Error: test error
doctor at which ({CWD}/{TESTDIR}/doctor.js:313:15)
doctor at Doctor.getGitPath ({CWD}/lib/commands/doctor.js:286:18)
doctor at Doctor.exec ({CWD}/lib/commands/doctor.js:125:33)
doctor at processTicksAndRejections (node:internal/process/task_queues:95:5)
doctor at MockNpm.exec ({CWD}/test/fixtures/mock-npm.js:80:26)
doctor at which {STACK}
doctor at Doctor.getGitPath {STACK}
doctor at Doctor.exec {STACK}
doctor at processTicksAndRejections {STACK}
doctor at MockNpm.exec {STACK}
),
],
}
Expand Down
23 changes: 14 additions & 9 deletions test/lib/commands/doctor.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const t = require('tap')
const fs = require('fs')
const fs = require('fs/promises')
const path = require('path')

const { load: loadMockNpm } = require('../../fixtures/mock-npm')
Expand All @@ -11,6 +11,7 @@ const cleanCacheSha = (str) =>
str.replace(/content-v2\/sha512\/[^"]+/g, 'content-v2/sha512/{sha}')

t.cleanSnapshot = p => cleanCacheSha(cleanDate(cleanCwd(p)))
.replace(/\s\((\{CWD\}|node:).*\d+:\d+\)$/gm, ' {STACK}')

const npmManifest = (version) => {
return {
Expand Down Expand Up @@ -389,15 +390,15 @@ t.test('incorrect owner', async t => {
const { joinedOutput, logs, npm } = await loadMockNpm(t, {
mocks: {
...mocks,
fs: {
'fs/promises': {
...fs,
lstat: (p, cb) => {
const stat = fs.lstatSync(p)
lstat: async (p) => {
const stat = await fs.lstat(p)
if (p.endsWith('_cacache')) {
stat.uid += 1
stat.gid += 1
}
return cb(null, stat)
return stat
},
},
},
Expand All @@ -418,9 +419,9 @@ t.test('incorrect permissions', async t => {
const { joinedOutput, logs, npm } = await loadMockNpm(t, {
mocks: {
...mocks,
fs: {
'fs/promises': {
...fs,
access: () => {
access: async () => {
throw new Error('Test Error')
},
},
Expand All @@ -442,9 +443,13 @@ t.test('error reading directory', async t => {
const { joinedOutput, logs, npm } = await loadMockNpm(t, {
mocks: {
...mocks,
fs: {
'fs/promises': {
...fs,
readdir: () => {
readdir: async (s, ...args) => {
if (s.endsWith('_logs')) {
return fs.readdir(s, ...args)
}
// if (s.endsWith)
throw new Error('Test Error')
},
},
Expand Down
4 changes: 2 additions & 2 deletions workspaces/arborist/lib/arborist/isolated-reifier.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const _makeIdealGraph = Symbol('makeIdealGraph')
const _createIsolatedTree = Symbol.for('createIsolatedTree')
const _createBundledTree = Symbol('createBundledTree')
const fs = require('fs')
const { mkdirSync } = require('fs')
const pacote = require('pacote')
const { join } = require('path')
const { depth } = require('treeverse')
Expand Down Expand Up @@ -108,7 +108,7 @@ module.exports = cls => class IsolatedReifier extends cls {
'.store',
`${node.name}@${node.version}`
)
fs.mkdirSync(dir, { recursive: true })
mkdirSync(dir, { recursive: true })
// TODO this approach feels wrong
// and shouldn't be necessary for shrinkwraps
await pacote.extract(node.resolved, dir, {
Expand Down
6 changes: 4 additions & 2 deletions workspaces/arborist/scripts/benchmark.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
process.env.ARBORIST_DEBUG = '0'

const { Suite } = require('benchmark')
const { relative, resolve } = require('path')
const { mkdir, rm } = require('fs/promises')
const { execSync } = require('child_process')
const { linkSync, writeFileSync, readdirSync } = require('fs')
const registryServer = require('../test/fixtures/server.js')

const shaCmd = 'git show --no-patch --pretty=%H HEAD'
const dirty = !!String(execSync('git status -s -uno')).trim()
const currentSha = String(execSync(shaCmd)).trim() + (dirty ? '-dirty' : '')
const lastBenchmark = resolve(__dirname, 'benchmark/saved/last-benchmark.json')
const { linkSync, writeFileSync, readdirSync } = require('fs')
const registryServer = require('../test/fixtures/server.js')

const red = m => `\x1B[31m${m}\x1B[39m`
const green = m => `\x1B[32m${m}\x1B[39m`
Expand Down
6 changes: 2 additions & 4 deletions workspaces/arborist/scripts/benchmark/load-actual.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
const Arborist = require('../..')
const { resolve, basename } = require('path')
const { writeFileSync } = require('fs')
const {
mkdir,
rm,
} = require('fs/promises')
const { mkdir, rm } = require('fs/promises')

const dir = resolve(__dirname, basename(__filename, '.js'))

const suite = async (suite, { registry, cache }) => {
Expand Down
3 changes: 1 addition & 2 deletions workspaces/arborist/scripts/benchmark/reify.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
const Arborist = require('../..')
const { resolve, basename } = require('path')
const { writeFileSync } = require('fs')
const { writeFileSync, rmSync } = require('fs')
const { mkdir } = require('fs/promises')
const { rmSync } = require('fs')
const dir = resolve(__dirname, basename(__filename, '.js'))

// these are not arbitrary, the empty/full and no-* bits matter
Expand Down
13 changes: 6 additions & 7 deletions workspaces/arborist/test/arborist/pruner.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,16 @@ t.test('prune with lockfile omit dev', async t => {
})

t.test('prune omit dev with bins', async t => {
const fs = require('fs')
const { promisify } = require('util')
const readdir = promisify(fs.readdir)
const { readdir } = require('fs/promises')
const { statSync, lstatSync } = require('fs')
const path = fixture(t, 'prune-dev-bins')

// should have bin files
const reifiedBin = resolve(path, 'node_modules/.bin/yes')
if (process.platform === 'win32') {
t.ok(fs.statSync(reifiedBin + '.cmd').isFile(), 'should have shim')
t.ok(statSync(reifiedBin + '.cmd').isFile(), 'should have shim')
} else {
t.ok(fs.lstatSync(reifiedBin).isSymbolicLink(), 'should have symlink')
t.ok(lstatSync(reifiedBin).isSymbolicLink(), 'should have symlink')
}

// PRUNE things
Expand All @@ -107,9 +106,9 @@ t.test('prune omit dev with bins', async t => {
// should also remove ./bin/* files
const bin = resolve(path, 'node_modules/.bin/yes')
if (process.platform === 'win32') {
t.throws(() => fs.statSync(bin + '.cmd').isFile(), /ENOENT/, 'should not have shim')
t.throws(() => statSync(bin + '.cmd').isFile(), /ENOENT/, 'should not have shim')
} else {
t.throws(() => fs.lstatSync(bin).isSymbolicLink(), /ENOENT/, 'should not have symlink')
t.throws(() => lstatSync(bin).isSymbolicLink(), /ENOENT/, 'should not have symlink')
}
})

Expand Down
4 changes: 2 additions & 2 deletions workspaces/config/lib/definitions/definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ const { join } = require('node:path')
const isWindows = process.platform === 'win32'

// used by cafile flattening to flatOptions.ca
const fs = require('fs')
const { readFileSync } = require('fs')
const maybeReadFile = file => {
try {
return fs.readFileSync(file, 'utf8')
return readFileSync(file, 'utf8')
} catch (er) {
if (er.code !== 'ENOENT') {
throw er
Expand Down
3 changes: 1 addition & 2 deletions workspaces/libnpmpack/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ const pacote = require('pacote')
const npa = require('npm-package-arg')
const runScript = require('@npmcli/run-script')
const path = require('path')
const util = require('util')
const Arborist = require('@npmcli/arborist')
const writeFile = util.promisify(require('fs').writeFile)
const { writeFile } = require('fs/promises')

module.exports = pack
async function pack (spec = 'file:.', opts = {}) {
Expand Down
3 changes: 1 addition & 2 deletions workspaces/libnpmversion/lib/read-json.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// can't use read-package-json-fast, because we want to ensure
// that we make as few changes as possible, even for safety issues.
const { promisify } = require('util')
const readFile = promisify(require('fs').readFile)
const { readFile } = require('fs/promises')
const parse = require('json-parse-even-better-errors')

module.exports = async path => parse(await readFile(path))
3 changes: 1 addition & 2 deletions workspaces/libnpmversion/lib/write-json.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// write the json back, preserving the line breaks and indent
const { promisify } = require('util')
const writeFile = promisify(require('fs').writeFile)
const { writeFile } = require('fs/promises')
const kIndent = Symbol.for('indent')
const kNewline = Symbol.for('newline')

Expand Down
Loading

0 comments on commit 78447d7

Please sign in to comment.