From ad7ab8c19994c1d2a452278edba65968185d3871 Mon Sep 17 00:00:00 2001 From: Gar Date: Thu, 18 Apr 2024 09:59:55 -0700 Subject: [PATCH] fix(perf): lazy loading optimizations (#7388) --- lib/base-command.js | 10 ++++----- lib/cli.js | 2 +- lib/commands/config.js | 10 ++++----- lib/commands/help-search.js | 4 ++-- lib/commands/install.js | 9 +++----- lib/commands/query.js | 2 +- lib/commands/run-script.js | 40 ++++++++++++++++----------------- lib/commands/version.js | 41 +++++++++++++++------------------- lib/npm.js | 4 ++-- lib/package-url-cmd.js | 2 +- lib/utils/error-message.js | 12 +++++----- lib/utils/exit-handler.js | 4 ++-- lib/utils/explain-dep.js | 2 +- lib/utils/is-windows.js | 4 +--- lib/utils/timers.js | 4 ++-- test/lib/utils/exit-handler.js | 12 +++++----- 16 files changed, 73 insertions(+), 89 deletions(-) diff --git a/lib/base-command.js b/lib/base-command.js index a5fdadb60870e..cfaa0bb394460 100644 --- a/lib/base-command.js +++ b/lib/base-command.js @@ -1,9 +1,3 @@ -// Base class for npm commands - -const { relative } = require('path') - -const { definitions } = require('@npmcli/config/lib/definitions') -const { aliases: cmdAliases } = require('./utils/cmd-list') const { log, output } = require('proc-log') class BaseCommand { @@ -18,6 +12,8 @@ class BaseCommand { // this is a static so that we can read from it without instantiating a command // which would require loading the config static get describeUsage () { + const { definitions } = require('@npmcli/config/lib/definitions') + const { aliases: cmdAliases } = require('./utils/cmd-list') const seenExclusive = new Set() const wrapWidth = 80 const { description, usage = [''], name, params } = this @@ -161,6 +157,8 @@ class BaseCommand { } async setWorkspaces () { + const { relative } = require('node:path') + const includeWorkspaceRoot = this.isArboristCmd ? false : this.npm.config.get('include-workspace-root') diff --git a/lib/cli.js b/lib/cli.js index c85ecb65a7005..84d5471ad5e91 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -1,4 +1,4 @@ const validateEngines = require('./es6/validate-engines.js') -const cliEntry = require('path').resolve(__dirname, 'cli-entry.js') +const cliEntry = require('node:path').resolve(__dirname, 'cli-entry.js') module.exports = (process) => validateEngines(process, () => require(cliEntry)) diff --git a/lib/commands/config.js b/lib/commands/config.js index b1273120cf9ee..21faa8838c593 100644 --- a/lib/commands/config.js +++ b/lib/commands/config.js @@ -1,8 +1,7 @@ -const { mkdir, readFile, writeFile } = require('fs/promises') -const { dirname, resolve } = require('path') -const { spawn } = require('child_process') -const { EOL } = require('os') -const ini = require('ini') +const { mkdir, readFile, writeFile } = require('node:fs/promises') +const { dirname, resolve } = require('node:path') +const { spawn } = require('node:child_process') +const { EOL } = require('node:os') const localeCompare = require('@isaacs/string-locale-compare')('en') const pkgJson = require('@npmcli/package-json') const { defaults, definitions } = require('@npmcli/config/lib/definitions') @@ -201,6 +200,7 @@ class Config extends BaseCommand { } async edit () { + const ini = require('ini') const e = this.npm.flatOptions.editor const where = this.npm.flatOptions.location const file = this.npm.config.data.get(where).source diff --git a/lib/commands/help-search.js b/lib/commands/help-search.js index e71ffe4aee288..8821d932525dc 100644 --- a/lib/commands/help-search.js +++ b/lib/commands/help-search.js @@ -1,5 +1,5 @@ -const { readFile } = require('fs/promises') -const path = require('path') +const { readFile } = require('node:fs/promises') +const path = require('node:path') const { glob } = require('glob') const { output } = require('proc-log') const BaseCommand = require('../base-command.js') diff --git a/lib/commands/install.js b/lib/commands/install.js index 13883499c13f0..6573b4d65555d 100644 --- a/lib/commands/install.js +++ b/lib/commands/install.js @@ -1,13 +1,10 @@ -/* eslint-disable camelcase */ -const fs = require('fs') -const util = require('util') -const readdir = util.promisify(fs.readdir) -const reifyFinish = require('../utils/reify-finish.js') +const { readdir } = require('node:fs/promises') +const { resolve, join } = require('node:path') const { log } = require('proc-log') -const { resolve, join } = require('path') const runScript = require('@npmcli/run-script') const pacote = require('pacote') const checks = require('npm-install-checks') +const reifyFinish = require('../utils/reify-finish.js') const ArboristWorkspaceCmd = require('../arborist-cmd.js') class Install extends ArboristWorkspaceCmd { diff --git a/lib/commands/query.js b/lib/commands/query.js index 319d3ee0cc293..a25180d6c38d8 100644 --- a/lib/commands/query.js +++ b/lib/commands/query.js @@ -1,6 +1,6 @@ 'use strict' -const { resolve } = require('path') +const { resolve } = require('node:path') const BaseCommand = require('../base-command.js') const { log, output } = require('proc-log') diff --git a/lib/commands/run-script.js b/lib/commands/run-script.js index d86f9b7326e3b..34c0b00fe95b5 100644 --- a/lib/commands/run-script.js +++ b/lib/commands/run-script.js @@ -1,20 +1,5 @@ -const runScript = require('@npmcli/run-script') -const { isServerPackage } = runScript -const pkgJson = require('@npmcli/package-json') const { log, output } = require('proc-log') -const didYouMean = require('../utils/did-you-mean.js') -const { isWindowsShell } = require('../utils/is-windows.js') - -const cmdList = [ - 'publish', - 'install', - 'uninstall', - 'test', - 'stop', - 'start', - 'restart', - 'version', -].reduce((l, p) => l.concat(['pre' + p, p, 'post' + p]), []) +const pkgJson = require('@npmcli/package-json') const BaseCommand = require('../base-command.js') class RunScript extends BaseCommand { @@ -64,9 +49,7 @@ class RunScript extends BaseCommand { } async run ([event, ...args], { path = this.npm.localPrefix, pkg } = {}) { - // this || undefined is because runScript will be unhappy with the default - // null value - const scriptShell = this.npm.config.get('script-shell') || undefined + const runScript = require('@npmcli/run-script') if (!pkg) { const { content } = await pkgJson.normalize(path) @@ -77,6 +60,7 @@ class RunScript extends BaseCommand { if (event === 'restart' && !scripts.restart) { scripts.restart = 'npm stop --if-present && npm start' } else if (event === 'env' && !scripts.env) { + const { isWindowsShell } = require('../utils/is-windows.js') scripts.env = isWindowsShell ? 'SET' : 'env' } @@ -84,12 +68,13 @@ class RunScript extends BaseCommand { if ( !Object.prototype.hasOwnProperty.call(scripts, event) && - !(event === 'start' && (await isServerPackage(path))) + !(event === 'start' && (await runScript.isServerPackage(path))) ) { if (this.npm.config.get('if-present')) { return } + const didYouMean = require('../utils/did-you-mean.js') const suggestions = await didYouMean(path, event) throw new Error( `Missing script: "${event}"${suggestions}\n\nTo see a list of scripts, run:\n npm run` @@ -111,7 +96,9 @@ class RunScript extends BaseCommand { for (const [ev, evArgs] of events) { await runScript({ path, - scriptShell, + // this || undefined is because runScript will be unhappy with the + // default null value + scriptShell: this.npm.config.get('script-shell') || undefined, stdio: 'inherit', pkg, event: ev, @@ -147,6 +134,17 @@ class RunScript extends BaseCommand { return allScripts } + // TODO this is missing things like prepare, prepublishOnly, and dependencies + const cmdList = [ + 'preinstall', 'install', 'postinstall', + 'prepublish', 'publish', 'postpublish', + 'prerestart', 'restart', 'postrestart', + 'prestart', 'start', 'poststart', + 'prestop', 'stop', 'poststop', + 'pretest', 'test', 'posttest', + 'preuninstall', 'uninstall', 'postuninstall', + 'preversion', 'version', 'postversion', + ] const indent = '\n ' const prefix = ' ' const cmds = [] diff --git a/lib/commands/version.js b/lib/commands/version.js index 9776b70240519..376cd88480d65 100644 --- a/lib/commands/version.js +++ b/lib/commands/version.js @@ -1,10 +1,7 @@ -const libnpmversion = require('libnpmversion') -const { resolve } = require('path') -const { promisify } = require('util') +const { resolve } = require('node:path') +const { readFile } = require('node:fs/promises') const { output } = require('proc-log') -const readFile = promisify(require('fs').readFile) -const updateWorkspaces = require('../workspaces/update-workspaces.js') const BaseCommand = require('../base-command.js') class Version extends BaseCommand { @@ -74,6 +71,7 @@ class Version extends BaseCommand { } async change (args) { + const libnpmversion = require('libnpmversion') const prefix = this.npm.config.get('tag-version-prefix') const version = await libnpmversion(args[0], { ...this.npm.flatOptions, @@ -83,20 +81,33 @@ class Version extends BaseCommand { } async changeWorkspaces (args) { + const updateWorkspaces = require('../workspaces/update-workspaces.js') + const libnpmversion = require('libnpmversion') const prefix = this.npm.config.get('tag-version-prefix') + const { + config, + flatOptions, + localPrefix, + } = this.npm await this.setWorkspaces() const updatedWorkspaces = [] for (const [name, path] of this.workspaces) { output.standard(name) const version = await libnpmversion(args[0], { - ...this.npm.flatOptions, + ...flatOptions, 'git-tag-version': false, path, }) updatedWorkspaces.push(name) output.standard(`${prefix}${version}`) } - return this.update(updatedWorkspaces) + return updateWorkspaces({ + config, + flatOptions, + localPrefix, + npm: this.npm, + workspaces: updatedWorkspaces, + }) } async list (results = {}) { @@ -136,22 +147,6 @@ class Version extends BaseCommand { } return this.list(results) } - - async update (workspaces) { - const { - config, - flatOptions, - localPrefix, - } = this.npm - - await updateWorkspaces({ - config, - flatOptions, - localPrefix, - npm: this.npm, - workspaces, - }) - } } module.exports = Version diff --git a/lib/npm.js b/lib/npm.js index 40be04e8b820b..509c2fa2abd36 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -1,7 +1,7 @@ -const { resolve, dirname, join } = require('path') +const { resolve, dirname, join } = require('node:path') const Config = require('@npmcli/config') const which = require('which') -const fs = require('fs/promises') +const fs = require('node:fs/promises') // Patch the global fs module here at the app level require('graceful-fs').gracefulify(require('fs')) diff --git a/lib/package-url-cmd.js b/lib/package-url-cmd.js index dc6b6f7c0b512..c81b933f69254 100644 --- a/lib/package-url-cmd.js +++ b/lib/package-url-cmd.js @@ -1,7 +1,6 @@ // Base command for opening urls from a package manifest (bugs, docs, repo) const pacote = require('pacote') -const hostedGitInfo = require('hosted-git-info') const openUrl = require('./utils/open-url.js') const { log } = require('proc-log') @@ -52,6 +51,7 @@ class PackageUrlCommand extends BaseCommand { // repository (if a string) or repository.url (if an object) returns null // if it's not a valid repo, or not a known hosted repo hostedFromMani (mani) { + const hostedGitInfo = require('hosted-git-info') const r = mani.repository const rurl = !r ? null : typeof r === 'string' ? r diff --git a/lib/utils/error-message.js b/lib/utils/error-message.js index c27b8a3447957..5d25734f08ad1 100644 --- a/lib/utils/error-message.js +++ b/lib/utils/error-message.js @@ -1,7 +1,6 @@ -const { format } = require('util') -const { resolve } = require('path') +const { format } = require('node:util') +const { resolve } = require('node:path') const { redactLog: replaceInfo } = require('@npmcli/redact') -const { report } = require('./explain-eresolve.js') const { log } = require('proc-log') const messageText = msg => msg.map(line => line.slice(1).join(' ')).join('\n') @@ -32,6 +31,7 @@ const errorMessage = (er, npm) => { switch (er.code) { case 'ERESOLVE': { + const { report } = require('./explain-eresolve.js') short.push(['ERESOLVE', er.message]) detail.push(['', '']) // XXX(display): error messages are logged so we use the logColor since that is based @@ -77,9 +77,7 @@ const errorMessage = (er, npm) => { npm.config.loaded && er.dest.startsWith(npm.config.get('cache')) - const { isWindows } = require('./is-windows.js') - - if (!isWindows && (isCachePath || isCacheDest)) { + if (process.platform !== 'win32' && (isCachePath || isCacheDest)) { // user probably doesn't need this, but still add it to the debug log log.verbose(er.stack) short.push([ @@ -101,7 +99,7 @@ const errorMessage = (er, npm) => { '', [ '\nThe operation was rejected by your operating system.', - isWindows + process.platform === 'win32' /* eslint-disable-next-line max-len */ ? "It's possible that the file was already in use (by a text editor or antivirus),\n" + 'or that you lack permissions to access it.' diff --git a/lib/utils/exit-handler.js b/lib/utils/exit-handler.js index d63f7becc1cb9..76d7d6036e072 100644 --- a/lib/utils/exit-handler.js +++ b/lib/utils/exit-handler.js @@ -1,5 +1,3 @@ -const os = require('os') -const fs = require('fs') const { log, output, time } = require('proc-log') const errorMessage = require('./error-message.js') const { redactLog: replaceInfo } = require('@npmcli/redact') @@ -149,6 +147,8 @@ const exitHandler = err => { log.error('weird error', err) noLogMessage = true } else { + const os = require('node:os') + const fs = require('node:fs') if (!err.code) { const matchErrorCode = err.message.match(/^(?:Error: )?(E[A-Z]+)/) err.code = matchErrorCode && matchErrorCode[1] diff --git a/lib/utils/explain-dep.js b/lib/utils/explain-dep.js index d5d1a40d3403d..efe0ceece0882 100644 --- a/lib/utils/explain-dep.js +++ b/lib/utils/explain-dep.js @@ -1,4 +1,4 @@ -const { relative } = require('path') +const { relative } = require('node:path') const explainNode = (node, depth, chalk) => printNode(node, chalk) + diff --git a/lib/utils/is-windows.js b/lib/utils/is-windows.js index 57f6599b6ae19..63c5671d8400e 100644 --- a/lib/utils/is-windows.js +++ b/lib/utils/is-windows.js @@ -1,6 +1,4 @@ -const isWindows = process.platform === 'win32' -const isWindowsShell = isWindows && +const isWindowsShell = (process.platform === 'win32') && !/^MINGW(32|64)$/.test(process.env.MSYSTEM) && process.env.TERM !== 'cygwin' -exports.isWindows = isWindows exports.isWindowsShell = isWindowsShell diff --git a/lib/utils/timers.js b/lib/utils/timers.js index 2b8da90afbae9..b27ddd588db06 100644 --- a/lib/utils/timers.js +++ b/lib/utils/timers.js @@ -1,5 +1,5 @@ -const EE = require('events') -const fs = require('fs') +const EE = require('node:events') +const fs = require('node:fs') const { log, time } = require('proc-log') const INITIAL_TIMER = 'npm' diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js index b82a8c6ab266a..da2fe3d6ce6ed 100644 --- a/test/lib/utils/exit-handler.js +++ b/test/lib/utils/exit-handler.js @@ -1,8 +1,8 @@ +const fs = require('node:fs') +const { join, resolve } = require('node:path') +const EventEmitter = require('node:events') const t = require('tap') -const fs = require('fs') const fsMiniPass = require('fs-minipass') -const { join, resolve } = require('path') -const EventEmitter = require('events') const { output, time } = require('proc-log') const { load: loadMockNpm } = require('../../fixtures/mock-npm') const mockGlobals = require('@npmcli/mock-globals') @@ -85,7 +85,7 @@ const mockExitHandler = async (t, { config, mocks, files, ...opts } = {}) => { }, }, }), - os: { + 'node:os': { type: () => 'Foo', release: () => '1.0.0', }, @@ -367,7 +367,7 @@ t.test('no logs dir', async (t) => { t.test('timers fail to write', async (t) => { // we want the fs.writeFileSync in the Timers class to fail const mockTimers = tmock(t, '{LIB}/utils/timers.js', { - fs: { + 'node:fs': { ...fs, writeFileSync: (file, ...rest) => { if (file.includes('LOGS_DIR')) { @@ -450,7 +450,7 @@ t.test('files from error message with error', async (t) => { ['error-file.txt', '# error file content'], ], mocks: { - fs: { + 'node:fs': { ...fs, writeFileSync: (dir) => { if (dir.includes('LOGS_DIR') && dir.endsWith('error-file.txt')) {