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

fix: prefer fs/promises over promisify #7399

Merged
merged 1 commit into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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')
lukekarrys marked this conversation as resolved.
Show resolved Hide resolved
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')
lukekarrys marked this conversation as resolved.
Show resolved Hide resolved
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')
lukekarrys marked this conversation as resolved.
Show resolved Hide resolved
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')
lukekarrys marked this conversation as resolved.
Show resolved Hide resolved
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
Loading