Skip to content

Commit

Permalink
feat: add --install-strategy config, replace --global-style with
Browse files Browse the repository at this point in the history
"shallow" style

BREAKING CHANGE: remove --global-style, --global now sets
--install-strategy=shallow
  • Loading branch information
fritzy committed Oct 18, 2022
1 parent 32336f6 commit 6d82835
Show file tree
Hide file tree
Showing 13 changed files with 48 additions and 38 deletions.
2 changes: 1 addition & 1 deletion lib/commands/dedupe.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class Dedupe extends ArboristWorkspaceCmd {
static description = 'Reduce duplication in the package tree'
static name = 'dedupe'
static params = [
'global-style',
'install-strategy',
'legacy-bundling',
'strict-peer-deps',
'package-lock',
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/find-dupes.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class FindDupes extends ArboristWorkspaceCmd {
static description = 'Find duplication in the package tree'
static name = 'find-dupes'
static params = [
'global-style',
'install-strategy',
'legacy-bundling',
'strict-peer-deps',
'package-lock',
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class Install extends ArboristWorkspaceCmd {
'save',
'save-exact',
'global',
'global-style',
'install-strategy',
'legacy-bundling',
'omit',
'strict-peer-deps',
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Link extends ArboristWorkspaceCmd {
'save',
'save-exact',
'global',
'global-style',
'install-strategy',
'legacy-bundling',
'strict-peer-deps',
'package-lock',
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/update.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class Update extends ArboristWorkspaceCmd {
static params = [
'save',
'global',
'global-style',
'install-strategy',
'legacy-bundling',
'omit',
'strict-peer-deps',
Expand Down
33 changes: 17 additions & 16 deletions lib/utils/config/definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -823,20 +823,6 @@ define('global', {
},
})

define('global-style', {
default: false,
type: Boolean,
description: `
Causes npm to install the package into your local \`node_modules\` folder
with the same layout it uses with the global \`node_modules\` folder.
Only your direct dependencies will show in \`node_modules\` and
everything they depend on will be flattened in their \`node_modules\`
folders. This obviously will eliminate some deduping. If used with
\`legacy-bundling\`, \`legacy-bundling\` will be preferred.
`,
flatten,
})

// the globalconfig has its default defined outside of this module
define('globalconfig', {
type: path,
Expand Down Expand Up @@ -1076,6 +1062,21 @@ define('install-links', {
flatten,
})

define('install-strategy', {
default: 'hoisted',
type: ['hoisted', 'nested', 'shallow'],
description: `
Sets the strategy for installing packages in node_modules.
hoisted (default): Install non-duplicated in top-level, and duplicated as
necessary within directory structure.
nested: (formerly --legacy-bundling) install in place, no hoisting.
shallow (formerly --global-style) only install direct deps at top-level.
linked: (coming soon) install in node_modules/.store, link in place,
unhoisted.
`,
flatten,
})

define('json', {
default: false,
type: Boolean,
Expand Down Expand Up @@ -1523,8 +1524,8 @@ define('prefix', {
short: 'C',
default: '',
defaultDescription: `
In global mode, the folder where the node executable is installed. In
local mode, the nearest parent folder containing either a package.json
In global mode, the folder where the node executable is installed.
Otherwise, the nearest parent folder containing either a package.json
file or a node_modules folder.
`,
description: `
Expand Down
8 changes: 4 additions & 4 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const _loadFailures = Symbol('loadFailures')
const _pruneFailedOptional = Symbol('pruneFailedOptional')
const _linkNodes = Symbol('linkNodes')
const _follow = Symbol('follow')
const _globalStyle = Symbol('globalStyle')
const _installStrategy = Symbol('installStrategy')
const _globalRootNode = Symbol('globalRootNode')
const _usePackageLock = Symbol.for('usePackageLock')
const _rpcache = Symbol.for('realpathCache')
Expand Down Expand Up @@ -114,7 +114,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
follow = false,
force = false,
global = false,
globalStyle = false,
installStrategy = 'hoisted',
idealTree = null,
includeWorkspaceRoot = false,
installLinks = false,
Expand All @@ -134,7 +134,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {

this[_usePackageLock] = packageLock
this[_global] = !!global
this[_globalStyle] = this[_global] || globalStyle
this[_installStrategy] = global ? 'shallow' : installStrategy
this[_follow] = !!follow

if (this[_workspaces].length && this[_global]) {
Expand Down Expand Up @@ -956,7 +956,7 @@ This is a one-time fix-up, please be patient...
strictPeerDeps: this[_strictPeerDeps],
installLinks: this.installLinks,
legacyPeerDeps: this.legacyPeerDeps,
globalStyle: this[_globalStyle],
installStrategy: this[_installStrategy],
}))

const promises = []
Expand Down
1 change: 1 addition & 0 deletions workspaces/arborist/lib/arborist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class Arborist extends Base {
workspacesEnabled: options.workspacesEnabled !== false,
replaceRegistryHost: options.replaceRegistryHost,
lockfileVersion: lockfileVersion(options.lockfileVersion),
installStrategy: options.global ? 'shallow' : (options.installStrategy ? options.installStrategy : 'hoisted'),
}
this.replaceRegistryHost = this.options.replaceRegistryHost =
(!this.options.replaceRegistryHost || this.options.replaceRegistryHost === 'npmjs') ?
Expand Down
8 changes: 4 additions & 4 deletions workspaces/arborist/lib/place-dep.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class PlaceDep {
strictPeerDeps,
installLinks,
legacyPeerDeps,
globalStyle,
installStrategy,
} = parent || options
Object.assign(this, {
preferDedupe,
Expand All @@ -58,8 +58,8 @@ class PlaceDep {
legacyBundling,
strictPeerDeps,
installLinks,
installStrategy,
legacyPeerDeps,
globalStyle,
})

this.children = []
Expand All @@ -78,10 +78,10 @@ class PlaceDep {
edge,
dep,
preferDedupe,
globalStyle,
legacyBundling,
explicitRequest,
updateNames,
installStrategy,
checks,
} = this

Expand Down Expand Up @@ -176,7 +176,7 @@ class PlaceDep {

// when installing globally, or just in global style, we never place
// deps above the first level.
if (globalStyle) {
if (installStrategy === 'shallow') {
const rp = target.resolveParent
if (rp && rp.isProjectRoot) {
break
Expand Down
6 changes: 3 additions & 3 deletions workspaces/arborist/test/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ t.test('link dep within node_modules and outside root', t => {
})

t.test('global style', t => t.resolveMatchSnapshot(printIdeal(t.testdir(), {
globalStyle: true,
installStrategy: 'shallow',
add: ['rimraf'],
})))

Expand Down Expand Up @@ -1136,7 +1136,7 @@ t.test('resolve links in global mode', async t => {

t.test('dont get confused if root matches duped metadep', async t => {
const path = resolve(fixtures, 'test-root-matches-metadep')
const arb = new Arborist({ path, ...OPT })
const arb = new Arborist({ path, installStrategy: 'hoisted', ...OPT })
const tree = await arb.buildIdealTree()
t.matchSnapshot(printTree(tree))
})
Expand Down Expand Up @@ -2154,7 +2154,7 @@ t.test('properly assign fsParent when paths have .. in them', async t => {
}
})

t.test('update global', async t => {
t.only('update global', async t => {
// global root
// ├─┬ @isaacs/testing-dev-optional-flags@1.0.0
// │ ├── own-or@1.0.0
Expand Down
10 changes: 9 additions & 1 deletion workspaces/arborist/test/arborist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ t.test('excludeSet includes nonworkspace metadeps', async t => {
spec: 'file:pkgs/b',
})

const arb = new Arborist()
const arb = new Arborist({})
const filter = arb.excludeWorkspacesDependencySet(tree)

t.equal(filter.size, 3)
Expand Down Expand Up @@ -245,3 +245,11 @@ t.test('valid replaceRegistryHost values', t => {
t.equal(new Arborist({ replaceRegistryHost: 'never' }).options.replaceRegistryHost, 'never')
t.end()
})

t.test('valid global/installStrategy values', t => {
t.equal(new Arborist({ global: true }).options.installStrategy, 'shallow')
t.equal(new Arborist({ global: false }).options.installStrategy, 'hoisted')
t.equal(new Arborist({}).options.installStrategy, 'hoisted')
t.equal(new Arborist({ installStrategy: 'hoisted' }).options.installStrategy, 'hoisted')
t.end()
})
2 changes: 1 addition & 1 deletion workspaces/arborist/test/arborist/reify.js
Original file line number Diff line number Diff line change
Expand Up @@ -1116,7 +1116,7 @@ t.test('global style', t => {
const rbinPart = '.bin/rimraf' +
(process.platform === 'win32' ? '.cmd' : '')
const rbin = resolve(nm, rbinPart)
return reify(path, { add: ['rimraf@2'], globalStyle: true })
return reify(path, { add: ['rimraf@2'], installStrategy: 'shallow' })
.then(() => fs.statSync(rbin))
.then(() => t.strictSame(fs.readdirSync(nm).sort(), ['.bin', '.package-lock.json', 'rimraf']))
})
Expand Down
8 changes: 4 additions & 4 deletions workspaces/arborist/test/place-dep.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ t.test('placement tests', t => {
// --legacy-peer-deps set?
legacyPeerDeps = false,
// installing with --global or --global-style?
globalStyle = false,
installStrategy = 'hoisted',
} = options

const node = tree.inventory.get(nodeLoc)
Expand Down Expand Up @@ -83,7 +83,7 @@ t.test('placement tests', t => {
legacyBundling,
strictPeerDeps,
legacyPeerDeps,
globalStyle,
installStrategy,
})
}

Expand Down Expand Up @@ -291,7 +291,7 @@ t.test('placement tests', t => {
}),
dep: new Node({ pkg: { name: 'bar', version: '1.0.0' } }),
nodeLoc: 'node_modules/foo',
globalStyle: true,
installStrategy: 'shallow',
test: (t, tree) => {
const foobar = tree.children.get('foo').resolve('bar')
t.equal(foobar.location, 'node_modules/foo/node_modules/bar')
Expand Down Expand Up @@ -323,7 +323,7 @@ t.test('placement tests', t => {
}),
dep: new Node({ pkg: { name: 'baz', version: '1.0.0' } }),
nodeLoc: 'node_modules/foo/node_modules/bar',
globalStyle: true,
installStrategy: 'shallow',
test: (t, tree) => {
const foobar = tree.children.get('foo').resolve('bar')
const foobarbaz = foobar.resolve('baz')
Expand Down

0 comments on commit 6d82835

Please sign in to comment.