diff --git a/.yarn/versions/614d282a.yml b/.yarn/versions/614d282a.yml new file mode 100644 index 000000000000..a4379ae0468c --- /dev/null +++ b/.yarn/versions/614d282a.yml @@ -0,0 +1,25 @@ +releases: + "@yarnpkg/cli": patch + "@yarnpkg/nm": major + "@yarnpkg/plugin-nm": patch + "@yarnpkg/pnpify": patch + +declined: + - "@yarnpkg/plugin-compat" + - "@yarnpkg/plugin-constraints" + - "@yarnpkg/plugin-dlx" + - "@yarnpkg/plugin-essentials" + - "@yarnpkg/plugin-init" + - "@yarnpkg/plugin-interactive-tools" + - "@yarnpkg/plugin-npm-cli" + - "@yarnpkg/plugin-pack" + - "@yarnpkg/plugin-patch" + - "@yarnpkg/plugin-pnp" + - "@yarnpkg/plugin-stage" + - "@yarnpkg/plugin-typescript" + - "@yarnpkg/plugin-version" + - "@yarnpkg/plugin-workspace-tools" + - vscode-zipfs + - "@yarnpkg/builder" + - "@yarnpkg/core" + - "@yarnpkg/doctor" diff --git a/CHANGELOG.md b/CHANGELOG.md index 933d44c19cd4..ea7a1ab28fd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ Yarn now accepts sponsorships! Please give a look at our [OpenCollective](https: **Note:** features in `master` can be tried out by running `yarn set version from sources` in your project (existing contrib plugins are updated automatically, while new contrib plugins can be added by running `yarn plugin import from sources `). +### Bugfixes + +- Direct portal dependencies for `node_modules` install are given priority during hoisting now, to prevent cases when indirect regular dependencies take place in the install tree first and block the way for direct portal dependencies. + ### Installs - `hardlinks-global` node modules mode is automatically downgraded to `hardlinks-local` when global cache and install folder are on a different devices and the install continues normally. Warning is produced to the user with mitigation steps provided in documentation. diff --git a/packages/acceptance-tests/pkg-tests-specs/sources/node-modules.test.ts b/packages/acceptance-tests/pkg-tests-specs/sources/node-modules.test.ts index 9c3b8a367681..44674f419418 100644 --- a/packages/acceptance-tests/pkg-tests-specs/sources/node-modules.test.ts +++ b/packages/acceptance-tests/pkg-tests-specs/sources/node-modules.test.ts @@ -1070,6 +1070,33 @@ describe(`Node_Modules`, () => { }) ); + test(`should give a priority to direct portal dependencies over indirect regular dependencies`, + makeTemporaryEnv({}, + { + nodeLinker: `node-modules`, + }, + async ({path, run}) => { + await xfs.mktempPromise(async portalTarget => { + await xfs.writeJsonPromise(`${portalTarget}/package.json` as PortablePath, { + name: `portal`, + dependencies: { + 'no-deps': `2.0.0`, + }, + }); + + await xfs.writeJsonPromise(`${path}/package.json` as PortablePath, { + dependencies: { + portal: `portal:${portalTarget}`, + 'one-fixed-dep': `1.0.0`, + 'one-range-dep': `1.0.0`, + }, + }); + + await expect(run(`install`)).resolves.not.toThrow(); + }); + }) + ); + test( `should not hoist dependencies in nested workspaces when using nmHoistingLimits`, makeTemporaryEnv( diff --git a/packages/yarnpkg-nm/sources/buildNodeModulesTree.ts b/packages/yarnpkg-nm/sources/buildNodeModulesTree.ts index 1595d3affcb9..74cf0541c9fb 100644 --- a/packages/yarnpkg-nm/sources/buildNodeModulesTree.ts +++ b/packages/yarnpkg-nm/sources/buildNodeModulesTree.ts @@ -248,6 +248,8 @@ const buildPackageTree = (pnp: PnpApi, options: NodeModulesTreeOptions): { packa nodes.set(nodeKey, packageTree); } + const isExternalSoftLinkPackage = isExternalSoftLink(pkg, locator); + if (!node) { node = { name, @@ -255,12 +257,13 @@ const buildPackageTree = (pnp: PnpApi, options: NodeModulesTreeOptions): { packa reference: locator.reference, dependencies: new Set(), peerNames: pkg.packagePeers, + hoistPriority: isExternalSoftLinkPackage ? 1 : 0, }; nodes.set(nodeKey, node); } - if (isHoistBorder && !isExternalSoftLink(pkg, locator)) { + if (isHoistBorder && !isExternalSoftLinkPackage) { const parentLocatorKey = stringifyLocator({name: parent.identName, reference: parent.reference}); const dependencyBorders = hoistingLimits.get(parentLocatorKey) || new Set(); hoistingLimits.set(parentLocatorKey, dependencyBorders); diff --git a/packages/yarnpkg-nm/sources/hoist.ts b/packages/yarnpkg-nm/sources/hoist.ts index 1228058c18c6..cc92ed3b9475 100644 --- a/packages/yarnpkg-nm/sources/hoist.ts +++ b/packages/yarnpkg-nm/sources/hoist.ts @@ -46,18 +46,18 @@ * until you run out of candidates for current tree root. */ type PackageName = string; -export type HoisterTree = {name: PackageName, identName: PackageName, reference: string, dependencies: Set, peerNames: Set}; +export type HoisterTree = {name: PackageName, identName: PackageName, reference: string, dependencies: Set, peerNames: Set, hoistPriority?: number}; export type HoisterResult = {name: PackageName, identName: PackageName, references: Set, dependencies: Set}; type Locator = string; type Ident = string; -type HoisterWorkTree = {name: PackageName, references: Set, ident: Ident, locator: Locator, dependencies: Map, originalDependencies: Map, hoistedDependencies: Map, peerNames: ReadonlySet, decoupled: boolean, reasons: Map, isHoistBorder: boolean, hoistedFrom: Array}; +type HoisterWorkTree = {name: PackageName, references: Set, ident: Ident, locator: Locator, dependencies: Map, originalDependencies: Map, hoistedDependencies: Map, peerNames: ReadonlySet, decoupled: boolean, reasons: Map, isHoistBorder: boolean, hoistedFrom: Array, hoistPriority: number}; /** * Mapping which packages depend on a given package alias + ident. It is used to determine hoisting weight, * e.g. which one among the group of packages with the same name should be hoisted. * The package having the biggest number of parents using this package will be hoisted. */ -type PreferenceMap = Map, dependents: Set }>; +type PreferenceMap = Map, dependents: Set, hoistPriority: number }>; enum Hoistable { YES, NO, DEPENDS, @@ -239,7 +239,7 @@ const decoupleGraphNode = (parent: HoisterWorkTree, node: HoisterWorkTree): Hois if (node.decoupled) return node; - const {name, references, ident, locator, dependencies, originalDependencies, hoistedDependencies, peerNames, reasons, isHoistBorder} = node; + const {name, references, ident, locator, dependencies, originalDependencies, hoistedDependencies, peerNames, reasons, isHoistBorder, hoistPriority} = node; // To perform node hoisting from parent node we must clone parent nodes up to the root node, // because some other package in the tree might depend on the parent package where hoisting // cannot be performed @@ -255,6 +255,7 @@ const decoupleGraphNode = (parent: HoisterWorkTree, node: HoisterWorkTree): Hois reasons: new Map(reasons), decoupled: true, isHoistBorder, + hoistPriority, hoistedFrom: [], }; const selfDep = clone.dependencies.get(name); @@ -290,7 +291,9 @@ const getHoistIdentMap = (rootNode: HoisterWorkTree, preferenceMap: PreferenceMa keyList.sort((key1, key2) => { const entry1 = preferenceMap.get(key1)!; const entry2 = preferenceMap.get(key2)!; - if (entry2.peerDependents.size !== entry1.peerDependents.size) { + if (entry2.hoistPriority !== entry1.hoistPriority) { + return entry2.hoistPriority - entry1.hoistPriority; + } else if (entry2.peerDependents.size !== entry1.peerDependents.size) { return entry2.peerDependents.size - entry1.peerDependents.size; } else { return entry2.dependents.size - entry1.dependents.size; @@ -724,6 +727,7 @@ const cloneTree = (tree: HoisterTree, options: InternalHoistOptions): HoisterWor reasons: new Map(), decoupled: true, isHoistBorder: true, + hoistPriority: 0, hoistedFrom: [], }; @@ -733,7 +737,7 @@ const cloneTree = (tree: HoisterTree, options: InternalHoistOptions): HoisterWor let workNode = seenNodes.get(node); const isSeen = !!workNode; if (!workNode) { - const {name, identName, reference, peerNames} = node; + const {name, identName, reference, peerNames, hoistPriority} = node; const dependenciesNmHoistingLimits = options.hoistingLimits.get(parentNode.locator); workNode = { name, @@ -747,6 +751,7 @@ const cloneTree = (tree: HoisterTree, options: InternalHoistOptions): HoisterWor reasons: new Map(), decoupled: true, isHoistBorder: dependenciesNmHoistingLimits ? dependenciesNmHoistingLimits.has(name) : false, + hoistPriority: hoistPriority || 0, hoistedFrom: [], }; seenNodes.set(node, workNode); @@ -854,7 +859,7 @@ const buildPreferenceMap = (rootNode: HoisterWorkTree): PreferenceMap => { const key = getPreferenceKey(node); let entry = preferenceMap.get(key); if (!entry) { - entry = {dependents: new Set(), peerDependents: new Set()}; + entry = {dependents: new Set(), peerDependents: new Set(), hoistPriority: 0}; preferenceMap.set(key, entry); } return entry; @@ -869,8 +874,9 @@ const buildPreferenceMap = (rootNode: HoisterWorkTree): PreferenceMap => { if (!isSeen) { seenNodes.add(node); for (const dep of node.dependencies.values()) { + const entry = getOrCreatePreferenceEntry(dep); + entry.hoistPriority = Math.max(entry.hoistPriority, node.hoistPriority); if (node.peerNames.has(dep.name)) { - const entry = getOrCreatePreferenceEntry(dep); entry.peerDependents.add(node.ident); } else { addDependent(node, dep);