Skip to content

Commit

Permalink
fix(perf): lazy loading optimizations (#7388)
Browse files Browse the repository at this point in the history
  • Loading branch information
wraithgar authored Apr 18, 2024
1 parent 8eae4b3 commit ad7ab8c
Show file tree
Hide file tree
Showing 16 changed files with 73 additions and 89 deletions.
10 changes: 4 additions & 6 deletions lib/base-command.js
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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')
Expand Down
2 changes: 1 addition & 1 deletion lib/cli.js
Original file line number Diff line number Diff line change
@@ -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))
10 changes: 5 additions & 5 deletions lib/commands/config.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/commands/help-search.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down
9 changes: 3 additions & 6 deletions lib/commands/install.js
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/query.js
Original file line number Diff line number Diff line change
@@ -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')

Expand Down
40 changes: 19 additions & 21 deletions lib/commands/run-script.js
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -77,19 +60,21 @@ 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'
}

pkg.scripts = scripts

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`
Expand All @@ -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,
Expand Down Expand Up @@ -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 = []
Expand Down
41 changes: 18 additions & 23 deletions lib/commands/version.js
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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 = {}) {
Expand Down Expand Up @@ -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
4 changes: 2 additions & 2 deletions lib/npm.js
Original file line number Diff line number Diff line change
@@ -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'))
Expand Down
2 changes: 1 addition & 1 deletion lib/package-url-cmd.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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
Expand Down
12 changes: 5 additions & 7 deletions lib/utils/error-message.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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([
Expand All @@ -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.'
Expand Down
4 changes: 2 additions & 2 deletions lib/utils/exit-handler.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion lib/utils/explain-dep.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { relative } = require('path')
const { relative } = require('node:path')

const explainNode = (node, depth, chalk) =>
printNode(node, chalk) +
Expand Down
4 changes: 1 addition & 3 deletions lib/utils/is-windows.js
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions lib/utils/timers.js
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
12 changes: 6 additions & 6 deletions test/lib/utils/exit-handler.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -85,7 +85,7 @@ const mockExitHandler = async (t, { config, mocks, files, ...opts } = {}) => {
},
},
}),
os: {
'node:os': {
type: () => 'Foo',
release: () => '1.0.0',
},
Expand Down Expand Up @@ -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')) {
Expand Down Expand Up @@ -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')) {
Expand Down

0 comments on commit ad7ab8c

Please sign in to comment.