From 78447d7a35fab870456ba66eee408b2baddca23e Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 24 Apr 2024 09:26:54 -0700 Subject: [PATCH] fix: prefer fs/promises over promisify (#7399) --- lib/commands/doctor.js | 7 +- lib/commands/edit.js | 26 ++--- lib/commands/init.js | 7 +- lib/commands/link.js | 8 +- lib/commands/shrinkwrap.js | 3 +- lib/commands/view.js | 5 +- lib/utils/reify-finish.js | 2 +- .../test/lib/commands/doctor.js.test.cjs | 10 +- test/lib/commands/doctor.js | 23 ++-- .../arborist/lib/arborist/isolated-reifier.js | 4 +- workspaces/arborist/scripts/benchmark.js | 6 +- .../arborist/scripts/benchmark/load-actual.js | 6 +- .../arborist/scripts/benchmark/reify.js | 3 +- workspaces/arborist/test/arborist/pruner.js | 13 ++- .../config/lib/definitions/definitions.js | 4 +- workspaces/libnpmpack/lib/index.js | 3 +- workspaces/libnpmversion/lib/read-json.js | 3 +- workspaces/libnpmversion/lib/write-json.js | 3 +- workspaces/libnpmversion/test/write-json.js | 100 ++++++++---------- 19 files changed, 105 insertions(+), 131 deletions(-) diff --git a/lib/commands/doctor.js b/lib/commands/doctor.js index 3048a123d6eb1..2c8581a4aba4d 100644 --- a/lib/commands/doctor.js +++ b/lib/commands/doctor.js @@ -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 = [] diff --git a/lib/commands/edit.js b/lib/commands/edit.js index fbc7840a39876..e6ab26278740c 100644 --- a/lib/commands/edit.js +++ b/lib/commands/edit.js @@ -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') @@ -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) }) }) } diff --git a/lib/commands/init.js b/lib/commands/init.js index d45dbaa1fa0d7..28067e4def1d0 100644 --- a/lib/commands/init.js +++ b/lib/commands/init.js @@ -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') @@ -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 = [ @@ -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 } diff --git a/lib/commands/link.js b/lib/commands/link.js index cdc248569849c..0442e50dc0d83 100644 --- a/lib/commands/link.js +++ b/lib/commands/link.js @@ -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' diff --git a/lib/commands/shrinkwrap.js b/lib/commands/shrinkwrap.js index 01e1d5fdc1189..56d7ffa88e99e 100644 --- a/lib/commands/shrinkwrap.js +++ b/lib/commands/shrinkwrap.js @@ -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' diff --git a/lib/commands/view.js b/lib/commands/view.js index 744e2badb9581..e8b102737a1e9 100644 --- a/lib/commands/view.js +++ b/lib/commands/view.js @@ -1,5 +1,5 @@ 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') @@ -7,10 +7,9 @@ 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') diff --git a/lib/utils/reify-finish.js b/lib/utils/reify-finish.js index 9b43abcb7610a..0b34a37576860 100644 --- a/lib/utils/reify-finish.js +++ b/lib/utils/reify-finish.js @@ -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) => { diff --git a/tap-snapshots/test/lib/commands/doctor.js.test.cjs b/tap-snapshots/test/lib/commands/doctor.js.test.cjs index 985d76e5d12a5..5c5b122477d81 100644 --- a/tap-snapshots/test/lib/commands/doctor.js.test.cjs +++ b/tap-snapshots/test/lib/commands/doctor.js.test.cjs @@ -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} ), ], } diff --git a/test/lib/commands/doctor.js b/test/lib/commands/doctor.js index 1682a6cccfa48..cbe74aba53ff7 100644 --- a/test/lib/commands/doctor.js +++ b/test/lib/commands/doctor.js @@ -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') @@ -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 { @@ -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 }, }, }, @@ -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') }, }, @@ -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') }, }, diff --git a/workspaces/arborist/lib/arborist/isolated-reifier.js b/workspaces/arborist/lib/arborist/isolated-reifier.js index f4f1bb8e44362..19e4e4a20d83a 100644 --- a/workspaces/arborist/lib/arborist/isolated-reifier.js +++ b/workspaces/arborist/lib/arborist/isolated-reifier.js @@ -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') @@ -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, { diff --git a/workspaces/arborist/scripts/benchmark.js b/workspaces/arborist/scripts/benchmark.js index c25140caa93c6..21f5432de76f6 100644 --- a/workspaces/arborist/scripts/benchmark.js +++ b/workspaces/arborist/scripts/benchmark.js @@ -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` diff --git a/workspaces/arborist/scripts/benchmark/load-actual.js b/workspaces/arborist/scripts/benchmark/load-actual.js index 6194098fa9e4d..ddc8145bcb60d 100644 --- a/workspaces/arborist/scripts/benchmark/load-actual.js +++ b/workspaces/arborist/scripts/benchmark/load-actual.js @@ -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 }) => { diff --git a/workspaces/arborist/scripts/benchmark/reify.js b/workspaces/arborist/scripts/benchmark/reify.js index 84c7ab4b0471d..797a5cb803682 100644 --- a/workspaces/arborist/scripts/benchmark/reify.js +++ b/workspaces/arborist/scripts/benchmark/reify.js @@ -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 diff --git a/workspaces/arborist/test/arborist/pruner.js b/workspaces/arborist/test/arborist/pruner.js index 8de95a5a9ddd4..9be29442acdea 100644 --- a/workspaces/arborist/test/arborist/pruner.js +++ b/workspaces/arborist/test/arborist/pruner.js @@ -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 @@ -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') } }) diff --git a/workspaces/config/lib/definitions/definitions.js b/workspaces/config/lib/definitions/definitions.js index 3565cdb4feb44..57ab171611838 100644 --- a/workspaces/config/lib/definitions/definitions.js +++ b/workspaces/config/lib/definitions/definitions.js @@ -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 diff --git a/workspaces/libnpmpack/lib/index.js b/workspaces/libnpmpack/lib/index.js index 00dc96974a968..c71716cf54428 100644 --- a/workspaces/libnpmpack/lib/index.js +++ b/workspaces/libnpmpack/lib/index.js @@ -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 = {}) { diff --git a/workspaces/libnpmversion/lib/read-json.js b/workspaces/libnpmversion/lib/read-json.js index 2dd0f7aa4902e..32c7289507697 100644 --- a/workspaces/libnpmversion/lib/read-json.js +++ b/workspaces/libnpmversion/lib/read-json.js @@ -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)) diff --git a/workspaces/libnpmversion/lib/write-json.js b/workspaces/libnpmversion/lib/write-json.js index f066d72c67e12..425be8e8e3efb 100644 --- a/workspaces/libnpmversion/lib/write-json.js +++ b/workspaces/libnpmversion/lib/write-json.js @@ -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') diff --git a/workspaces/libnpmversion/test/write-json.js b/workspaces/libnpmversion/test/write-json.js index a8e5b15728bd7..63dccf6cf5d0e 100644 --- a/workspaces/libnpmversion/test/write-json.js +++ b/workspaces/libnpmversion/test/write-json.js @@ -1,60 +1,54 @@ const t = require('tap') -const requireInject = require('require-inject') -const fs = require('fs') -const writeJson = requireInject('../lib/write-json.js', { - fs: { - ...fs, - writeFile: (path, data, cb) => cb(null, [path, data]), - }, -}) +const path = require('path') +const writeJson = require('../lib/write-json.js') +const { readFile } = require('fs/promises') const kIndent = Symbol.for('indent') const kNewline = Symbol.for('newline') t.test('write json with newlines and indent set', async t => { - t.same(await writeJson('x', { - [kNewline]: '\r\n', - [kIndent]: 3, - a: 1, - b: [2, 3], - }), [ - 'x', - '{\r\n' + - ' "a": 1,\r\n' + - ' "b": [\r\n' + - ' 2,\r\n' + - ' 3\r\n' + - ' ]\r\n' + - '}\r\n', - ], 'numeric three space indent, CRLF line breaks') - - t.same(await writeJson('x', { - [kNewline]: 'XYZ\n', - [kIndent]: '\t', - a: 1, - b: [2, 3], - }), [ - 'x', - '{XYZ\n' + - '\t"a": 1,XYZ\n' + - '\t"b": [XYZ\n' + - '\t\t2,XYZ\n' + - '\t\t3XYZ\n' + - '\t]XYZ\n' + - '}XYZ\n', - ], 'string tap indent, CRLF line breaks') - - t.same(await writeJson('x', { - a: 1, - b: [2, 3], - }), [ - 'x', - '{\n' + - ' "a": 1,\n' + - ' "b": [\n' + - ' 2,\n' + - ' 3\n' + - ' ]\n' + - '}\n', - ], 'default newline and indent') + t.test('numeric three space indent, CRLF line breaks', async t => { + const dir = t.testdir() + const file = path.join(dir, 'x') + + await writeJson(file, { + [kNewline]: '\r\n', + [kIndent]: 3, + a: 1, + b: [2, 3], + }) + + const str = await readFile(file, 'utf-8') + t.equal(str, `{\r\n "a": 1,\r\n "b": [\r\n 2,\r\n 3\r\n ]\r\n}\r\n`) + }) + + t.test('string tap indent, CRLF line breaks', async t => { + const dir = t.testdir() + const file = path.join(dir, 'x') + + await writeJson(file, { + [kNewline]: 'XYZ\n', + [kIndent]: '\t', + a: 1, + b: [2, 3], + }) + + const str = await readFile(file, 'utf-8') + t.equal(str, `{XYZ\n\t"a": 1,XYZ\n\t"b": [XYZ\n\t\t2,XYZ\n\t\t3XYZ\n\t]XYZ\n}XYZ\n`) + }) + + t.test('default newline and indent', async t => { + const dir = t.testdir() + const file = path.join(dir, 'x') + + await writeJson(file, { + a: 1, + b: [2, 3], + }) + + const str = await readFile(file, 'utf-8') + t.match(str, `{\n "a": 1,\n "b": [\n 2,\n 3\n ]\n}\n`) + }) + + t.end() })