Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add --install-strategy=hoisted|nested|shallow, remove --global-style, --legacy-bundling #5709

Merged
merged 3 commits into from
Oct 18, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
feat: add --install-strategy config, replace --global-style with
"shallow" style

BREAKING CHANGE: remove --global-style, --global now sets
--install-strategy=shallow
  • Loading branch information
fritzy committed Oct 18, 2022
commit 6d82835b311bee861ef8e428f21885eb04d99938
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/[email protected]
// │ ├── [email protected]
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