From d08cd4b782b1a783ddbafff59f99d976322288bf Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Wed, 8 May 2024 13:09:56 -0400 Subject: [PATCH 01/18] fix(ci): rm workspace node_modules --- lib/base-cmd.js | 13 +++++++++++++ lib/commands/ci.js | 15 ++++++++++----- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/lib/base-cmd.js b/lib/base-cmd.js index 99ae6d7f43c70..1e0f5c0ccf724 100644 --- a/lib/base-cmd.js +++ b/lib/base-cmd.js @@ -151,6 +151,19 @@ class BaseCommand { this.workspaceNames = [...ws.keys()] this.workspacePaths = [...ws.values()] } + + /** some commands like ci untilize workspace configuration but don't require --workspace flag */ + async setWorkspaceFlagIndependent () { + this.workspace = new Map() + this.workspaceNames = [] + this.workspacePaths = [] + + try { + await this.setWorkspaces({ forgiveFlagError: true }) + } catch { + // noop + } + } } module.exports = BaseCommand diff --git a/lib/commands/ci.js b/lib/commands/ci.js index 7e79d7208c9c4..c7feb57f1c58d 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -33,6 +33,8 @@ class CI extends ArboristWorkspaceCmd { }) } + await this.setWorkspaceFlagIndependent() + const where = this.npm.prefix const Arborist = require('@npmcli/arborist') const opts = { @@ -79,11 +81,14 @@ class CI extends ArboristWorkspaceCmd { // Only remove node_modules after we've successfully loaded the virtual // tree and validated the lockfile await time.start('npm-ci:rm', async () => { - const path = `${where}/node_modules` - // get the list of entries so we can skip the glob for performance - const entries = await fs.readdir(path, null).catch(() => []) - return Promise.all(entries.map(f => fs.rm(`${path}/${f}`, - { force: true, recursive: true }))) + return await Promise.all([where, ...this.workspacePaths].map(async modulePath => { + const path = `${modulePath}/node_modules` + // get the list of entries so we can skip the glob for performance + const entries = await fs.readdir(path, null).catch(() => []) + return Promise.all(entries.map(f => { + return fs.rm(`${path}/${f}`, { force: true, recursive: true }) + })) + })) }) } From 8459bf30ac32d2379cada45ef7465e667f5b0f34 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Wed, 8 May 2024 13:23:22 -0400 Subject: [PATCH 02/18] chore: update setWorkspaceFlagIndependent comment --- lib/base-cmd.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/base-cmd.js b/lib/base-cmd.js index 1e0f5c0ccf724..a448d503c78b5 100644 --- a/lib/base-cmd.js +++ b/lib/base-cmd.js @@ -152,7 +152,7 @@ class BaseCommand { this.workspacePaths = [...ws.values()] } - /** some commands like ci untilize workspace configuration but don't require --workspace flag */ + /** commands like ci use workspace config but not --workspace */ async setWorkspaceFlagIndependent () { this.workspace = new Map() this.workspaceNames = [] From 807c01735381b8f31eff9a16d75badbfbd8d059d Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Mon, 13 May 2024 16:49:08 -0400 Subject: [PATCH 03/18] simplify setting ws --- lib/base-cmd.js | 13 ------------- lib/commands/ci.js | 8 +++++++- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/lib/base-cmd.js b/lib/base-cmd.js index a448d503c78b5..99ae6d7f43c70 100644 --- a/lib/base-cmd.js +++ b/lib/base-cmd.js @@ -151,19 +151,6 @@ class BaseCommand { this.workspaceNames = [...ws.keys()] this.workspacePaths = [...ws.values()] } - - /** commands like ci use workspace config but not --workspace */ - async setWorkspaceFlagIndependent () { - this.workspace = new Map() - this.workspaceNames = [] - this.workspacePaths = [] - - try { - await this.setWorkspaces({ forgiveFlagError: true }) - } catch { - // noop - } - } } module.exports = BaseCommand diff --git a/lib/commands/ci.js b/lib/commands/ci.js index c7feb57f1c58d..f9c6a46c14f38 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -4,6 +4,7 @@ const fs = require('fs/promises') const { log, time } = require('proc-log') const validateLockfile = require('../utils/validate-lockfile.js') const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const getWorkspaces = require('../utils/get-workspaces.js') class CI extends ArboristWorkspaceCmd { static description = 'Clean install a project' @@ -33,7 +34,12 @@ class CI extends ArboristWorkspaceCmd { }) } - await this.setWorkspaceFlagIndependent() + this.workspaces = await getWorkspaces([], { + path: this.npm.localPrefix, + includeWorkspaceRoot: true, + }) + this.workspaceNames = [...this.workspaces.keys()] + this.workspacePaths = [...this.workspaces.values()] const where = this.npm.prefix const Arborist = require('@npmcli/arborist') From 6a72f97e29514370cbbf2d9aa2d999649c04a5c0 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Tue, 14 May 2024 12:34:37 -0400 Subject: [PATCH 04/18] adds test, use set to dedupe --- lib/commands/ci.js | 3 +- test/lib/commands/ci.js | 150 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 149 insertions(+), 4 deletions(-) diff --git a/lib/commands/ci.js b/lib/commands/ci.js index f9c6a46c14f38..eafdefd225ae8 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -87,7 +87,8 @@ class CI extends ArboristWorkspaceCmd { // Only remove node_modules after we've successfully loaded the virtual // tree and validated the lockfile await time.start('npm-ci:rm', async () => { - return await Promise.all([where, ...this.workspacePaths].map(async modulePath => { + const paths = new Set([where, ...this.workspacePaths]) + return await Promise.all([...paths].map(async modulePath => { const path = `${modulePath}/node_modules` // get the list of entries so we can skip the glob for performance const entries = await fs.readdir(path, null).catch(() => []) diff --git a/test/lib/commands/ci.js b/test/lib/commands/ci.js index c4b855932a9ed..b84016463e2a5 100644 --- a/test/lib/commands/ci.js +++ b/test/lib/commands/ci.js @@ -12,6 +12,7 @@ const loadMockNpm = async (t, opts) => { const registry = new MockRegistry({ tap: t, registry: mock.npm.config.get('registry'), + strict: true, }) return { registry, ...mock } } @@ -113,6 +114,11 @@ t.test('reifies, audits, removes node_modules on repeat run', async t => { manifest: manifest.versions['1.0.0'], tarball: path.join(npm.prefix, 'abbrev'), }) + await registry.tarball({ + manifest: manifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'abbrev'), + }) + registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) await npm.exec('ci', []) await npm.exec('ci', []) @@ -142,9 +148,6 @@ t.test('--no-audit and --ignore-scripts', async t => { 'package-lock.json': JSON.stringify(packageLock), }, }) - require('nock').emitter.on('no match', () => { - t.fail('Should not audit') - }) const manifest = registry.manifest({ name: 'abbrev' }) await registry.tarball({ manifest: manifest.versions['1.0.0'], @@ -230,3 +233,144 @@ t.test('should throw error when ideal inventory mismatches virtual', async t => const nmTestFile = path.join(npm.prefix, 'node_modules', 'test-file') t.equal(fs.existsSync(nmTestFile), true, 'does not remove node_modules') }) + +t.test('should remove node_modules within workspaces', async t => { + const { npm, registry } = await loadMockNpm(t, { + prefixDir: { + tarballs: { + oneOneZero: { + 'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.0' }), + 'index.js': 'module.exports = "hello world"', + }, + oneOneOne: { + 'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.1' }), + 'index.js': 'module.exports = "hello world"', + }, + }, + node_modules: { + abbrev: { + 'foo.txt': '', + 'package.json': JSON.stringify({ + name: 'abbrev', + version: '1.1.0', + }), + }, + 'workspace-a': t.fixture('symlink', '../workspace-a'), + 'workspace-b': t.fixture('symlink', '../workspace-b'), + }, + 'package-lock.json': JSON.stringify({ + name: 'workspace-test-3', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { + '': { + name: 'workspace-test-3', + version: '1.0.0', + license: 'ISC', + workspaces: [ + 'workspace-a', + 'workspace-b', + ], + }, + 'node_modules/abbrev': { + version: '1.1.0', + resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.0.tgz', + }, + 'node_modules/workspace-a': { + resolved: 'workspace-a', + link: true, + }, + 'node_modules/workspace-b': { + resolved: 'workspace-b', + link: true, + }, + 'workspace-a': { + version: '1.0.0', + license: 'ISC', + dependencies: { + abbrev: '1.1.0', + }, + }, + 'workspace-b': { + version: '1.0.0', + dependencies: { + abbrev: '1.1.1', + }, + devDependencies: {}, + }, + 'workspace-b/node_modules/abbrev': { + version: '1.1.1', + resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz', + }, + }, + }), + 'package.json': JSON.stringify({ + name: 'workspace-test-3', + version: '1.0.0', + workspaces: [ + 'workspace-a', + 'workspace-b', + ], + }), + 'workspace-a': { + 'package.json': JSON.stringify({ + name: 'workspace-a', + version: '1.0.0', + dependencies: { + abbrev: '1.1.0', + }, + }), + }, + 'workspace-b': { + node_modules: { + abbrev: { + 'bar.txt': '', + 'package.json': JSON.stringify({ + name: 'abbrev', + version: '1.1.1', + }), + }, + }, + 'package.json': JSON.stringify({ + name: 'workspace-b', + version: '1.0.0', + dependencies: { + abbrev: '1.1.1', + }, + }), + }, + }, + }) + + const manifest = registry.manifest({ + name: 'abbrev', + versions: ['1.1.0', '1.1.1'], + }) + + await registry.tarball({ + manifest: manifest.versions['1.1.0'], + tarball: path.join(npm.prefix, 'tarballs/oneOneZero'), + }) + + await registry.tarball({ + manifest: manifest.versions['1.1.1'], + tarball: path.join(npm.prefix, 'tarballs/oneOneOne'), + }) + + registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + + await npm.exec('ci', []) + + const rmFooTest = path.join(npm.prefix, 'node_modules/abbrev/foo.txt') + t.equal(fs.existsSync(rmFooTest), false, 'existing node_modules is removed') + + const rmBarTest = path.join(npm.prefix, 'workspace-b/node_modules/abbrev/bar.txt') + t.equal(fs.existsSync(rmBarTest), false, 'existing ws node_modules is removed') + + const checkNmAbbrev = path.join(npm.prefix, 'node_modules/abbrev') + t.equal(fs.existsSync(checkNmAbbrev), true, 'installs abbrev') + + const checkWsAbbrev = path.join(npm.prefix, 'workspace-b/node_modules/abbrev') + t.equal(fs.existsSync(checkWsAbbrev), true, 'installs abbrev') +}) From dd277456601150fab3eda3b520b26fd043413a1e Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Tue, 14 May 2024 12:41:06 -0400 Subject: [PATCH 05/18] consolidate workspace vars --- lib/commands/ci.js | 14 ++++++-------- test/lib/commands/ci.js | 1 - 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/commands/ci.js b/lib/commands/ci.js index eafdefd225ae8..62974b826809e 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -34,13 +34,6 @@ class CI extends ArboristWorkspaceCmd { }) } - this.workspaces = await getWorkspaces([], { - path: this.npm.localPrefix, - includeWorkspaceRoot: true, - }) - this.workspaceNames = [...this.workspaces.keys()] - this.workspacePaths = [...this.workspaces.values()] - const where = this.npm.prefix const Arborist = require('@npmcli/arborist') const opts = { @@ -82,12 +75,17 @@ class CI extends ArboristWorkspaceCmd { ) } + const workspacePaths = (await getWorkspaces([], { + path: this.npm.localPrefix, + includeWorkspaceRoot: true, + })).values() + const dryRun = this.npm.config.get('dry-run') if (!dryRun) { // Only remove node_modules after we've successfully loaded the virtual // tree and validated the lockfile await time.start('npm-ci:rm', async () => { - const paths = new Set([where, ...this.workspacePaths]) + const paths = new Set([where, ...workspacePaths]) return await Promise.all([...paths].map(async modulePath => { const path = `${modulePath}/node_modules` // get the list of entries so we can skip the glob for performance diff --git a/test/lib/commands/ci.js b/test/lib/commands/ci.js index b84016463e2a5..ca5e0e971e2a4 100644 --- a/test/lib/commands/ci.js +++ b/test/lib/commands/ci.js @@ -267,7 +267,6 @@ t.test('should remove node_modules within workspaces', async t => { '': { name: 'workspace-test-3', version: '1.0.0', - license: 'ISC', workspaces: [ 'workspace-a', 'workspace-b', From 9c088d5540be3d8367463d0985e311b23a534d62 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Tue, 14 May 2024 12:41:51 -0400 Subject: [PATCH 06/18] rm another licence" --- test/lib/commands/ci.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/lib/commands/ci.js b/test/lib/commands/ci.js index ca5e0e971e2a4..672c6b4971773 100644 --- a/test/lib/commands/ci.js +++ b/test/lib/commands/ci.js @@ -286,7 +286,6 @@ t.test('should remove node_modules within workspaces', async t => { }, 'workspace-a': { version: '1.0.0', - license: 'ISC', dependencies: { abbrev: '1.1.0', }, From 0a81f200c2621e1d9c0bfadda537d23e8fe90838 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Tue, 14 May 2024 12:44:55 -0400 Subject: [PATCH 07/18] rm where --- lib/commands/ci.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/commands/ci.js b/lib/commands/ci.js index 62974b826809e..9c10ac2f62a14 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -75,18 +75,17 @@ class CI extends ArboristWorkspaceCmd { ) } - const workspacePaths = (await getWorkspaces([], { + const workspacePaths = await getWorkspaces([], { path: this.npm.localPrefix, includeWorkspaceRoot: true, - })).values() + }) const dryRun = this.npm.config.get('dry-run') if (!dryRun) { // Only remove node_modules after we've successfully loaded the virtual // tree and validated the lockfile await time.start('npm-ci:rm', async () => { - const paths = new Set([where, ...workspacePaths]) - return await Promise.all([...paths].map(async modulePath => { + return await Promise.all(workspacePaths.values().map(async modulePath => { const path = `${modulePath}/node_modules` // get the list of entries so we can skip the glob for performance const entries = await fs.readdir(path, null).catch(() => []) From 142fe7ae96fda0ccc0d2d039144cd827153cf978 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Tue, 14 May 2024 12:50:12 -0400 Subject: [PATCH 08/18] fix spread over map --- lib/commands/ci.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/commands/ci.js b/lib/commands/ci.js index 9c10ac2f62a14..7979d1b58f0bc 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -85,7 +85,7 @@ class CI extends ArboristWorkspaceCmd { // Only remove node_modules after we've successfully loaded the virtual // tree and validated the lockfile await time.start('npm-ci:rm', async () => { - return await Promise.all(workspacePaths.values().map(async modulePath => { + return await Promise.all([...workspacePaths.values()].map(async modulePath => { const path = `${modulePath}/node_modules` // get the list of entries so we can skip the glob for performance const entries = await fs.readdir(path, null).catch(() => []) From d4e23d4354e5c444035542fc17cc44630cfb687a Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Tue, 21 May 2024 12:21:31 -0400 Subject: [PATCH 09/18] added tests, handle workspace flag behavor --- lib/arborist-cmd.js | 4 - lib/arborist-ws-cmd.js | 20 + lib/commands/audit.js | 2 +- lib/commands/ci.js | 19 +- lib/commands/dedupe.js | 2 +- lib/commands/explain.js | 2 +- lib/commands/find-dupes.js | 2 +- lib/commands/fund.js | 2 +- lib/commands/install.js | 2 +- lib/commands/link.js | 2 +- lib/commands/ls.js | 2 +- lib/commands/outdated.js | 2 +- lib/commands/prune.js | 2 +- lib/commands/rebuild.js | 2 +- lib/commands/uninstall.js | 2 +- lib/commands/update.js | 2 +- mock-registry/lib/index.js | 44 ++ tap-snapshots/test/lib/docs.js.test.cjs | 14 +- test/fixtures/mock-npm.js | 393 ++++++++++++++++++ .../{arborist-cmd.js => arborist-ws-cmd.js} | 2 +- test/lib/commands/ci.js | 217 ++++------ test/lib/commands/install.js | 172 ++++++-- 22 files changed, 697 insertions(+), 214 deletions(-) create mode 100644 lib/arborist-ws-cmd.js rename test/lib/{arborist-cmd.js => arborist-ws-cmd.js} (98%) diff --git a/lib/arborist-cmd.js b/lib/arborist-cmd.js index 9d247d02fa181..c5b258efc7197 100644 --- a/lib/arborist-cmd.js +++ b/lib/arborist-cmd.js @@ -10,9 +10,6 @@ class ArboristCmd extends BaseCommand { } static params = [ - 'workspace', - 'workspaces', - 'include-workspace-root', 'install-links', ] @@ -44,7 +41,6 @@ class ArboristCmd extends BaseCommand { } async execWorkspaces (args) { - await this.setWorkspaces() return this.exec(args) } } diff --git a/lib/arborist-ws-cmd.js b/lib/arborist-ws-cmd.js new file mode 100644 index 0000000000000..e5aed300a2b05 --- /dev/null +++ b/lib/arborist-ws-cmd.js @@ -0,0 +1,20 @@ +const ArboristCmd = require('./arborist-cmd.js') + +// This is the base for all commands whose execWorkspaces just gets +// a list of workspace names and passes it on to new Arborist() to +// be able to run a filtered Arborist.reify() at some point. +class ArboristWsCmd extends ArboristCmd { + static params = [ + 'workspace', + 'workspaces', + 'include-workspace-root', + ...super.params, + ] + + async execWorkspaces (args) { + await this.setWorkspaces() + return this.exec(args) + } +} + +module.exports = ArboristWsCmd diff --git a/lib/commands/audit.js b/lib/commands/audit.js index aed1be7a82906..e783d0e6e3d8a 100644 --- a/lib/commands/audit.js +++ b/lib/commands/audit.js @@ -1,5 +1,5 @@ const npmAuditReport = require('npm-audit-report') -const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') const auditError = require('../utils/audit-error.js') const { log, output } = require('proc-log') const reifyFinish = require('../utils/reify-finish.js') diff --git a/lib/commands/ci.js b/lib/commands/ci.js index 7979d1b58f0bc..59ef06f17e467 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -3,13 +3,16 @@ const runScript = require('@npmcli/run-script') const fs = require('fs/promises') const { log, time } = require('proc-log') const validateLockfile = require('../utils/validate-lockfile.js') -const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const ArboristCmd = require('../arborist-cmd.js') const getWorkspaces = require('../utils/get-workspaces.js') -class CI extends ArboristWorkspaceCmd { +class CI extends ArboristCmd { static description = 'Clean install a project' static name = 'ci' + static workspaces = true + static ignoreImplicitWorkspace = true + // These are in the order they will show up in when running "-h" static params = [ 'install-strategy', @@ -34,6 +37,11 @@ class CI extends ArboristWorkspaceCmd { }) } + const workspacePaths = await getWorkspaces([], { + path: this.npm.localPrefix, + includeWorkspaceRoot: true, + }) + const where = this.npm.prefix const Arborist = require('@npmcli/arborist') const opts = { @@ -41,7 +49,7 @@ class CI extends ArboristWorkspaceCmd { packageLock: true, // npm ci should never skip lock files path: where, save: false, // npm ci should never modify the lockfile or package.json - workspaces: this.workspaceNames, + workspaces: [...workspacePaths.keys()], } const arb = new Arborist(opts) @@ -75,11 +83,6 @@ class CI extends ArboristWorkspaceCmd { ) } - const workspacePaths = await getWorkspaces([], { - path: this.npm.localPrefix, - includeWorkspaceRoot: true, - }) - const dryRun = this.npm.config.get('dry-run') if (!dryRun) { // Only remove node_modules after we've successfully loaded the virtual diff --git a/lib/commands/dedupe.js b/lib/commands/dedupe.js index e07bcd31e894b..2c16b26a8aa61 100644 --- a/lib/commands/dedupe.js +++ b/lib/commands/dedupe.js @@ -1,5 +1,5 @@ const reifyFinish = require('../utils/reify-finish.js') -const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') // dedupe duplicated packages, or find them in the tree class Dedupe extends ArboristWorkspaceCmd { diff --git a/lib/commands/explain.js b/lib/commands/explain.js index 2e7d07df729a8..93d0a05eefa4f 100644 --- a/lib/commands/explain.js +++ b/lib/commands/explain.js @@ -4,7 +4,7 @@ const semver = require('semver') const { relative, resolve } = require('path') const validName = require('validate-npm-package-name') const { output } = require('proc-log') -const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') class Explain extends ArboristWorkspaceCmd { static description = 'Explain installed packages' diff --git a/lib/commands/find-dupes.js b/lib/commands/find-dupes.js index 735ac7c4a7ed0..1e411c502478b 100644 --- a/lib/commands/find-dupes.js +++ b/lib/commands/find-dupes.js @@ -1,4 +1,4 @@ -const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') // dedupe duplicated packages, or find them in the tree class FindDupes extends ArboristWorkspaceCmd { diff --git a/lib/commands/fund.js b/lib/commands/fund.js index 8bb4c304b379b..14990f1f6ce8c 100644 --- a/lib/commands/fund.js +++ b/lib/commands/fund.js @@ -6,7 +6,7 @@ const npa = require('npm-package-arg') const { depth } = require('treeverse') const { readTree: getFundingInfo, normalizeFunding, isValidFunding } = require('libnpmfund') const { openUrl } = require('../utils/open-url.js') -const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') const getPrintableName = ({ name, version }) => { const printableVersion = version ? `@${version}` : '' diff --git a/lib/commands/install.js b/lib/commands/install.js index 24e5f6819b314..db42f62b4acdc 100644 --- a/lib/commands/install.js +++ b/lib/commands/install.js @@ -5,7 +5,7 @@ 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') +const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') class Install extends ArboristWorkspaceCmd { static description = 'Install a package' diff --git a/lib/commands/link.js b/lib/commands/link.js index bde761c4226dc..9272c848b6d40 100644 --- a/lib/commands/link.js +++ b/lib/commands/link.js @@ -4,7 +4,7 @@ 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') +const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') class Link extends ArboristWorkspaceCmd { static description = 'Symlink a package folder' diff --git a/lib/commands/ls.js b/lib/commands/ls.js index 51e99f429816a..7f68d2f524fd5 100644 --- a/lib/commands/ls.js +++ b/lib/commands/ls.js @@ -3,7 +3,7 @@ const archy = require('archy') const { breadth } = require('treeverse') const npa = require('npm-package-arg') const { output } = require('proc-log') -const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') const localeCompare = require('@isaacs/string-locale-compare')('en') const relativePrefix = `.${sep}` diff --git a/lib/commands/outdated.js b/lib/commands/outdated.js index 2249808110bbb..9c019689a292b 100644 --- a/lib/commands/outdated.js +++ b/lib/commands/outdated.js @@ -6,7 +6,7 @@ const npa = require('npm-package-arg') const pickManifest = require('npm-pick-manifest') const { output } = require('proc-log') const localeCompare = require('@isaacs/string-locale-compare')('en') -const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') class Outdated extends ArboristWorkspaceCmd { static description = 'Check for outdated packages' diff --git a/lib/commands/prune.js b/lib/commands/prune.js index 1bcf8a9576316..9a63d5b90a957 100644 --- a/lib/commands/prune.js +++ b/lib/commands/prune.js @@ -1,5 +1,5 @@ const reifyFinish = require('../utils/reify-finish.js') -const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') // prune extraneous packages class Prune extends ArboristWorkspaceCmd { diff --git a/lib/commands/rebuild.js b/lib/commands/rebuild.js index 3894f0aa290cc..748c94b57db7f 100644 --- a/lib/commands/rebuild.js +++ b/lib/commands/rebuild.js @@ -2,7 +2,7 @@ const { resolve } = require('path') const { output } = require('proc-log') const npa = require('npm-package-arg') const semver = require('semver') -const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') class Rebuild extends ArboristWorkspaceCmd { static description = 'Rebuild a package' diff --git a/lib/commands/uninstall.js b/lib/commands/uninstall.js index 7496c02deb28f..7b49247db483e 100644 --- a/lib/commands/uninstall.js +++ b/lib/commands/uninstall.js @@ -2,7 +2,7 @@ const { resolve } = require('path') const pkgJson = require('@npmcli/package-json') const reifyFinish = require('../utils/reify-finish.js') const completion = require('../utils/installed-shallow.js') -const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') class Uninstall extends ArboristWorkspaceCmd { static description = 'Remove a package' diff --git a/lib/commands/update.js b/lib/commands/update.js index ddc3e4a47f38a..aa61f5d9f4c87 100644 --- a/lib/commands/update.js +++ b/lib/commands/update.js @@ -1,7 +1,7 @@ const path = require('path') const { log } = require('proc-log') const reifyFinish = require('../utils/reify-finish.js') -const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') class Update extends ArboristWorkspaceCmd { static description = 'Update packages' diff --git a/mock-registry/lib/index.js b/mock-registry/lib/index.js index 03787ea7cd4e6..e431dd01c80c7 100644 --- a/mock-registry/lib/index.js +++ b/mock-registry/lib/index.js @@ -439,6 +439,50 @@ class MockRegistry { ...packument, } } + + /** + * this is a simpler convience method for creating mockable registry with + * tarballs for specific versions + */ + async setup (packages) { + const format = Object.keys(packages).map(v => { + const [name, version] = v.split('@') + return { name, version } + }).reduce((acc, inc) => { + const exists = acc.find(pkg => pkg.name === inc.name) + if (exists) { + exists.tarballs = { + ...exists.tarballs, + [inc.version]: packages[`${inc.name}@${inc.version}`], + } + } else { + acc.push({ name: inc.name, + tarballs: { + [inc.version]: packages[`${inc.name}@${inc.version}`], + }, + }) + } + return acc + }, []) + const registry = this + for (const pkg of format) { + const { name, tarballs } = pkg + const versions = Object.keys(tarballs) + const manifest = await registry.manifest({ name, versions }) + + for (const version of versions) { + const tarballPath = pkg.tarballs[version] + if (!tarballPath) { + throw new Error(`Tarball path not provided for version ${version}`) + } + + await registry.tarball({ + manifest: manifest.versions[version], + tarball: tarballPath, + }) + } + } + } } module.exports = MockRegistry diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs index 85657b9e75dc4..fa5747139eeff 100644 --- a/tap-snapshots/test/lib/docs.js.test.cjs +++ b/tap-snapshots/test/lib/docs.js.test.cjs @@ -2710,9 +2710,7 @@ Options: [--global-style] [--omit [--omit ...]] [--include [--include ...]] [--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] [--no-audit] -[--no-bin-links] [--no-fund] [--dry-run] -[-w|--workspace [-w|--workspace ...]] -[-ws|--workspaces] [--include-workspace-root] [--install-links] +[--no-bin-links] [--no-fund] [--dry-run] [--install-links] aliases: clean-install, ic, install-clean, isntall-clean @@ -2736,9 +2734,6 @@ aliases: clean-install, ic, install-clean, isntall-clean #### \`bin-links\` #### \`fund\` #### \`dry-run\` -#### \`workspace\` -#### \`workspaces\` -#### \`include-workspace-root\` #### \`install-links\` ` @@ -3329,9 +3324,7 @@ Options: [--global-style] [--omit [--omit ...]] [--include [--include ...]] [--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] [--no-audit] -[--no-bin-links] [--no-fund] [--dry-run] -[-w|--workspace [-w|--workspace ...]] -[-ws|--workspaces] [--include-workspace-root] [--install-links] +[--no-bin-links] [--no-fund] [--dry-run] [--install-links] aliases: cit, clean-install-test, sit @@ -3355,9 +3348,6 @@ aliases: cit, clean-install-test, sit #### \`bin-links\` #### \`fund\` #### \`dry-run\` -#### \`workspace\` -#### \`workspaces\` -#### \`include-workspace-root\` #### \`install-links\` ` diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index 674147781af4c..6464c5088ea93 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -1,10 +1,12 @@ const os = require('os') const fs = require('fs').promises +const fsSync = require('fs') const path = require('path') const tap = require('tap') const mockLogs = require('./mock-logs.js') const mockGlobals = require('@npmcli/mock-globals') const tmock = require('./tmock') +const MockRegistry = require('@npmcli/mock-registry') const defExitCode = process.exitCode const changeDir = (dir) => { @@ -288,6 +290,397 @@ const setupMockNpm = async (t, { } } +function workspaceFolderStructureNoHoist (t, opts) { + const { clean } = { cleacleannStart: true, ...opts } + return { + tarballs: { + oneOneZero: { + 'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.0' }), + 'index.js': 'module.exports = "hello world"', + }, + oneOneOne: { + 'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.1' }), + 'index.js': 'module.exports = "hello world"', + }, + }, + node_modules: clean ? {} : { + abbrev: { + 'foo.txt': '', + 'package.json': JSON.stringify({ + name: 'abbrev', + version: '1.1.0', + }), + }, + 'workspace-a': t.fixture('symlink', '../workspace-a'), + 'workspace-b': t.fixture('symlink', '../workspace-b'), + }, + 'package-lock.json': JSON.stringify({ + name: 'workspace-test-3', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { + '': { + name: 'workspace-test-3', + version: '1.0.0', + workspaces: [ + 'workspace-a', + 'workspace-b', + ], + }, + 'node_modules/abbrev': { + version: '1.1.0', + resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.0.tgz', + }, + 'node_modules/workspace-a': { + resolved: 'workspace-a', + link: true, + }, + 'node_modules/workspace-b': { + resolved: 'workspace-b', + link: true, + }, + 'workspace-a': { + version: '1.0.0', + dependencies: { + abbrev: '1.1.0', + }, + }, + 'workspace-b': { + version: '1.0.0', + dependencies: { + abbrev: '1.1.1', + }, + devDependencies: {}, + }, + 'workspace-b/node_modules/abbrev': { + version: '1.1.1', + resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz', + }, + }, + }), + 'package.json': JSON.stringify({ + name: 'workspace-test-3', + version: '1.0.0', + workspaces: [ + 'workspace-a', + 'workspace-b', + ], + }), + 'workspace-a': { + 'package.json': JSON.stringify({ + name: 'workspace-a', + version: '1.0.0', + dependencies: { + abbrev: '1.1.0', + }, + }), + }, + 'workspace-b': { + node_modules: clean ? {} : { + abbrev: { + 'bar.txt': '', + 'package.json': JSON.stringify({ + name: 'abbrev', + version: '1.1.1', + }), + }, + }, + 'package.json': JSON.stringify({ + name: 'workspace-b', + version: '1.0.0', + dependencies: { + abbrev: '1.1.1', + }, + }), + }, + } +} + +function workspaceFolderStructureHoist (t, opts) { + const { clean } = { clean: true, ...opts } + return { + tarballs: { + oneOneZero: { + 'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.0' }), + 'index.js': 'module.exports = "hello world"', + }, + oneOneOne: { + 'package.json': JSON.stringify({ name: 'lodash', version: '1.1.1' }), + 'index.js': 'module.exports = "hello world"', + }, + }, + node_modules: clean ? {} : { + abbrev: { + 'foo.txt': '', + 'package.json': JSON.stringify({ + name: 'abbrev', + version: '1.1.0', + }), + }, + 'workspace-a': t.fixture('symlink', '../workspace-a'), + 'workspace-b': t.fixture('symlink', '../workspace-b'), + }, + 'package-lock.json': JSON.stringify({ + name: 'workspace-test-3', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { + '': { + name: 'workspace-test-3', + version: '1.0.0', + workspaces: [ + 'workspace-a', + 'workspace-b', + ], + }, + 'node_modules/abbrev': { + version: '1.1.0', + resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.0.tgz', + }, + 'node_modules/workspace-a': { + resolved: 'workspace-a', + link: true, + }, + 'node_modules/workspace-b': { + resolved: 'workspace-b', + link: true, + }, + 'workspace-a': { + version: '1.0.0', + dependencies: { + abbrev: '1.1.0', + }, + }, + 'workspace-b': { + version: '1.0.0', + dependencies: { + lodash: '1.1.1', + }, + devDependencies: {}, + }, + 'node_modules/lodash': { + version: '1.1.1', + resolved: 'https://registry.npmjs.org/lodash/-/lodash-1.1.1.tgz', + }, + }, + }), + 'package.json': JSON.stringify({ + name: 'workspace-test-3', + version: '1.0.0', + workspaces: [ + 'workspace-a', + 'workspace-b', + ], + }), + 'workspace-a': { + 'package.json': JSON.stringify({ + name: 'workspace-a', + version: '1.0.0', + dependencies: { + abbrev: '1.1.0', + }, + }), + }, + 'workspace-b': { + node_modules: clean ? {} : { + lodash: { + 'bar.txt': '', + 'package.json': JSON.stringify({ + name: 'lodash', + version: '1.1.1', + }), + }, + }, + 'package.json': JSON.stringify({ + name: 'workspace-b', + version: '1.0.0', + dependencies: { + lodash: '1.1.1', + }, + }), + }, + } +} + +const loadNpmWithRegistry = async (t, opts) => { + const mock = await setupMockNpm(t, opts) + const registry = new MockRegistry({ + tap: t, + registry: mock.npm.config.get('registry'), + strict: true, + }) + + const fileShouldExist = (filePath) => { + t.equal( + fsSync.existsSync(path.join(mock.npm.prefix, filePath)), true, `${filePath} should exist` + ) + } + + const fileShouldNotExist = (filePath) => { + t.equal( + fsSync.existsSync(path.join(mock.npm.prefix, filePath)), false, `${filePath} should not exist` + ) + } + + const packageVersionMatches = (filePath, version) => { + t.equal( + JSON.parse(fsSync.readFileSync(path.join(mock.npm.prefix, filePath), 'utf8')).version, version + ) + } + + const assert = { fileShouldExist, fileShouldNotExist, packageVersionMatches } + + return { registry, assert, ...mock } +} + module.exports = setupMockNpm module.exports.load = setupMockNpm +module.exports.loadNpmWithRegistry = loadNpmWithRegistry module.exports.setGlobalNodeModules = setGlobalNodeModules +module.exports.workspaceFolderStructureHoist = workspaceFolderStructureHoist +module.exports.workspaceFolderStructureNoHoist = workspaceFolderStructureNoHoist +module.exports.workspaceMock = workspaceMock + +function dep (spec, opt) { + const [name, version = '1.0.0'] = spec.split('@') + const lockPath = !opt.hoist && opt.parent ? `${opt.parent}/` : '' + const { definition } = opt + + const depsMap = definition ? Object.entries(definition).map(([s, o]) => { + return dep(s, { ...o, parent: name }) + }) : [] + const dependencies = Object.assign({}, ...depsMap.map(d => d.asDependency)) + + const asPackageJSON = JSON.stringify({ + name, version, ...(Object.keys(dependencies).length ? { dependencies } : {}), + }) + + const asDependency = { + [name]: version, + } + + const asPackageLock = { + [`${lockPath}node_modules/${name}`]: { + version, + resolved: `https://registry.npmjs.org/${name}/-/${name}-${version}.tgz`, + }, + } + const asPackage = { + 'package.json': asPackageJSON, + 'index.js': 'module.exports = "hello world"', + } + + const asTarball = [`${name}@${version}`, asPackage] + + const asDirtyModule = { + [name]: { + [`${name}@${version}.txt`]: '', + 'package.json': asPackageJSON, + }, + } + + const asLockLink = { + [`node_modules/${name}`]: { + resolved: `${name}`, + link: true, + }, + } + + const asDepLock = depsMap.map(d => d.asPackageLock) + const asLocalLockEntry = { [name]: { version, dependencies } } + + const asModule = { + [name]: { + node_modules: Object.assign({}, ...depsMap.map(d => d.hoist ? {} : d.asDirtyModule)), + ...asPackage, + }, + } + + const asLocalizedDirty = lockPath ? { + ...asDirtyModule, + } : {} + + return { + ...opt, + name, + version, + asTarball, + asPackage, + asLocalizedDirty, + asDirtyModule, + asPackageJSON, + asPackageLock, + asDependency, + asModule, + depsMap, + asLockLink, + asDepLock, + asLocalLockEntry, + } +} + +function workspaceMock (t, opts) { + /* eslint-disable max-len */ + const { clean, workspaces } = { clean: true, ...opts } + + const root = 'workspace-root' + const version = '1.0.0' + const names = Object.keys(workspaces) + const ws = Object.entries(workspaces).map(([name, definition]) => dep(name, { definition })) + const deps = ws.map(w => w.depsMap).flat() + const tarballs = Object.fromEntries(deps.map(flatDep => flatDep.asTarball)) + const symlinks = Object.fromEntries(names.map((name) => [name, t.fixture('symlink', `../${name}`)])) + const hoisted = Object.assign({}, ...deps.filter(d => d.hoist).map(d => d.asDirtyModule)) + const workspaceFolders = Object.assign({}, ...ws.map(w => w.asModule)) + const packageJSON = { name: root, version, workspaces: names } + const packageLockJSON = ({ + name: root, + version, + lockfileVersion: 3, + requires: true, + packages: { + '': { name: root, version, workspaces: names }, + ...Object.assign({}, ...ws.map(d => d.asLockLink).flat()), + ...Object.assign({}, ...ws.map(d => d.asDepLock).flat()), + ...Object.assign({}, ...ws.map(d => d.asLocalLockEntry).flat()), + }, + }) + + return { + tarballs, + node_modules: clean ? {} : { + ...hoisted, + ...symlinks, + }, + 'package-lock.json': JSON.stringify(packageLockJSON), + 'package.json': JSON.stringify(packageJSON), + ...Object.fromEntries(Object.entries(workspaceFolders).map(([_key, value]) => { + return [_key, Object.fromEntries(Object.entries(value).map(([key, valueTwo]) => { + if (key === 'node_modules' && clean) { + return [key, {}] + } + return [key, valueTwo] + }))] + })), + } +} + +// const t = require('tap') + +// const v = workspaceMock(t, { +// clean: false, +// workspaces: { +// 'workspace-a': { +// 'abbrev@1.1.0': { hoist: true }, +// }, +// 'workspace-b': { +// 'abbrev@1.1.1': { hoist: false }, +// }, +// }, +// }) + +// // const v = workspaceFolderStructureNoHoist(t, { clean: false }) + +// console.log(JSON.stringify(v, null, 2)) diff --git a/test/lib/arborist-cmd.js b/test/lib/arborist-ws-cmd.js similarity index 98% rename from test/lib/arborist-cmd.js rename to test/lib/arborist-ws-cmd.js index dd90d47b9a000..65b7a01cc20fb 100644 --- a/test/lib/arborist-cmd.js +++ b/test/lib/arborist-ws-cmd.js @@ -4,7 +4,7 @@ const { load: loadMockNpm } = require('../fixtures/mock-npm') const tmock = require('../fixtures/tmock') const mockArboristCmd = async (t, exec, workspace, { mocks = {}, ...opts } = {}) => { - const ArboristCmd = tmock(t, '{LIB}/arborist-cmd.js', mocks) + const ArboristCmd = tmock(t, '{LIB}/arborist-ws-cmd.js', mocks) const config = (typeof workspace === 'function') ? (dirs) => ({ workspace: workspace(dirs) }) diff --git a/test/lib/commands/ci.js b/test/lib/commands/ci.js index 672c6b4971773..8167d98c5b69f 100644 --- a/test/lib/commands/ci.js +++ b/test/lib/commands/ci.js @@ -1,22 +1,14 @@ +const fs = require('fs') +const path = require('path') const t = require('tap') -const { load: _loadMockNpm } = require('../../fixtures/mock-npm') -const MockRegistry = require('@npmcli/mock-registry') -const path = require('path') -const fs = require('fs') +const { + loadNpmWithRegistry: loadMockNpm, + workspaceMock, +} = require('../../fixtures/mock-npm') // t.cleanSnapshot = str => str.replace(/ in [0-9ms]+/g, ' in {TIME}') -const loadMockNpm = async (t, opts) => { - const mock = await _loadMockNpm(t, opts) - const registry = new MockRegistry({ - tap: t, - registry: mock.npm.config.get('registry'), - strict: true, - }) - return { registry, ...mock } -} - const packageJson = { name: 'test-package', version: '1.0.0', @@ -111,6 +103,7 @@ t.test('reifies, audits, removes node_modules on repeat run', async t => { }) const manifest = registry.manifest({ name: 'abbrev' }) await registry.tarball({ + times: 2, manifest: manifest.versions['1.0.0'], tarball: path.join(npm.prefix, 'abbrev'), }) @@ -234,141 +227,89 @@ t.test('should throw error when ideal inventory mismatches virtual', async t => t.equal(fs.existsSync(nmTestFile), true, 'does not remove node_modules') }) -t.test('should remove node_modules within workspaces', async t => { - const { npm, registry } = await loadMockNpm(t, { - prefixDir: { - tarballs: { - oneOneZero: { - 'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.0' }), - 'index.js': 'module.exports = "hello world"', +t.test('should remove dirty node_modules with unhoisted workspace module', async t => { + const { npm, registry, assert } = await loadMockNpm(t, { + prefixDir: workspaceMock(t, { + clean: false, + workspaces: { + 'workspace-a': { + 'abbrev@1.1.0': { hoist: true }, }, - oneOneOne: { - 'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.1' }), - 'index.js': 'module.exports = "hello world"', + 'workspace-b': { + 'abbrev@1.1.1': { hoist: false }, }, }, - node_modules: { - abbrev: { - 'foo.txt': '', - 'package.json': JSON.stringify({ - name: 'abbrev', - version: '1.1.0', - }), + }), + }) + await registry.setup({ + 'abbrev@1.1.0': path.join(npm.prefix, 'tarballs/abbrev@1.1.0'), + 'abbrev@1.1.1': path.join(npm.prefix, 'tarballs/abbrev@1.1.1'), + }) + registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + await npm.exec('ci', []) + assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') + assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') + assert.fileShouldExist('node_modules/abbrev/index.js') + assert.fileShouldNotExist('workspace-b/node_modules/abbrev/abbrev@1.1.1.txt') + assert.packageVersionMatches('workspace-b/node_modules/abbrev/package.json', '1.1.1') + assert.fileShouldExist('workspace-b/node_modules/abbrev/index.js') +}) + +t.test('should remove dirty node_modules with hoisted workspace modules', async t => { + const { npm, registry, assert } = await loadMockNpm(t, { + prefixDir: workspaceMock(t, { + clean: false, + workspaces: { + 'workspace-a': { + 'abbrev@1.1.0': { hoist: true }, }, - 'workspace-a': t.fixture('symlink', '../workspace-a'), - 'workspace-b': t.fixture('symlink', '../workspace-b'), - }, - 'package-lock.json': JSON.stringify({ - name: 'workspace-test-3', - version: '1.0.0', - lockfileVersion: 3, - requires: true, - packages: { - '': { - name: 'workspace-test-3', - version: '1.0.0', - workspaces: [ - 'workspace-a', - 'workspace-b', - ], - }, - 'node_modules/abbrev': { - version: '1.1.0', - resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.0.tgz', - }, - 'node_modules/workspace-a': { - resolved: 'workspace-a', - link: true, - }, - 'node_modules/workspace-b': { - resolved: 'workspace-b', - link: true, - }, - 'workspace-a': { - version: '1.0.0', - dependencies: { - abbrev: '1.1.0', - }, - }, - 'workspace-b': { - version: '1.0.0', - dependencies: { - abbrev: '1.1.1', - }, - devDependencies: {}, - }, - 'workspace-b/node_modules/abbrev': { - version: '1.1.1', - resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz', - }, + 'workspace-b': { + 'lodash@1.1.1': { hoist: true }, }, - }), - 'package.json': JSON.stringify({ - name: 'workspace-test-3', - version: '1.0.0', - workspaces: [ - 'workspace-a', - 'workspace-b', - ], - }), - 'workspace-a': { - 'package.json': JSON.stringify({ - name: 'workspace-a', - version: '1.0.0', - dependencies: { - abbrev: '1.1.0', - }, - }), }, - 'workspace-b': { - node_modules: { - abbrev: { - 'bar.txt': '', - 'package.json': JSON.stringify({ - name: 'abbrev', - version: '1.1.1', - }), - }, - }, - 'package.json': JSON.stringify({ - name: 'workspace-b', - version: '1.0.0', - dependencies: { - abbrev: '1.1.1', - }, - }), - }, - }, + }), }) - - const manifest = registry.manifest({ - name: 'abbrev', - versions: ['1.1.0', '1.1.1'], + await registry.setup({ + 'abbrev@1.1.0': path.join(npm.prefix, 'tarballs/abbrev@1.1.0'), + 'lodash@1.1.1': path.join(npm.prefix, 'tarballs/lodash@1.1.1'), }) + registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + await npm.exec('ci', []) + assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') + assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') + assert.fileShouldExist('node_modules/abbrev/index.js') + assert.fileShouldNotExist('node_modules/lodash/abbrev@1.1.1.txt') + assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') + assert.fileShouldExist('node_modules/lodash/index.js') +}) - await registry.tarball({ - manifest: manifest.versions['1.1.0'], - tarball: path.join(npm.prefix, 'tarballs/oneOneZero'), +t.test('should ignore --workspace flag', async t => { + const { npm, registry, assert } = await loadMockNpm(t, { + config: { + workspace: 'workspace-a', + }, + prefixDir: workspaceMock(t, { + clean: false, + workspaces: { + 'workspace-a': { + 'abbrev@1.1.0': { hoist: true }, + }, + 'workspace-b': { + 'lodash@1.1.1': { hoist: true }, + }, + }, + }), }) - - await registry.tarball({ - manifest: manifest.versions['1.1.1'], - tarball: path.join(npm.prefix, 'tarballs/oneOneOne'), + await registry.setup({ + 'abbrev@1.1.0': path.join(npm.prefix, 'tarballs/abbrev@1.1.0'), + 'lodash@1.1.1': path.join(npm.prefix, 'tarballs/lodash@1.1.1'), }) - registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) - await npm.exec('ci', []) - - const rmFooTest = path.join(npm.prefix, 'node_modules/abbrev/foo.txt') - t.equal(fs.existsSync(rmFooTest), false, 'existing node_modules is removed') - - const rmBarTest = path.join(npm.prefix, 'workspace-b/node_modules/abbrev/bar.txt') - t.equal(fs.existsSync(rmBarTest), false, 'existing ws node_modules is removed') - - const checkNmAbbrev = path.join(npm.prefix, 'node_modules/abbrev') - t.equal(fs.existsSync(checkNmAbbrev), true, 'installs abbrev') - - const checkWsAbbrev = path.join(npm.prefix, 'workspace-b/node_modules/abbrev') - t.equal(fs.existsSync(checkWsAbbrev), true, 'installs abbrev') + assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') + assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') + assert.fileShouldExist('node_modules/abbrev/index.js') + assert.fileShouldNotExist('node_modules/lodash/abbrev@1.1.1.txt') + assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') + assert.fileShouldExist('node_modules/lodash/index.js') }) diff --git a/test/lib/commands/install.js b/test/lib/commands/install.js index 8bb84bfff581f..74adea169f9eb 100644 --- a/test/lib/commands/install.js +++ b/test/lib/commands/install.js @@ -1,9 +1,15 @@ -const t = require('tap') +/* eslint-disable max-len */ const tspawk = require('../../fixtures/tspawk') -const MockRegistry = require('@npmcli/mock-registry') -const { load: loadMockNpm } = require('../../fixtures/mock-npm') -const path = require('node:path') +const path = require('path') +const t = require('tap') + +const { + loadNpmWithRegistry: loadMockNpm, + workspaceMock, +} = require('../../fixtures/mock-npm') + +// tspawk calls preventUnmatched which assures that no scripts run if we don't mock any const spawk = tspawk(t) const abbrev = { @@ -19,11 +25,9 @@ const packageJson = { }, } -// tspawk calls preventUnmatched which assures that no scripts run if we don't mock any - t.test('exec commands', async t => { await t.test('with args does not run lifecycle scripts', async t => { - const { npm } = await loadMockNpm(t, { + const { npm, registry } = await loadMockNpm(t, { config: { audit: false, }, @@ -37,11 +41,6 @@ t.test('exec commands', async t => { abbrev, }, }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), - }) - const manifest = registry.manifest({ name: 'abbrev' }) await registry.package({ manifest }) await registry.tarball({ @@ -70,7 +69,7 @@ t.test('exec commands', async t => { }) scripts[script] = `${script} lifecycle script` } - const { npm } = await loadMockNpm(t, { + const { npm, registry } = await loadMockNpm(t, { config: { audit: false, }, @@ -83,10 +82,6 @@ t.test('exec commands', async t => { }, }) const runOrder = [] - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), - }) const manifest = registry.manifest({ name: 'abbrev' }) await registry.package({ manifest }) await registry.tarball({ @@ -99,7 +94,7 @@ t.test('exec commands', async t => { }) await t.test('should ignore scripts with --ignore-scripts', async t => { - const { npm } = await loadMockNpm(t, { + const { npm, registry } = await loadMockNpm(t, { config: { 'ignore-scripts': true, audit: false, @@ -114,11 +109,6 @@ t.test('exec commands', async t => { abbrev, }, }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), - }) - const manifest = registry.manifest({ name: 'abbrev' }) await registry.package({ manifest }) await registry.tarball({ @@ -143,7 +133,7 @@ t.test('exec commands', async t => { }) await t.test('npm i -g npm engines check success', async t => { - const { npm } = await loadMockNpm(t, { + const { npm, registry } = await loadMockNpm(t, { prefixDir: { npm: { 'package.json': JSON.stringify({ name: 'npm', version: '1.0.0' }), @@ -152,10 +142,6 @@ t.test('exec commands', async t => { }, config: { global: true }, }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), - }) const manifest = registry.manifest({ name: 'npm', packuments: [{ version: '1.0.0', engines: { node: '>1' } }], @@ -170,7 +156,7 @@ t.test('exec commands', async t => { }) await t.test('npm i -g npm engines check failure', async t => { - const { npm } = await loadMockNpm(t, { + const { npm, registry } = await loadMockNpm(t, { prefixDir: { npm: { 'package.json': JSON.stringify({ name: 'npm', version: '1.0.0' }), @@ -179,10 +165,7 @@ t.test('exec commands', async t => { }, config: { global: true }, }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), - }) + const manifest = registry.manifest({ name: 'npm', packuments: [{ version: '1.0.0', engines: { node: '~1' } }], @@ -206,7 +189,7 @@ t.test('exec commands', async t => { }) await t.test('npm i -g npm engines check failure forced override', async t => { - const { npm } = await loadMockNpm(t, { + const { npm, registry } = await loadMockNpm(t, { prefixDir: { npm: { 'package.json': JSON.stringify({ name: 'npm', version: '1.0.0' }), @@ -215,10 +198,6 @@ t.test('exec commands', async t => { }, config: { global: true, force: true }, }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), - }) const manifest = registry.manifest({ name: 'npm', packuments: [{ version: '1.0.0', engines: { node: '~1' } }], @@ -288,3 +267,120 @@ t.test('completion', async t => { t.strictSame(res, []) }) }) + +t.test('should install in workspace with unhoisted module', async t => { + const { npm, registry, assert } = await loadMockNpm(t, { + prefixDir: workspaceMock(t, { + clean: true, + workspaces: { + 'workspace-a': { + 'abbrev@1.1.0': { hoist: true }, + }, + 'workspace-b': { + 'abbrev@1.1.1': { hoist: false }, + }, + }, + }), + }) + await registry.setup({ + 'abbrev@1.1.0': path.join(npm.prefix, 'tarballs/abbrev@1.1.0'), + 'abbrev@1.1.1': path.join(npm.prefix, 'tarballs/abbrev@1.1.1'), + }) + registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + await npm.exec('install', []) + assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') + assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') + assert.fileShouldExist('node_modules/abbrev/index.js') + assert.fileShouldNotExist('workspace-b/node_modules/abbrev/abbrev@1.1.1.txt') + assert.packageVersionMatches('workspace-b/node_modules/abbrev/package.json', '1.1.1') + assert.fileShouldExist('workspace-b/node_modules/abbrev/index.js') +}) + +t.test('should install in workspace with hoisted modules', async t => { + const prefixDir = workspaceMock(t, { + clean: true, + workspaces: { + 'workspace-a': { + 'abbrev@1.1.0': { hoist: true }, + }, + 'workspace-b': { + 'lodash@1.1.1': { hoist: true }, + }, + }, + }) + const { npm, registry, assert } = await loadMockNpm(t, { prefixDir }) + await registry.setup({ + 'abbrev@1.1.0': path.join(npm.prefix, 'tarballs/abbrev@1.1.0'), + 'lodash@1.1.1': path.join(npm.prefix, 'tarballs/lodash@1.1.1'), + }) + registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + await npm.exec('install', []) + assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') + assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') + assert.fileShouldExist('node_modules/abbrev/index.js') + assert.fileShouldNotExist('node_modules/lodash/abbrev@1.1.1.txt') + assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') + assert.fileShouldExist('node_modules/lodash/index.js') +}) + +t.test('should install unhoisted module with --workspace flag', async t => { + const { npm, registry, assert } = await loadMockNpm(t, { + config: { + workspace: 'workspace-b', + }, + prefixDir: workspaceMock(t, { + clean: true, + workspaces: { + 'workspace-a': { + 'abbrev@1.1.0': { hoist: true }, + }, + 'workspace-b': { + 'abbrev@1.1.1': { hoist: false }, + }, + }, + }), + }) + await registry.setup({ + 'abbrev@1.1.1': path.join(npm.prefix, 'tarballs/abbrev@1.1.1'), + }) + registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + await npm.exec('install', []) + assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') + assert.fileShouldNotExist('node_modules/abbrev/package.json') + assert.fileShouldNotExist('node_modules/abbrev/index.js') + + assert.fileShouldNotExist('workspace-b/node_modules/abbrev/abbrev@1.1.1.txt') + assert.packageVersionMatches('workspace-b/node_modules/abbrev/package.json', '1.1.1') + assert.fileShouldExist('workspace-b/node_modules/abbrev/index.js') +}) + +t.test('should install hoisted module with --workspace flag', async t => { + const { npm, registry, assert } = await loadMockNpm(t, { + config: { + workspace: 'workspace-b', + }, + prefixDir: workspaceMock(t, { + clean: true, + workspaces: { + 'workspace-a': { + 'abbrev@1.1.0': { hoist: true }, + }, + 'workspace-b': { + 'lodash@1.1.1': { hoist: true }, + }, + }, + }), + }) + await registry.setup({ + 'lodash@1.1.1': path.join(npm.prefix, 'tarballs/lodash@1.1.1'), + }) + registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + await npm.exec('install', []) + assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') + assert.fileShouldNotExist('node_modules/abbrev/package.json') + assert.fileShouldNotExist('node_modules/abbrev/index.js') + + assert.fileShouldNotExist('node_modules/lodash/abbrev@1.1.1.txt') + assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') + assert.fileShouldExist('node_modules/lodash/index.js') +}) From b7fc789eb0e8a152312f76941c05f9aab20739de Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Tue, 21 May 2024 12:23:57 -0400 Subject: [PATCH 10/18] rm disable max-len --- test/lib/commands/install.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/lib/commands/install.js b/test/lib/commands/install.js index 74adea169f9eb..e0de4d5b685d9 100644 --- a/test/lib/commands/install.js +++ b/test/lib/commands/install.js @@ -1,4 +1,3 @@ -/* eslint-disable max-len */ const tspawk = require('../../fixtures/tspawk') const path = require('path') From afdb6f37dd62eea888d40d8a49b58f8443d7c341 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Tue, 21 May 2024 12:27:18 -0400 Subject: [PATCH 11/18] cleanup fixtures --- test/fixtures/mock-npm.js | 250 ++------------------------------------ 1 file changed, 9 insertions(+), 241 deletions(-) diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index 6464c5088ea93..e72088a078dbc 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -290,220 +290,6 @@ const setupMockNpm = async (t, { } } -function workspaceFolderStructureNoHoist (t, opts) { - const { clean } = { cleacleannStart: true, ...opts } - return { - tarballs: { - oneOneZero: { - 'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.0' }), - 'index.js': 'module.exports = "hello world"', - }, - oneOneOne: { - 'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.1' }), - 'index.js': 'module.exports = "hello world"', - }, - }, - node_modules: clean ? {} : { - abbrev: { - 'foo.txt': '', - 'package.json': JSON.stringify({ - name: 'abbrev', - version: '1.1.0', - }), - }, - 'workspace-a': t.fixture('symlink', '../workspace-a'), - 'workspace-b': t.fixture('symlink', '../workspace-b'), - }, - 'package-lock.json': JSON.stringify({ - name: 'workspace-test-3', - version: '1.0.0', - lockfileVersion: 3, - requires: true, - packages: { - '': { - name: 'workspace-test-3', - version: '1.0.0', - workspaces: [ - 'workspace-a', - 'workspace-b', - ], - }, - 'node_modules/abbrev': { - version: '1.1.0', - resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.0.tgz', - }, - 'node_modules/workspace-a': { - resolved: 'workspace-a', - link: true, - }, - 'node_modules/workspace-b': { - resolved: 'workspace-b', - link: true, - }, - 'workspace-a': { - version: '1.0.0', - dependencies: { - abbrev: '1.1.0', - }, - }, - 'workspace-b': { - version: '1.0.0', - dependencies: { - abbrev: '1.1.1', - }, - devDependencies: {}, - }, - 'workspace-b/node_modules/abbrev': { - version: '1.1.1', - resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz', - }, - }, - }), - 'package.json': JSON.stringify({ - name: 'workspace-test-3', - version: '1.0.0', - workspaces: [ - 'workspace-a', - 'workspace-b', - ], - }), - 'workspace-a': { - 'package.json': JSON.stringify({ - name: 'workspace-a', - version: '1.0.0', - dependencies: { - abbrev: '1.1.0', - }, - }), - }, - 'workspace-b': { - node_modules: clean ? {} : { - abbrev: { - 'bar.txt': '', - 'package.json': JSON.stringify({ - name: 'abbrev', - version: '1.1.1', - }), - }, - }, - 'package.json': JSON.stringify({ - name: 'workspace-b', - version: '1.0.0', - dependencies: { - abbrev: '1.1.1', - }, - }), - }, - } -} - -function workspaceFolderStructureHoist (t, opts) { - const { clean } = { clean: true, ...opts } - return { - tarballs: { - oneOneZero: { - 'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.0' }), - 'index.js': 'module.exports = "hello world"', - }, - oneOneOne: { - 'package.json': JSON.stringify({ name: 'lodash', version: '1.1.1' }), - 'index.js': 'module.exports = "hello world"', - }, - }, - node_modules: clean ? {} : { - abbrev: { - 'foo.txt': '', - 'package.json': JSON.stringify({ - name: 'abbrev', - version: '1.1.0', - }), - }, - 'workspace-a': t.fixture('symlink', '../workspace-a'), - 'workspace-b': t.fixture('symlink', '../workspace-b'), - }, - 'package-lock.json': JSON.stringify({ - name: 'workspace-test-3', - version: '1.0.0', - lockfileVersion: 3, - requires: true, - packages: { - '': { - name: 'workspace-test-3', - version: '1.0.0', - workspaces: [ - 'workspace-a', - 'workspace-b', - ], - }, - 'node_modules/abbrev': { - version: '1.1.0', - resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.0.tgz', - }, - 'node_modules/workspace-a': { - resolved: 'workspace-a', - link: true, - }, - 'node_modules/workspace-b': { - resolved: 'workspace-b', - link: true, - }, - 'workspace-a': { - version: '1.0.0', - dependencies: { - abbrev: '1.1.0', - }, - }, - 'workspace-b': { - version: '1.0.0', - dependencies: { - lodash: '1.1.1', - }, - devDependencies: {}, - }, - 'node_modules/lodash': { - version: '1.1.1', - resolved: 'https://registry.npmjs.org/lodash/-/lodash-1.1.1.tgz', - }, - }, - }), - 'package.json': JSON.stringify({ - name: 'workspace-test-3', - version: '1.0.0', - workspaces: [ - 'workspace-a', - 'workspace-b', - ], - }), - 'workspace-a': { - 'package.json': JSON.stringify({ - name: 'workspace-a', - version: '1.0.0', - dependencies: { - abbrev: '1.1.0', - }, - }), - }, - 'workspace-b': { - node_modules: clean ? {} : { - lodash: { - 'bar.txt': '', - 'package.json': JSON.stringify({ - name: 'lodash', - version: '1.1.1', - }), - }, - }, - 'package.json': JSON.stringify({ - name: 'workspace-b', - version: '1.0.0', - dependencies: { - lodash: '1.1.1', - }, - }), - }, - } -} - const loadNpmWithRegistry = async (t, opts) => { const mock = await setupMockNpm(t, opts) const registry = new MockRegistry({ @@ -535,14 +321,7 @@ const loadNpmWithRegistry = async (t, opts) => { return { registry, assert, ...mock } } -module.exports = setupMockNpm -module.exports.load = setupMockNpm -module.exports.loadNpmWithRegistry = loadNpmWithRegistry -module.exports.setGlobalNodeModules = setGlobalNodeModules -module.exports.workspaceFolderStructureHoist = workspaceFolderStructureHoist -module.exports.workspaceFolderStructureNoHoist = workspaceFolderStructureNoHoist -module.exports.workspaceMock = workspaceMock - +/** breaks down a spec "abbrev@1.1.1" into different parts for mocking */ function dep (spec, opt) { const [name, version = '1.0.0'] = spec.split('@') const lockPath = !opt.hoist && opt.parent ? `${opt.parent}/` : '' @@ -622,7 +401,6 @@ function dep (spec, opt) { } function workspaceMock (t, opts) { - /* eslint-disable max-len */ const { clean, workspaces } = { clean: true, ...opts } const root = 'workspace-root' @@ -631,7 +409,9 @@ function workspaceMock (t, opts) { const ws = Object.entries(workspaces).map(([name, definition]) => dep(name, { definition })) const deps = ws.map(w => w.depsMap).flat() const tarballs = Object.fromEntries(deps.map(flatDep => flatDep.asTarball)) - const symlinks = Object.fromEntries(names.map((name) => [name, t.fixture('symlink', `../${name}`)])) + const symlinks = Object.fromEntries(names.map((name) => { + return [name, t.fixture('symlink', `../${name}`)] + })) const hoisted = Object.assign({}, ...deps.filter(d => d.hoist).map(d => d.asDirtyModule)) const workspaceFolders = Object.assign({}, ...ws.map(w => w.asModule)) const packageJSON = { name: root, version, workspaces: names } @@ -667,20 +447,8 @@ function workspaceMock (t, opts) { } } -// const t = require('tap') - -// const v = workspaceMock(t, { -// clean: false, -// workspaces: { -// 'workspace-a': { -// 'abbrev@1.1.0': { hoist: true }, -// }, -// 'workspace-b': { -// 'abbrev@1.1.1': { hoist: false }, -// }, -// }, -// }) - -// // const v = workspaceFolderStructureNoHoist(t, { clean: false }) - -// console.log(JSON.stringify(v, null, 2)) +module.exports = setupMockNpm +module.exports.load = setupMockNpm +module.exports.loadNpmWithRegistry = loadNpmWithRegistry +module.exports.setGlobalNodeModules = setGlobalNodeModules +module.exports.workspaceMock = workspaceMock From 8657864854d34cf75c18c9a3dbcaba6afbe1ac0d Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Tue, 21 May 2024 14:07:03 -0400 Subject: [PATCH 12/18] fix smoke --- smoke-tests/tap-snapshots/test/index.js.test.cjs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/smoke-tests/tap-snapshots/test/index.js.test.cjs b/smoke-tests/tap-snapshots/test/index.js.test.cjs index 69c9ceab87fd0..98fac1f0de537 100644 --- a/smoke-tests/tap-snapshots/test/index.js.test.cjs +++ b/smoke-tests/tap-snapshots/test/index.js.test.cjs @@ -59,9 +59,7 @@ npm error [--install-strategy ] [--legacy-bundlin npm error [--global-style] [--omit [--omit ...]] npm error [--include [--include ...]] npm error [--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] [--no-audit] -npm error [--no-bin-links] [--no-fund] [--dry-run] -npm error [-w|--workspace [-w|--workspace ...]] -npm error [-ws|--workspaces] [--include-workspace-root] [--install-links] +npm error [--no-bin-links] [--no-fund] [--dry-run] [--install-links] npm error npm error aliases: clean-install, ic, install-clean, isntall-clean npm error From 7e70e01e28c9a87b8196b8df5a9ff5596e4bff4a Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Wed, 22 May 2024 15:33:28 -0400 Subject: [PATCH 13/18] rollback arborist-ws-cmd, keep ws bau, remove all node_modules for ci --- lib/arborist-cmd.js | 4 ++++ lib/arborist-ws-cmd.js | 20 ------------------- lib/commands/audit.js | 2 +- lib/commands/ci.js | 19 ++++++++---------- lib/commands/dedupe.js | 2 +- lib/commands/explain.js | 2 +- lib/commands/find-dupes.js | 2 +- lib/commands/fund.js | 2 +- lib/commands/install.js | 2 +- lib/commands/link.js | 2 +- lib/commands/ls.js | 2 +- lib/commands/outdated.js | 2 +- lib/commands/prune.js | 2 +- lib/commands/rebuild.js | 2 +- lib/commands/uninstall.js | 2 +- lib/commands/update.js | 2 +- .../tap-snapshots/test/index.js.test.cjs | 4 +++- tap-snapshots/test/lib/docs.js.test.cjs | 14 +++++++++++-- test/lib/arborist-ws-cmd.js | 2 +- test/lib/commands/ci.js | 14 ++++++------- test/lib/commands/install.js | 5 ++--- 21 files changed, 50 insertions(+), 58 deletions(-) delete mode 100644 lib/arborist-ws-cmd.js diff --git a/lib/arborist-cmd.js b/lib/arborist-cmd.js index c5b258efc7197..9d247d02fa181 100644 --- a/lib/arborist-cmd.js +++ b/lib/arborist-cmd.js @@ -10,6 +10,9 @@ class ArboristCmd extends BaseCommand { } static params = [ + 'workspace', + 'workspaces', + 'include-workspace-root', 'install-links', ] @@ -41,6 +44,7 @@ class ArboristCmd extends BaseCommand { } async execWorkspaces (args) { + await this.setWorkspaces() return this.exec(args) } } diff --git a/lib/arborist-ws-cmd.js b/lib/arborist-ws-cmd.js deleted file mode 100644 index e5aed300a2b05..0000000000000 --- a/lib/arborist-ws-cmd.js +++ /dev/null @@ -1,20 +0,0 @@ -const ArboristCmd = require('./arborist-cmd.js') - -// This is the base for all commands whose execWorkspaces just gets -// a list of workspace names and passes it on to new Arborist() to -// be able to run a filtered Arborist.reify() at some point. -class ArboristWsCmd extends ArboristCmd { - static params = [ - 'workspace', - 'workspaces', - 'include-workspace-root', - ...super.params, - ] - - async execWorkspaces (args) { - await this.setWorkspaces() - return this.exec(args) - } -} - -module.exports = ArboristWsCmd diff --git a/lib/commands/audit.js b/lib/commands/audit.js index e783d0e6e3d8a..aed1be7a82906 100644 --- a/lib/commands/audit.js +++ b/lib/commands/audit.js @@ -1,5 +1,5 @@ const npmAuditReport = require('npm-audit-report') -const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') const auditError = require('../utils/audit-error.js') const { log, output } = require('proc-log') const reifyFinish = require('../utils/reify-finish.js') diff --git a/lib/commands/ci.js b/lib/commands/ci.js index 59ef06f17e467..80401d6ae2100 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -3,16 +3,13 @@ const runScript = require('@npmcli/run-script') const fs = require('fs/promises') const { log, time } = require('proc-log') const validateLockfile = require('../utils/validate-lockfile.js') -const ArboristCmd = require('../arborist-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') const getWorkspaces = require('../utils/get-workspaces.js') -class CI extends ArboristCmd { +class CI extends ArboristWorkspaceCmd { static description = 'Clean install a project' static name = 'ci' - static workspaces = true - static ignoreImplicitWorkspace = true - // These are in the order they will show up in when running "-h" static params = [ 'install-strategy', @@ -37,11 +34,6 @@ class CI extends ArboristCmd { }) } - const workspacePaths = await getWorkspaces([], { - path: this.npm.localPrefix, - includeWorkspaceRoot: true, - }) - const where = this.npm.prefix const Arborist = require('@npmcli/arborist') const opts = { @@ -49,7 +41,7 @@ class CI extends ArboristCmd { packageLock: true, // npm ci should never skip lock files path: where, save: false, // npm ci should never modify the lockfile or package.json - workspaces: [...workspacePaths.keys()], + workspaces: this.workspaceNames, } const arb = new Arborist(opts) @@ -85,6 +77,11 @@ class CI extends ArboristCmd { const dryRun = this.npm.config.get('dry-run') if (!dryRun) { + const workspacePaths = await getWorkspaces([], { + path: this.npm.localPrefix, + includeWorkspaceRoot: true, + }) + // Only remove node_modules after we've successfully loaded the virtual // tree and validated the lockfile await time.start('npm-ci:rm', async () => { diff --git a/lib/commands/dedupe.js b/lib/commands/dedupe.js index 2c16b26a8aa61..e07bcd31e894b 100644 --- a/lib/commands/dedupe.js +++ b/lib/commands/dedupe.js @@ -1,5 +1,5 @@ const reifyFinish = require('../utils/reify-finish.js') -const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') // dedupe duplicated packages, or find them in the tree class Dedupe extends ArboristWorkspaceCmd { diff --git a/lib/commands/explain.js b/lib/commands/explain.js index 93d0a05eefa4f..2e7d07df729a8 100644 --- a/lib/commands/explain.js +++ b/lib/commands/explain.js @@ -4,7 +4,7 @@ const semver = require('semver') const { relative, resolve } = require('path') const validName = require('validate-npm-package-name') const { output } = require('proc-log') -const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') class Explain extends ArboristWorkspaceCmd { static description = 'Explain installed packages' diff --git a/lib/commands/find-dupes.js b/lib/commands/find-dupes.js index 1e411c502478b..735ac7c4a7ed0 100644 --- a/lib/commands/find-dupes.js +++ b/lib/commands/find-dupes.js @@ -1,4 +1,4 @@ -const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') // dedupe duplicated packages, or find them in the tree class FindDupes extends ArboristWorkspaceCmd { diff --git a/lib/commands/fund.js b/lib/commands/fund.js index 14990f1f6ce8c..8bb4c304b379b 100644 --- a/lib/commands/fund.js +++ b/lib/commands/fund.js @@ -6,7 +6,7 @@ const npa = require('npm-package-arg') const { depth } = require('treeverse') const { readTree: getFundingInfo, normalizeFunding, isValidFunding } = require('libnpmfund') const { openUrl } = require('../utils/open-url.js') -const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') const getPrintableName = ({ name, version }) => { const printableVersion = version ? `@${version}` : '' diff --git a/lib/commands/install.js b/lib/commands/install.js index db42f62b4acdc..24e5f6819b314 100644 --- a/lib/commands/install.js +++ b/lib/commands/install.js @@ -5,7 +5,7 @@ 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-ws-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') class Install extends ArboristWorkspaceCmd { static description = 'Install a package' diff --git a/lib/commands/link.js b/lib/commands/link.js index 9272c848b6d40..bde761c4226dc 100644 --- a/lib/commands/link.js +++ b/lib/commands/link.js @@ -4,7 +4,7 @@ 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-ws-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') class Link extends ArboristWorkspaceCmd { static description = 'Symlink a package folder' diff --git a/lib/commands/ls.js b/lib/commands/ls.js index 7f68d2f524fd5..51e99f429816a 100644 --- a/lib/commands/ls.js +++ b/lib/commands/ls.js @@ -3,7 +3,7 @@ const archy = require('archy') const { breadth } = require('treeverse') const npa = require('npm-package-arg') const { output } = require('proc-log') -const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') const localeCompare = require('@isaacs/string-locale-compare')('en') const relativePrefix = `.${sep}` diff --git a/lib/commands/outdated.js b/lib/commands/outdated.js index 9c019689a292b..2249808110bbb 100644 --- a/lib/commands/outdated.js +++ b/lib/commands/outdated.js @@ -6,7 +6,7 @@ const npa = require('npm-package-arg') const pickManifest = require('npm-pick-manifest') const { output } = require('proc-log') const localeCompare = require('@isaacs/string-locale-compare')('en') -const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') class Outdated extends ArboristWorkspaceCmd { static description = 'Check for outdated packages' diff --git a/lib/commands/prune.js b/lib/commands/prune.js index 9a63d5b90a957..1bcf8a9576316 100644 --- a/lib/commands/prune.js +++ b/lib/commands/prune.js @@ -1,5 +1,5 @@ const reifyFinish = require('../utils/reify-finish.js') -const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') // prune extraneous packages class Prune extends ArboristWorkspaceCmd { diff --git a/lib/commands/rebuild.js b/lib/commands/rebuild.js index 748c94b57db7f..3894f0aa290cc 100644 --- a/lib/commands/rebuild.js +++ b/lib/commands/rebuild.js @@ -2,7 +2,7 @@ const { resolve } = require('path') const { output } = require('proc-log') const npa = require('npm-package-arg') const semver = require('semver') -const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') class Rebuild extends ArboristWorkspaceCmd { static description = 'Rebuild a package' diff --git a/lib/commands/uninstall.js b/lib/commands/uninstall.js index 7b49247db483e..7496c02deb28f 100644 --- a/lib/commands/uninstall.js +++ b/lib/commands/uninstall.js @@ -2,7 +2,7 @@ const { resolve } = require('path') const pkgJson = require('@npmcli/package-json') const reifyFinish = require('../utils/reify-finish.js') const completion = require('../utils/installed-shallow.js') -const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') class Uninstall extends ArboristWorkspaceCmd { static description = 'Remove a package' diff --git a/lib/commands/update.js b/lib/commands/update.js index aa61f5d9f4c87..ddc3e4a47f38a 100644 --- a/lib/commands/update.js +++ b/lib/commands/update.js @@ -1,7 +1,7 @@ const path = require('path') const { log } = require('proc-log') const reifyFinish = require('../utils/reify-finish.js') -const ArboristWorkspaceCmd = require('../arborist-ws-cmd.js') +const ArboristWorkspaceCmd = require('../arborist-cmd.js') class Update extends ArboristWorkspaceCmd { static description = 'Update packages' diff --git a/smoke-tests/tap-snapshots/test/index.js.test.cjs b/smoke-tests/tap-snapshots/test/index.js.test.cjs index 98fac1f0de537..69c9ceab87fd0 100644 --- a/smoke-tests/tap-snapshots/test/index.js.test.cjs +++ b/smoke-tests/tap-snapshots/test/index.js.test.cjs @@ -59,7 +59,9 @@ npm error [--install-strategy ] [--legacy-bundlin npm error [--global-style] [--omit [--omit ...]] npm error [--include [--include ...]] npm error [--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] [--no-audit] -npm error [--no-bin-links] [--no-fund] [--dry-run] [--install-links] +npm error [--no-bin-links] [--no-fund] [--dry-run] +npm error [-w|--workspace [-w|--workspace ...]] +npm error [-ws|--workspaces] [--include-workspace-root] [--install-links] npm error npm error aliases: clean-install, ic, install-clean, isntall-clean npm error diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs index fa5747139eeff..85657b9e75dc4 100644 --- a/tap-snapshots/test/lib/docs.js.test.cjs +++ b/tap-snapshots/test/lib/docs.js.test.cjs @@ -2710,7 +2710,9 @@ Options: [--global-style] [--omit [--omit ...]] [--include [--include ...]] [--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] [--no-audit] -[--no-bin-links] [--no-fund] [--dry-run] [--install-links] +[--no-bin-links] [--no-fund] [--dry-run] +[-w|--workspace [-w|--workspace ...]] +[-ws|--workspaces] [--include-workspace-root] [--install-links] aliases: clean-install, ic, install-clean, isntall-clean @@ -2734,6 +2736,9 @@ aliases: clean-install, ic, install-clean, isntall-clean #### \`bin-links\` #### \`fund\` #### \`dry-run\` +#### \`workspace\` +#### \`workspaces\` +#### \`include-workspace-root\` #### \`install-links\` ` @@ -3324,7 +3329,9 @@ Options: [--global-style] [--omit [--omit ...]] [--include [--include ...]] [--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] [--no-audit] -[--no-bin-links] [--no-fund] [--dry-run] [--install-links] +[--no-bin-links] [--no-fund] [--dry-run] +[-w|--workspace [-w|--workspace ...]] +[-ws|--workspaces] [--include-workspace-root] [--install-links] aliases: cit, clean-install-test, sit @@ -3348,6 +3355,9 @@ aliases: cit, clean-install-test, sit #### \`bin-links\` #### \`fund\` #### \`dry-run\` +#### \`workspace\` +#### \`workspaces\` +#### \`include-workspace-root\` #### \`install-links\` ` diff --git a/test/lib/arborist-ws-cmd.js b/test/lib/arborist-ws-cmd.js index 65b7a01cc20fb..dd90d47b9a000 100644 --- a/test/lib/arborist-ws-cmd.js +++ b/test/lib/arborist-ws-cmd.js @@ -4,7 +4,7 @@ const { load: loadMockNpm } = require('../fixtures/mock-npm') const tmock = require('../fixtures/tmock') const mockArboristCmd = async (t, exec, workspace, { mocks = {}, ...opts } = {}) => { - const ArboristCmd = tmock(t, '{LIB}/arborist-ws-cmd.js', mocks) + const ArboristCmd = tmock(t, '{LIB}/arborist-cmd.js', mocks) const config = (typeof workspace === 'function') ? (dirs) => ({ workspace: workspace(dirs) }) diff --git a/test/lib/commands/ci.js b/test/lib/commands/ci.js index 8167d98c5b69f..993530cee7feb 100644 --- a/test/lib/commands/ci.js +++ b/test/lib/commands/ci.js @@ -278,15 +278,16 @@ t.test('should remove dirty node_modules with hoisted workspace modules', async assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') assert.fileShouldExist('node_modules/abbrev/index.js') - assert.fileShouldNotExist('node_modules/lodash/abbrev@1.1.1.txt') + assert.fileShouldNotExist('node_modules/lodash/lodash@1.1.1.txt') assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') assert.fileShouldExist('node_modules/lodash/index.js') }) -t.test('should ignore --workspace flag', async t => { +/** this behaves the same way as install but will remove all workspace node_modules */ +t.test('should use --workspace flag', async t => { const { npm, registry, assert } = await loadMockNpm(t, { config: { - workspace: 'workspace-a', + workspace: 'workspace-b', }, prefixDir: workspaceMock(t, { clean: false, @@ -301,15 +302,14 @@ t.test('should ignore --workspace flag', async t => { }), }) await registry.setup({ - 'abbrev@1.1.0': path.join(npm.prefix, 'tarballs/abbrev@1.1.0'), 'lodash@1.1.1': path.join(npm.prefix, 'tarballs/lodash@1.1.1'), }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) await npm.exec('ci', []) assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') - assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') - assert.fileShouldExist('node_modules/abbrev/index.js') - assert.fileShouldNotExist('node_modules/lodash/abbrev@1.1.1.txt') + assert.fileShouldNotExist('node_modules/abbrev/package.json') + assert.fileShouldNotExist('node_modules/abbrev/index.js') + assert.fileShouldNotExist('node_modules/lodash/lodash@1.1.1.txt') assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') assert.fileShouldExist('node_modules/lodash/index.js') }) diff --git a/test/lib/commands/install.js b/test/lib/commands/install.js index e0de4d5b685d9..7ae23ba044e66 100644 --- a/test/lib/commands/install.js +++ b/test/lib/commands/install.js @@ -317,7 +317,7 @@ t.test('should install in workspace with hoisted modules', async t => { assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') assert.fileShouldExist('node_modules/abbrev/index.js') - assert.fileShouldNotExist('node_modules/lodash/abbrev@1.1.1.txt') + assert.fileShouldNotExist('node_modules/lodash/lodash@1.1.1.txt') assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') assert.fileShouldExist('node_modules/lodash/index.js') }) @@ -378,8 +378,7 @@ t.test('should install hoisted module with --workspace flag', async t => { assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') assert.fileShouldNotExist('node_modules/abbrev/package.json') assert.fileShouldNotExist('node_modules/abbrev/index.js') - - assert.fileShouldNotExist('node_modules/lodash/abbrev@1.1.1.txt') + assert.fileShouldNotExist('node_modules/lodash/lodash@1.1.1.txt') assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') assert.fileShouldExist('node_modules/lodash/index.js') }) From c60e4f4c5955f5acc47f80d9b1eaf212faad8e83 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Wed, 22 May 2024 15:36:23 -0400 Subject: [PATCH 14/18] comment test --- test/lib/commands/ci.js | 6 ++++++ test/lib/commands/install.js | 9 ++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/test/lib/commands/ci.js b/test/lib/commands/ci.js index 993530cee7feb..918f9a2df4e13 100644 --- a/test/lib/commands/ci.js +++ b/test/lib/commands/ci.js @@ -247,9 +247,11 @@ t.test('should remove dirty node_modules with unhoisted workspace module', async }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) await npm.exec('ci', []) + // for abbrev@1.1.0 assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') assert.fileShouldExist('node_modules/abbrev/index.js') + // for abbrev@1.1.1 assert.fileShouldNotExist('workspace-b/node_modules/abbrev/abbrev@1.1.1.txt') assert.packageVersionMatches('workspace-b/node_modules/abbrev/package.json', '1.1.1') assert.fileShouldExist('workspace-b/node_modules/abbrev/index.js') @@ -275,9 +277,11 @@ t.test('should remove dirty node_modules with hoisted workspace modules', async }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) await npm.exec('ci', []) + // for abbrev@1.1.0 assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') assert.fileShouldExist('node_modules/abbrev/index.js') + // for lodash@1.1.1 assert.fileShouldNotExist('node_modules/lodash/lodash@1.1.1.txt') assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') assert.fileShouldExist('node_modules/lodash/index.js') @@ -306,9 +310,11 @@ t.test('should use --workspace flag', async t => { }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) await npm.exec('ci', []) + // for abbrev@1.1.0 assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') assert.fileShouldNotExist('node_modules/abbrev/package.json') assert.fileShouldNotExist('node_modules/abbrev/index.js') + // for lodash@1.1.1 assert.fileShouldNotExist('node_modules/lodash/lodash@1.1.1.txt') assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') assert.fileShouldExist('node_modules/lodash/index.js') diff --git a/test/lib/commands/install.js b/test/lib/commands/install.js index 7ae23ba044e66..040ec6628667f 100644 --- a/test/lib/commands/install.js +++ b/test/lib/commands/install.js @@ -287,9 +287,11 @@ t.test('should install in workspace with unhoisted module', async t => { }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) await npm.exec('install', []) + // for abbrev@1.1.0 assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') assert.fileShouldExist('node_modules/abbrev/index.js') + // for abbrev@1.1.1 assert.fileShouldNotExist('workspace-b/node_modules/abbrev/abbrev@1.1.1.txt') assert.packageVersionMatches('workspace-b/node_modules/abbrev/package.json', '1.1.1') assert.fileShouldExist('workspace-b/node_modules/abbrev/index.js') @@ -314,9 +316,11 @@ t.test('should install in workspace with hoisted modules', async t => { }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) await npm.exec('install', []) + // for abbrev@1.1.0 assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') assert.fileShouldExist('node_modules/abbrev/index.js') + // for lodash@1.1.1 assert.fileShouldNotExist('node_modules/lodash/lodash@1.1.1.txt') assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') assert.fileShouldExist('node_modules/lodash/index.js') @@ -344,10 +348,11 @@ t.test('should install unhoisted module with --workspace flag', async t => { }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) await npm.exec('install', []) + // for abbrev@1.1.0 assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') assert.fileShouldNotExist('node_modules/abbrev/package.json') assert.fileShouldNotExist('node_modules/abbrev/index.js') - + // for abbrev@1.1.1 assert.fileShouldNotExist('workspace-b/node_modules/abbrev/abbrev@1.1.1.txt') assert.packageVersionMatches('workspace-b/node_modules/abbrev/package.json', '1.1.1') assert.fileShouldExist('workspace-b/node_modules/abbrev/index.js') @@ -375,9 +380,11 @@ t.test('should install hoisted module with --workspace flag', async t => { }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) await npm.exec('install', []) + // for abbrev@1.1.0 assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') assert.fileShouldNotExist('node_modules/abbrev/package.json') assert.fileShouldNotExist('node_modules/abbrev/index.js') + // for lodash@1.1.1 assert.fileShouldNotExist('node_modules/lodash/lodash@1.1.1.txt') assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') assert.fileShouldExist('node_modules/lodash/index.js') From 4cf3f2ab2d7d03116393547b86065b6b992e7713 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Thu, 23 May 2024 10:22:23 -0400 Subject: [PATCH 15/18] add workspace back to ci, add pre command assertions --- lib/commands/ci.js | 9 +- test/fixtures/mock-npm.js | 186 ++++++++++++++++++----------------- test/lib/commands/ci.js | 47 ++++----- test/lib/commands/install.js | 75 ++++++++------ 4 files changed, 160 insertions(+), 157 deletions(-) diff --git a/lib/commands/ci.js b/lib/commands/ci.js index 80401d6ae2100..4432aea408fbd 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -1,6 +1,7 @@ const reifyFinish = require('../utils/reify-finish.js') const runScript = require('@npmcli/run-script') const fs = require('fs/promises') +const path = require('path') const { log, time } = require('proc-log') const validateLockfile = require('../utils/validate-lockfile.js') const ArboristWorkspaceCmd = require('../arborist-cmd.js') @@ -86,11 +87,11 @@ class CI extends ArboristWorkspaceCmd { // tree and validated the lockfile await time.start('npm-ci:rm', async () => { return await Promise.all([...workspacePaths.values()].map(async modulePath => { - const path = `${modulePath}/node_modules` + const fullPath = path.join(modulePath, 'node_modules') // get the list of entries so we can skip the glob for performance - const entries = await fs.readdir(path, null).catch(() => []) - return Promise.all(entries.map(f => { - return fs.rm(`${path}/${f}`, { force: true, recursive: true }) + const entries = await fs.readdir(fullPath, null).catch(() => []) + return Promise.all(entries.map(folder => { + return fs.rm(path.join(fullPath, folder), { force: true, recursive: true }) })) })) }) diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index e72088a078dbc..977d05c7e3509 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -316,104 +316,112 @@ const loadNpmWithRegistry = async (t, opts) => { ) } - const assert = { fileShouldExist, fileShouldNotExist, packageVersionMatches } - - return { registry, assert, ...mock } -} - -/** breaks down a spec "abbrev@1.1.1" into different parts for mocking */ -function dep (spec, opt) { - const [name, version = '1.0.0'] = spec.split('@') - const lockPath = !opt.hoist && opt.parent ? `${opt.parent}/` : '' - const { definition } = opt - - const depsMap = definition ? Object.entries(definition).map(([s, o]) => { - return dep(s, { ...o, parent: name }) - }) : [] - const dependencies = Object.assign({}, ...depsMap.map(d => d.asDependency)) - - const asPackageJSON = JSON.stringify({ - name, version, ...(Object.keys(dependencies).length ? { dependencies } : {}), - }) - - const asDependency = { - [name]: version, + const packageInstalled = (target) => { + const spec = path.basename(target) + const dirname = path.dirname(target) + const [name, version = '1.0.0'] = spec.split('@') + assert.fileShouldNotExist(`${dirname}/${name}/${name}@${version}.txt`) + assert.packageVersionMatches(`${dirname}/${name}/package.json`, version) + assert.fileShouldExist(`${dirname}/${name}/index.js`) } - const asPackageLock = { - [`${lockPath}node_modules/${name}`]: { - version, - resolved: `https://registry.npmjs.org/${name}/-/${name}-${version}.tgz`, - }, + const packageMissing = (target) => { + const spec = path.basename(target) + const dirname = path.dirname(target) + const [name, version = '1.0.0'] = spec.split('@') + assert.fileShouldNotExist(`${dirname}/${name}/${name}@${version}.txt`) + assert.fileShouldNotExist(`${dirname}/${name}/package.json`) + assert.fileShouldNotExist(`${dirname}/${name}/index.js`) } - const asPackage = { - 'package.json': asPackageJSON, - 'index.js': 'module.exports = "hello world"', - } - - const asTarball = [`${name}@${version}`, asPackage] - const asDirtyModule = { - [name]: { - [`${name}@${version}.txt`]: '', - 'package.json': asPackageJSON, - }, + const packageDirty = (target) => { + const spec = path.basename(target) + const dirname = path.dirname(target) + const [name, version = '1.0.0'] = spec.split('@') + assert.fileShouldExist(`${dirname}/${name}/${name}@${version}.txt`) + assert.packageVersionMatches(`${dirname}/${name}/package.json`, version) + assert.fileShouldNotExist(`${dirname}/${name}/index.js`) } - const asLockLink = { - [`node_modules/${name}`]: { - resolved: `${name}`, - link: true, - }, + const assert = { + fileShouldExist, + fileShouldNotExist, + packageVersionMatches, + packageInstalled, + packageMissing, + packageDirty, } - const asDepLock = depsMap.map(d => d.asPackageLock) - const asLocalLockEntry = { [name]: { version, dependencies } } - - const asModule = { - [name]: { - node_modules: Object.assign({}, ...depsMap.map(d => d.hoist ? {} : d.asDirtyModule)), - ...asPackage, - }, - } + return { registry, assert, ...mock } +} - const asLocalizedDirty = lockPath ? { - ...asDirtyModule, - } : {} +/** breaks down a spec "abbrev@1.1.1" into different parts for mocking */ +function dependencyDetails (spec, opt = {}) { + const [name, version = '1.0.0'] = spec.split('@') + const { parent, hoist = true, ws, clean = true } = opt + const modulePathPrefix = !hoist && parent ? `${parent}/` : '' + const modulePath = `${modulePathPrefix}node_modules/${name}` + const resolved = `https://registry.npmjs.org/${name}/-/${name}-${version}.tgz` + // deps + const wsEntries = Object.entries({ ...ws }) + const depsMap = wsEntries.map(([s, o]) => dependencyDetails(s, { ...o, parent: name })) + const dependencies = Object.assign({}, ...depsMap.map(d => d.packageDependency)) + const spreadDependencies = depsMap.length ? { dependencies } : {} + // package and lock objects + const packageDependency = { [name]: version } + const packageLockEntry = { [modulePath]: { version, resolved } } + const packageLockLink = { [modulePath]: { resolved: name, link: true } } + const packageLockLocal = { [name]: { version, dependencies } } + // build package.js + const packageJSON = { name, version, ...spreadDependencies } + const packageJSONString = JSON.stringify(packageJSON) + const packageJSONFile = { 'package.json': packageJSONString } + // build index.js + const indexJSString = 'module.exports = "hello world"' + const indexJSFile = { 'index.js': indexJSString } + // tarball + const packageFiles = { ...packageJSONFile, ...indexJSFile } + const nodeModules = Object.assign({}, ...depsMap.map(d => d.hoist ? {} : d.dirtyOrCleanDir)) + const nodeModulesDir = { node_modules: nodeModules } + const packageDir = { [name]: { ...packageFiles, ...nodeModulesDir } } + const tarballDir = { [`${name}@${version}`]: packageFiles } + // dirty files + const dirtyFile = { [`${name}@${version}.txt`]: 'dirty file' } + const dirtyFiles = { ...packageJSONFile, ...dirtyFile } + const dirtyDir = { [name]: dirtyFiles } + const dirtyOrCleanDir = clean ? {} : dirtyDir return { - ...opt, - name, - version, - asTarball, - asPackage, - asLocalizedDirty, - asDirtyModule, - asPackageJSON, - asPackageLock, - asDependency, - asModule, + packageDependency, + hoist, depsMap, - asLockLink, - asDepLock, - asLocalLockEntry, + dirtyOrCleanDir, + tarballDir, + packageDir, + packageLockEntry, + packageLockLink, + packageLockLocal, } } function workspaceMock (t, opts) { - const { clean, workspaces } = { clean: true, ...opts } - + const toObject = [(a, c) => ({ ...a, ...c }), {}] + const { workspaces: workspacesDef, ...rest } = { clean: true, ...opts } + const workspaces = Object.fromEntries(Object.entries(workspacesDef).map(([name, ws]) => { + return [name, Object.fromEntries(Object.entries(ws).map(([wsPackageDep, wsPackageDepOpts]) => { + return [wsPackageDep, { ...rest, ...wsPackageDepOpts }] + }))] + })) const root = 'workspace-root' const version = '1.0.0' const names = Object.keys(workspaces) - const ws = Object.entries(workspaces).map(([name, definition]) => dep(name, { definition })) - const deps = ws.map(w => w.depsMap).flat() - const tarballs = Object.fromEntries(deps.map(flatDep => flatDep.asTarball)) - const symlinks = Object.fromEntries(names.map((name) => { - return [name, t.fixture('symlink', `../${name}`)] - })) - const hoisted = Object.assign({}, ...deps.filter(d => d.hoist).map(d => d.asDirtyModule)) - const workspaceFolders = Object.assign({}, ...ws.map(w => w.asModule)) + const ws = Object.entries(workspaces).map(([name, _ws]) => dependencyDetails(name, { ws: _ws })) + const deps = ws.map(({ depsMap }) => depsMap).flat() + const tarballs = deps.map(w => w.tarballDir).reduce(...toObject) + const symlinks = names + .map((name) => ({ [name]: t.fixture('symlink', `../${name}`) })).reduce(...toObject) + const hoisted = deps.filter(d => d.hoist).map(w => w.dirtyOrCleanDir).reduce(...toObject) + const workspaceFolders = ws.map(w => w.packageDir).reduce(...toObject) const packageJSON = { name: root, version, workspaces: names } const packageLockJSON = ({ name: root, @@ -422,28 +430,22 @@ function workspaceMock (t, opts) { requires: true, packages: { '': { name: root, version, workspaces: names }, - ...Object.assign({}, ...ws.map(d => d.asLockLink).flat()), - ...Object.assign({}, ...ws.map(d => d.asDepLock).flat()), - ...Object.assign({}, ...ws.map(d => d.asLocalLockEntry).flat()), + ...deps.filter(d => d.hoist).map(d => d.packageLockEntry).reduce(...toObject), + ...ws.map(d => d.packageLockEntry).flat().reduce(...toObject), + ...ws.map(d => d.packageLockLink).flat().reduce(...toObject), + ...ws.map(d => d.packageLockLocal).flat().reduce(...toObject), + ...deps.filter(d => !d.hoist).map(d => d.packageLockEntry).reduce(...toObject), }, }) - return { tarballs, - node_modules: clean ? {} : { + node_modules: { ...hoisted, ...symlinks, }, 'package-lock.json': JSON.stringify(packageLockJSON), 'package.json': JSON.stringify(packageJSON), - ...Object.fromEntries(Object.entries(workspaceFolders).map(([_key, value]) => { - return [_key, Object.fromEntries(Object.entries(value).map(([key, valueTwo]) => { - if (key === 'node_modules' && clean) { - return [key, {}] - } - return [key, valueTwo] - }))] - })), + ...workspaceFolders, } } diff --git a/test/lib/commands/ci.js b/test/lib/commands/ci.js index 918f9a2df4e13..bcca4876fce79 100644 --- a/test/lib/commands/ci.js +++ b/test/lib/commands/ci.js @@ -233,7 +233,7 @@ t.test('should remove dirty node_modules with unhoisted workspace module', async clean: false, workspaces: { 'workspace-a': { - 'abbrev@1.1.0': { hoist: true }, + 'abbrev@1.1.0': { }, }, 'workspace-b': { 'abbrev@1.1.1': { hoist: false }, @@ -246,15 +246,11 @@ t.test('should remove dirty node_modules with unhoisted workspace module', async 'abbrev@1.1.1': path.join(npm.prefix, 'tarballs/abbrev@1.1.1'), }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + assert.packageDirty('node_modules/abbrev@1.1.0') + assert.packageDirty('workspace-b/node_modules/abbrev@1.1.1') await npm.exec('ci', []) - // for abbrev@1.1.0 - assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') - assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') - assert.fileShouldExist('node_modules/abbrev/index.js') - // for abbrev@1.1.1 - assert.fileShouldNotExist('workspace-b/node_modules/abbrev/abbrev@1.1.1.txt') - assert.packageVersionMatches('workspace-b/node_modules/abbrev/package.json', '1.1.1') - assert.fileShouldExist('workspace-b/node_modules/abbrev/index.js') + assert.packageInstalled('node_modules/abbrev@1.1.0') + assert.packageInstalled('workspace-b/node_modules/abbrev@1.1.1') }) t.test('should remove dirty node_modules with hoisted workspace modules', async t => { @@ -263,10 +259,10 @@ t.test('should remove dirty node_modules with hoisted workspace modules', async clean: false, workspaces: { 'workspace-a': { - 'abbrev@1.1.0': { hoist: true }, + 'abbrev@1.1.0': { }, }, 'workspace-b': { - 'lodash@1.1.1': { hoist: true }, + 'lodash@1.1.1': { }, }, }, }), @@ -276,19 +272,16 @@ t.test('should remove dirty node_modules with hoisted workspace modules', async 'lodash@1.1.1': path.join(npm.prefix, 'tarballs/lodash@1.1.1'), }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + assert.packageDirty('node_modules/abbrev@1.1.0') + assert.packageDirty('node_modules/lodash@1.1.1') await npm.exec('ci', []) - // for abbrev@1.1.0 - assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') - assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') - assert.fileShouldExist('node_modules/abbrev/index.js') - // for lodash@1.1.1 - assert.fileShouldNotExist('node_modules/lodash/lodash@1.1.1.txt') - assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') - assert.fileShouldExist('node_modules/lodash/index.js') + assert.packageInstalled('node_modules/abbrev@1.1.0') + assert.packageInstalled('node_modules/lodash@1.1.1') }) /** this behaves the same way as install but will remove all workspace node_modules */ t.test('should use --workspace flag', async t => { + t.saveFixture = true const { npm, registry, assert } = await loadMockNpm(t, { config: { workspace: 'workspace-b', @@ -297,10 +290,10 @@ t.test('should use --workspace flag', async t => { clean: false, workspaces: { 'workspace-a': { - 'abbrev@1.1.0': { hoist: true }, + 'abbrev@1.1.0': { }, }, 'workspace-b': { - 'lodash@1.1.1': { hoist: true }, + 'lodash@1.1.1': { }, }, }, }), @@ -309,13 +302,9 @@ t.test('should use --workspace flag', async t => { 'lodash@1.1.1': path.join(npm.prefix, 'tarballs/lodash@1.1.1'), }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + assert.packageDirty('node_modules/abbrev@1.1.0') + assert.packageDirty('node_modules/lodash@1.1.1') await npm.exec('ci', []) - // for abbrev@1.1.0 - assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') - assert.fileShouldNotExist('node_modules/abbrev/package.json') - assert.fileShouldNotExist('node_modules/abbrev/index.js') - // for lodash@1.1.1 - assert.fileShouldNotExist('node_modules/lodash/lodash@1.1.1.txt') - assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') - assert.fileShouldExist('node_modules/lodash/index.js') + assert.packageMissing('node_modules/abbrev@1.1.0') + assert.packageInstalled('node_modules/lodash@1.1.1') }) diff --git a/test/lib/commands/install.js b/test/lib/commands/install.js index 040ec6628667f..4fa234df99bea 100644 --- a/test/lib/commands/install.js +++ b/test/lib/commands/install.js @@ -286,15 +286,11 @@ t.test('should install in workspace with unhoisted module', async t => { 'abbrev@1.1.1': path.join(npm.prefix, 'tarballs/abbrev@1.1.1'), }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + assert.packageMissing('node_modules/abbrev@1.1.0') + assert.packageMissing('workspace-b/node_modules/abbrev@1.1.1') await npm.exec('install', []) - // for abbrev@1.1.0 - assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') - assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') - assert.fileShouldExist('node_modules/abbrev/index.js') - // for abbrev@1.1.1 - assert.fileShouldNotExist('workspace-b/node_modules/abbrev/abbrev@1.1.1.txt') - assert.packageVersionMatches('workspace-b/node_modules/abbrev/package.json', '1.1.1') - assert.fileShouldExist('workspace-b/node_modules/abbrev/index.js') + assert.packageInstalled('node_modules/abbrev@1.1.0') + assert.packageInstalled('workspace-b/node_modules/abbrev@1.1.1') }) t.test('should install in workspace with hoisted modules', async t => { @@ -315,15 +311,11 @@ t.test('should install in workspace with hoisted modules', async t => { 'lodash@1.1.1': path.join(npm.prefix, 'tarballs/lodash@1.1.1'), }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + assert.packageMissing('node_modules/abbrev@1.1.0') + assert.packageMissing('node_modules/lodash@1.1.1') await npm.exec('install', []) - // for abbrev@1.1.0 - assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') - assert.packageVersionMatches('node_modules/abbrev/package.json', '1.1.0') - assert.fileShouldExist('node_modules/abbrev/index.js') - // for lodash@1.1.1 - assert.fileShouldNotExist('node_modules/lodash/lodash@1.1.1.txt') - assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') - assert.fileShouldExist('node_modules/lodash/index.js') + assert.packageInstalled('node_modules/abbrev@1.1.0') + assert.packageInstalled('node_modules/lodash@1.1.1') }) t.test('should install unhoisted module with --workspace flag', async t => { @@ -347,15 +339,11 @@ t.test('should install unhoisted module with --workspace flag', async t => { 'abbrev@1.1.1': path.join(npm.prefix, 'tarballs/abbrev@1.1.1'), }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + assert.packageMissing('node_modules/abbrev@1.1.0') + assert.packageMissing('workspace-b/node_modules/abbrev@1.1.1') await npm.exec('install', []) - // for abbrev@1.1.0 - assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') - assert.fileShouldNotExist('node_modules/abbrev/package.json') - assert.fileShouldNotExist('node_modules/abbrev/index.js') - // for abbrev@1.1.1 - assert.fileShouldNotExist('workspace-b/node_modules/abbrev/abbrev@1.1.1.txt') - assert.packageVersionMatches('workspace-b/node_modules/abbrev/package.json', '1.1.1') - assert.fileShouldExist('workspace-b/node_modules/abbrev/index.js') + assert.packageMissing('node_modules/abbrev@1.1.0') + assert.packageInstalled('workspace-b/node_modules/abbrev@1.1.1') }) t.test('should install hoisted module with --workspace flag', async t => { @@ -379,13 +367,36 @@ t.test('should install hoisted module with --workspace flag', async t => { 'lodash@1.1.1': path.join(npm.prefix, 'tarballs/lodash@1.1.1'), }) registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + assert.packageMissing('node_modules/abbrev@1.1.0') + assert.packageMissing('node_modules/lodash@1.1.1') await npm.exec('install', []) - // for abbrev@1.1.0 - assert.fileShouldNotExist('node_modules/abbrev/abbrev@1.1.0.txt') - assert.fileShouldNotExist('node_modules/abbrev/package.json') - assert.fileShouldNotExist('node_modules/abbrev/index.js') - // for lodash@1.1.1 - assert.fileShouldNotExist('node_modules/lodash/lodash@1.1.1.txt') - assert.packageVersionMatches('node_modules/lodash/package.json', '1.1.1') - assert.fileShouldExist('node_modules/lodash/index.js') + assert.packageMissing('node_modules/abbrev@1.1.0') + assert.packageInstalled('node_modules/lodash@1.1.1') +}) + +t.test('should show install keeps dirty --workspace flag', async t => { + const { npm, registry, assert } = await loadMockNpm(t, { + config: { + workspace: 'workspace-b', + }, + prefixDir: workspaceMock(t, { + workspaces: { + 'workspace-a': { + 'abbrev@1.1.0': { clean: false, hoist: true }, + }, + 'workspace-b': { + 'lodash@1.1.1': { clean: true, hoist: true }, + }, + }, + }), + }) + await registry.setup({ + 'lodash@1.1.1': path.join(npm.prefix, 'tarballs/lodash@1.1.1'), + }) + registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {}) + assert.packageDirty('node_modules/abbrev@1.1.0') + assert.packageMissing('node_modules/lodash@1.1.1') + await npm.exec('install', []) + assert.packageDirty('node_modules/abbrev@1.1.0') + assert.packageInstalled('node_modules/lodash@1.1.1') }) From 37bb0f2b69bcbd8a10fd3b697690f294082be316 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Thu, 23 May 2024 10:25:41 -0400 Subject: [PATCH 16/18] undo arboist-cmd test rename --- test/lib/{arborist-ws-cmd.js => arborist-cmd.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/lib/{arborist-ws-cmd.js => arborist-cmd.js} (100%) diff --git a/test/lib/arborist-ws-cmd.js b/test/lib/arborist-cmd.js similarity index 100% rename from test/lib/arborist-ws-cmd.js rename to test/lib/arborist-cmd.js From e63c442581c298aaafc1737f8efaca093eee1895 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Thu, 23 May 2024 10:26:39 -0400 Subject: [PATCH 17/18] add new export to the bottom --- test/fixtures/mock-npm.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index 977d05c7e3509..debefd84ae7b1 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -451,6 +451,6 @@ function workspaceMock (t, opts) { module.exports = setupMockNpm module.exports.load = setupMockNpm -module.exports.loadNpmWithRegistry = loadNpmWithRegistry module.exports.setGlobalNodeModules = setGlobalNodeModules +module.exports.loadNpmWithRegistry = loadNpmWithRegistry module.exports.workspaceMock = workspaceMock From d23bd853883a0405e72255c21163c99e6d6e7995 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Thu, 23 May 2024 10:59:58 -0400 Subject: [PATCH 18/18] rm local assert call --- test/fixtures/mock-npm.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index debefd84ae7b1..e1003e8da068a 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -320,27 +320,27 @@ const loadNpmWithRegistry = async (t, opts) => { const spec = path.basename(target) const dirname = path.dirname(target) const [name, version = '1.0.0'] = spec.split('@') - assert.fileShouldNotExist(`${dirname}/${name}/${name}@${version}.txt`) - assert.packageVersionMatches(`${dirname}/${name}/package.json`, version) - assert.fileShouldExist(`${dirname}/${name}/index.js`) + fileShouldNotExist(`${dirname}/${name}/${name}@${version}.txt`) + packageVersionMatches(`${dirname}/${name}/package.json`, version) + fileShouldExist(`${dirname}/${name}/index.js`) } const packageMissing = (target) => { const spec = path.basename(target) const dirname = path.dirname(target) const [name, version = '1.0.0'] = spec.split('@') - assert.fileShouldNotExist(`${dirname}/${name}/${name}@${version}.txt`) - assert.fileShouldNotExist(`${dirname}/${name}/package.json`) - assert.fileShouldNotExist(`${dirname}/${name}/index.js`) + fileShouldNotExist(`${dirname}/${name}/${name}@${version}.txt`) + fileShouldNotExist(`${dirname}/${name}/package.json`) + fileShouldNotExist(`${dirname}/${name}/index.js`) } const packageDirty = (target) => { const spec = path.basename(target) const dirname = path.dirname(target) const [name, version = '1.0.0'] = spec.split('@') - assert.fileShouldExist(`${dirname}/${name}/${name}@${version}.txt`) - assert.packageVersionMatches(`${dirname}/${name}/package.json`, version) - assert.fileShouldNotExist(`${dirname}/${name}/index.js`) + fileShouldExist(`${dirname}/${name}/${name}@${version}.txt`) + packageVersionMatches(`${dirname}/${name}/package.json`, version) + fileShouldNotExist(`${dirname}/${name}/index.js`) } const assert = {