Skip to content

Commit 1ef45ae

Browse files
clemyanclemyan
authored
Fix deduplication of virtual packages installed under aliases (#6735)
## What's the problem this PR addresses? Fixes #6573 In order to explain *why* the bug happens, let's do a refresher on virtual packages and deduplication, and *how* we end up here ### Virtuals Let's say we have the following packages: - `pkg-z` has a peer dependency on `pkg-p@*` - `pkg-a` has a dependency on `pkg-z@^1.0.0` and `pkg-p@^1.0.0` - `pkg-b` has a dependency on `pkg-z@^1.0.0` and `pkg-p@^2.0.0` ``` pkg-a --[ pkg-z@^1.0.0 ]--> pkg-z@npm:1.0.0 ==> pkg-p@* --[ pkg-p@^1.0.0 ]--> pkg-p@npm:1.0.0 pkg-b --[ pkg-z@^1.0.0 ]--> pkg-z@npm:1.0.0 ==> pkg-p@* --[ pkg-p@^2.0.0 ]--> pkg-p@npm:2.0.0 ``` When we resolve the peer dependencies, we resolve which instance of `pkg-p` we each `pkg-z` instance should get, then we add that instance of `pkg-p` as a regular dependency to the `pkg-z` instance in our internal package data ``` pkg-a --[ pkg-z@^1.0.0 ]--> pkg-z@npm:1.0.0 --[...]--> pkg-p@npm:1.0.0 pkg-b --[ pkg-z@^1.0.0 ]--> pkg-z@npm:1.0.0 --[...]--> pkg-p@npm:2.0.0 ``` This is a problem because we end up with the same package having two different sets of dependencies. To avoid that, we create **virtual packages** for each instance of peer requesters (i.e. packages with peer dependencies) in the tree. We also replace descriptors that resolve to those packages with **virtual descriptors**. ``` pkg-a --[ pkg-z@virtual:<id-a> ]--> pkg-z@virtual:<id-a> --[...]--> pkg-p@npm:1.0.0 pkg-b --[ pkg-z@virtual:<id-b> ]--> pkg-z@virtual:<id-b> --[...]--> pkg-p@npm:2.0.0 ``` The two instance of `pkg-z` that have different dependency sets are now literally two different packages in our internals. Note that the virtual id is based on the parent package and the pre-virtualization package. ### Deduplication However, just virtualizing every peer requester naively can lead to some problems. Take this example: ``` pkg-a --[ pkg-z@^1.0.0 ]--> pkg-z@npm:1.0.0 ==> pkg-p@* --[ pkg-p@^1.0.0 ]--> pkg-p@npm:1.0.0 pkg-b --[ pkg-z@^1.0.0 ]--> pkg-z@npm:1.0.0 ==> pkg-p@* --[ pkg-p@^1.0.0 ]--> pkg-p@npm:1.0.0 ``` ``` pkg-a --[ pkg-z@virtual:<id-a> ]--> pkg-z@virtual:<id-a> --[...]-> pkg-p@npm:1.0.0 pkg-b --[ pkg-z@virtual:<id-b> ]--> pkg-z@virtual:<id-b> --[...]-> pkg-p@npm:1.0.0 ``` We have the reverse "problem" -- we end up with two different virtual `pkg-z` with the same dependency sets. That's not incorrect *per se*, but it'd be nice if we can use the same package instance for both. That's why we also have a deduplication process where we find virtual packages that are the virtualizations of the same package and have the same dependency sets, and replace all of their virtual descriptors and packages with a single "master" descriptor/package among them ``` pkg-a --[ pkg-z@virtual:<id-a> ]--> pkg-z@virtual:<id-a> --[...]-> pkg-p@npm:1.0.0 pkg-b --[ pkg-z@virtual:<id-a> ]--> pkg-z@virtual:<id-a> --[...]-> pkg-p@npm:1.0.0 ``` Now we go back to having one `pkg-z` instance, and we can remove the discarded virtual descriptor/package (`pkg-z@virtual:<id-b>`) from our internals. ### Bug with aliases This deduplication, however, in turn leads to a bug (#1352) in some edge cases. (In this and subsequent examples, I'll omit `pkg-p`. Assume that all instances of `pkg-z` has their peer requests satisfied the same way) ```json { "name": "pkg-a", "dependencies": { "pkg-x": "npm:pkg-z@^1.0.0", "pkg-y": "npm:pkg-z@^1.0.0" } } ``` ``` pkg-a --[ pkg-x@npm:pkg-z@^1.0.0 ]--> pkg-z@npm:1.0.0 --[ pkg-y@npm:pkg-z@^1.0.0 ]--> pkg-z@npm:1.0.0 ``` ``` pkg-a --[ pkg-x@virtual:<id-a> ]--> pkg-z@virtual:<id-a> --[ pkg-y@virtual:<id-a> ]--> pkg-z@virtual:<id-a> ``` We have used aliases to make two different virtual descriptors resolve to the same virtual package. When deduping, we will dedupe one of the virtual descriptors to the other descriptor, and **discard the package it resolves to**. Now we end up with zero instances of `pkg-z`... #1726 fixes that, by having the deduplication key include the descriptor ident (`pkg-x` vs `pkg-y`), so those two are not deduped against each other. This is where we are currently at. ### More edge cases !!! This fix, however, once again causes more edge cases. In one edge case, it will cause virtual packages to not dedupe when they should ``` pkg-a --[ pkg-x@npm:pkg-z@^1.0.0 ]--> pkg-z@npm:1.0.0 pkg-b --[ pkg-y@npm:pkg-z@^1.0.0 ]--> pkg-z@npm:1.0.0 ``` ``` pkg-a --[ pkg-x@virtual:<id-a> ]--> pkg-z@virtual:<id-a> pkg-b --[ pkg-y@virtual:<id-b> ]--> pkg-z@virtual:<id-b> ``` #1726 fixes specifically for two aliased dependencies of the same package. What if we have two packages each using an alias? We don't dedupe those because the virtual descriptors have different idents, and end up with two virtual instances because the parent package is part of the virtual id calculation ---- That case also not a problem *per se*, just inefficient. However, using that we can construct a situation where the same package can either dedupe or not dedupe depending on which descriptor is being checked. ``` pkg-a --[ pkg-x@npm:pkg-z@^1.0.0 ]--> pkg-z@npm:1.0.0 pkg-b --[ pkg-x@npm:pkg-z@^1.0.0 ]--> pkg-z@npm:1.0.0 --[ pkg-y@npm:pkg-z@^1.0.0 ]--> pkg-z@npm:1.0.0 ``` ``` pkg-a --[ pkg-x@virtual:<id-a> ]--> pkg-z@virtual:<id-a> pkg-b --[ pkg-x@virtual:<id-b> ]--> pkg-z@virtual:<id-b> --[ pkg-y@virtual:<id-b> ]--> pkg-z@virtual:<id-b> ``` Here, the second descriptor `pkg-x@virtual:<id-b>` will be deduped into the first: they have the same ident and resolve to the same "physical" package. Once again we will **discard `pkg-z@virtual:<id-b>` in the process**. However, the third descriptor `pkg-x@virtual:<id-b>` is not deduped, leaving a dangling reference to the discarded package. This is the case reported in #6573 ## How did you fix it? At its root, #1352 is caused by the same virtual package being checked for dedupe twice against two diffent virtual descriptors that resolve to it. What if we just... not do that? **Instead of deduping virtual descriptors, just dedupe the virtual packages.** We would have virtual descriptors resolving to virtual packages with a different virtual id, but that would not cause problems. In fact, the entire point of resolution is to link descriptors and locators. This ends up simplifying the implementation a lot because we don't need to keep track of which packages have which virtual descriptors as dependencies. We just need a reverse resolution map from virtual locators to virtual descriptors. Deduping is as simple as updating the project resolution map. The only downside is we have a few more descriptors in the internals, but that's just a very tiny cost compared to the benefits. ## Checklist <!--- Don't worry if you miss something, chores are automatically tested. --> <!--- This checklist exists to help you remember doing the chores when you submit a PR. --> <!--- Put an `x` in all the boxes that apply. --> - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing). <!-- See https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released for more details. --> <!-- Check with `yarn version check` and fix with `yarn version check -i` --> - [x] I have set the packages that need to be released for my changes to be effective. <!-- The "Testing chores" workflow validates that your PR follows our guidelines. --> <!-- If it doesn't pass, click on it to see details as to what your PR might be missing. --> - [x] I will check that all automated PR checks pass before the PR gets reviewed.
1 parent 3263f61 commit 1ef45ae

File tree

3 files changed

+207
-81
lines changed

3 files changed

+207
-81
lines changed

‎.yarn/versions/340bb886.yml

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
releases:
2+
"@yarnpkg/core": patch
3+
4+
declined:
5+
- "@yarnpkg/plugin-compat"
6+
- "@yarnpkg/plugin-constraints"
7+
- "@yarnpkg/plugin-dlx"
8+
- "@yarnpkg/plugin-essentials"
9+
- "@yarnpkg/plugin-exec"
10+
- "@yarnpkg/plugin-file"
11+
- "@yarnpkg/plugin-git"
12+
- "@yarnpkg/plugin-github"
13+
- "@yarnpkg/plugin-http"
14+
- "@yarnpkg/plugin-init"
15+
- "@yarnpkg/plugin-interactive-tools"
16+
- "@yarnpkg/plugin-link"
17+
- "@yarnpkg/plugin-nm"
18+
- "@yarnpkg/plugin-npm"
19+
- "@yarnpkg/plugin-npm-cli"
20+
- "@yarnpkg/plugin-pack"
21+
- "@yarnpkg/plugin-patch"
22+
- "@yarnpkg/plugin-pnp"
23+
- "@yarnpkg/plugin-pnpm"
24+
- "@yarnpkg/plugin-stage"
25+
- "@yarnpkg/plugin-typescript"
26+
- "@yarnpkg/plugin-version"
27+
- "@yarnpkg/plugin-workspace-tools"
28+
- "@yarnpkg/builder"
29+
- "@yarnpkg/cli"
30+
- "@yarnpkg/doctor"
31+
- "@yarnpkg/extensions"
32+
- "@yarnpkg/nm"
33+
- "@yarnpkg/pnpify"
34+
- "@yarnpkg/sdks"

‎packages/acceptance-tests/pkg-tests-specs/sources/dragon.test.js

+107
Original file line numberDiff line numberDiff line change
@@ -726,4 +726,111 @@ describe(`Dragon tests`, () => {
726726
},
727727
),
728728
);
729+
730+
test(`it should pass the dragon test 14`,
731+
makeTemporaryEnv(
732+
{
733+
private: true,
734+
workspaces: [
735+
`pkg-a`,
736+
`pkg-b`,
737+
],
738+
},
739+
async ({path, run, source}) => {
740+
// This dragon test represents the following scenario:
741+
//
742+
// .
743+
// ├── pkg-a/
744+
// │ └── (alias-1) [email protected]
745+
// │ └── (peer) does-not-matter
746+
// └── pkg-b/
747+
// └── (alias-2) [email protected]
748+
// └── (peer) does-not-matter
749+
//
750+
// The same package is installed under two different aliases. This test
751+
// checks that the two instances are properly deduplicated despite
752+
// having different idents.
753+
754+
await xfs.mkdirpPromise(`${path}/pkg-a`);
755+
await xfs.writeJsonPromise(`${path}/pkg-a/package.json`, {
756+
name: `a`,
757+
dependencies: {
758+
[`alias-1`]: `npm:[email protected]`,
759+
[`no-deps`]: `1.0.0`,
760+
},
761+
});
762+
763+
await xfs.mkdirpPromise(`${path}/pkg-b`);
764+
await xfs.writeJsonPromise(`${path}/pkg-b/package.json`, {
765+
name: `b`,
766+
dependencies: {
767+
[`alias-2`]: `npm:[email protected]`,
768+
[`no-deps`]: `1.0.0`,
769+
},
770+
});
771+
772+
await expect(run(`install`)).resolves.toBeTruthy();
773+
774+
// The virtual descriptors should be different but the virtual package should be the same
775+
const aPath = npath.fromPortablePath(ppath.join(path, `pkg-a/package.json`));
776+
const bPath = npath.fromPortablePath(ppath.join(path, `pkg-b/package.json`));
777+
await expect(source(`(
778+
createRequire = require('module').createRequire,
779+
createRequire(${JSON.stringify(aPath)}).resolve('alias-1/package.json') === createRequire(${JSON.stringify(bPath)}).resolve('alias-2/package.json')
780+
)`)).resolves.toBe(true);
781+
},
782+
),
783+
);
784+
785+
test(`it should pass the dragon test 15`,
786+
makeTemporaryEnv(
787+
{
788+
private: true,
789+
workspaces: [
790+
`pkg-a`,
791+
`pkg-b`,
792+
],
793+
},
794+
async ({path, run, source}) => {
795+
// This dragon test represents the following scenario:
796+
//
797+
// .
798+
// ├── pkg-a/
799+
// │ └── (alias-1) [email protected]
800+
// │ └── (peer) does-not-matter
801+
// └── pkg-b/
802+
// └── (alias-1) [email protected]
803+
// │ └── (peer) does-not-matter
804+
// └── (alias-2) [email protected]
805+
// └── (peer) does-not-matter
806+
//
807+
// The same package is installed under two different aliases. When
808+
// traversing pkg-b, we deduplicate the virtual package installed under
809+
// alias-1 to the one under pkg-a, but the virtual package is the same
810+
// one installed under alias-2. This test checks that we don't leave
811+
// dangling references to the virtual package that was removed.
812+
813+
await xfs.mkdirpPromise(`${path}/pkg-a`);
814+
await xfs.writeJsonPromise(`${path}/pkg-a/package.json`, {
815+
name: `a`,
816+
dependencies: {
817+
[`alias-1`]: `npm:[email protected]`,
818+
[`no-deps`]: `1.0.0`,
819+
},
820+
});
821+
822+
await xfs.mkdirpPromise(`${path}/pkg-b`);
823+
await xfs.writeJsonPromise(`${path}/pkg-b/package.json`, {
824+
name: `b`,
825+
dependencies: {
826+
[`alias-1`]: `npm:[email protected]`,
827+
[`alias-2`]: `npm:[email protected]`,
828+
[`no-deps`]: `1.0.0`,
829+
},
830+
});
831+
832+
await expect(run(`install`)).resolves.toBeTruthy();
833+
},
834+
),
835+
);
729836
});

‎packages/yarnpkg-core/sources/Project.ts

+66-81
Original file line numberDiff line numberDiff line change
@@ -2191,10 +2191,11 @@ function applyVirtualResolutionMutations({
21912191

21922192
const allIdents = new Map<IdentHash, Ident>();
21932193

2194-
// We'll be keeping track of all virtual descriptors; once they have all
2195-
// been generated we'll check whether they can be deduplicated into one.
2196-
const allVirtualInstances = new Map<LocatorHash, Map<string, Descriptor>>();
2197-
const allVirtualDependents = new Map<DescriptorHash, Set<LocatorHash>>();
2194+
/** Maps dependency hashes to the first virtual locator encountered with that hash, for deduplication */
2195+
const allVirtualInstances = new Map<string, Locator>();
2196+
const allVirtualDependents = new Map<LocatorHash, Set<LocatorHash>>();
2197+
/** Maps virtual locators to all (virtual) descriptors that resolve to them, for deduplication */
2198+
const allVirtualResolutions = new Map<LocatorHash, Set<DescriptorHash>>();
21982199

21992200
const allPeerRequests = new Map<LocatorHash, Map<IdentHash, PeerRequestNode>>();
22002201

@@ -2263,7 +2264,7 @@ function applyVirtualResolutionMutations({
22632264
if (!parentPackage)
22642265
throw new Error(`Assertion failed: The package (${structUtils.prettyLocator(project.configuration, parentLocator)}) should have been registered`);
22652266

2266-
const newVirtualInstances: Array<[Locator, Descriptor, Package]> = [];
2267+
const dedupeCandidates = new Set<LocatorHash>();
22672268
const parentPeerRequirements = new Map<IdentHash, PeerRequirementNode>();
22682269

22692270
const firstPass = [];
@@ -2322,16 +2323,15 @@ function applyVirtualResolutionMutations({
23222323
virtualizedDescriptor = structUtils.virtualizeDescriptor(descriptor, parentLocator.locatorHash);
23232324
virtualizedPackage = structUtils.virtualizePackage(pkg, parentLocator.locatorHash);
23242325

2325-
parentPackage.dependencies.delete(descriptor.identHash);
2326-
parentPackage.dependencies.set(virtualizedDescriptor.identHash, virtualizedDescriptor);
2326+
parentPackage.dependencies.set(descriptor.identHash, virtualizedDescriptor);
23272327

23282328
allResolutions.set(virtualizedDescriptor.descriptorHash, virtualizedPackage.locatorHash);
23292329
allDescriptors.set(virtualizedDescriptor.descriptorHash, virtualizedDescriptor);
23302330

23312331
allPackages.set(virtualizedPackage.locatorHash, virtualizedPackage);
23322332

2333-
// Keep track of all new virtual packages since we'll want to dedupe them
2334-
newVirtualInstances.push([pkg, virtualizedDescriptor, virtualizedPackage]);
2333+
miscUtils.getSetWithDefault(allVirtualResolutions, virtualizedPackage.locatorHash).add(virtualizedDescriptor.descriptorHash);
2334+
dedupeCandidates.add(virtualizedPackage.locatorHash);
23352335
});
23362336

23372337
// In the second pass we resolve the peer requests to their provision.
@@ -2397,10 +2397,12 @@ function applyVirtualResolutionMutations({
23972397

23982398
virtualizedPackage.dependencies.set(peerDescriptor.identHash, peerProvision);
23992399

2400-
// Need to track when a virtual descriptor is set as a dependency in case
2401-
// the descriptor will be deduplicated.
2400+
// Need to keep track when a virtual depends on a sibling virtual so
2401+
// that if and when the latter is deduplicated, we know the former
2402+
// needs to be deduplicated again
24022403
if (structUtils.isVirtualDescriptor(peerProvision)) {
2403-
const dependents = miscUtils.getSetWithDefault(allVirtualDependents, peerProvision.descriptorHash);
2404+
const dependentLocatorHash = allResolutions.get(peerProvision.descriptorHash);
2405+
const dependents = miscUtils.getSetWithDefault(allVirtualDependents, dependentLocatorHash);
24042406
dependents.add(virtualizedPackage.locatorHash);
24052407
}
24062408

@@ -2447,11 +2449,7 @@ function applyVirtualResolutionMutations({
24472449
// In the fourth pass, we register information about the peer requirement
24482450
// and peer request trees, using the post-deduplication information.
24492451
fourthPass.push(() => {
2450-
const finalDescriptor = parentPackage.dependencies.get(descriptor.identHash);
2451-
if (typeof finalDescriptor === `undefined`)
2452-
throw new Error(`Assertion failed: Expected the peer dependency to have been turned into a dependency`);
2453-
2454-
const finalResolution = allResolutions.get(finalDescriptor.descriptorHash)!;
2452+
const finalResolution = allResolutions.get(virtualizedDescriptor.descriptorHash)!;
24552453
if (typeof finalResolution === `undefined`)
24562454
throw new Error(`Assertion failed: Expected the descriptor to be registered`);
24572455

@@ -2464,10 +2462,10 @@ function applyVirtualResolutionMutations({
24642462
if (!peerRequest)
24652463
continue;
24662464

2467-
peerRequirement.requests.set(finalDescriptor.descriptorHash, peerRequest);
2465+
peerRequirement.requests.set(virtualizedDescriptor.descriptorHash, peerRequest);
24682466
peerRequirementNodes.set(peerRequirement.hash, peerRequirement);
24692467
if (!peerRequirement.root) {
2470-
parentPeerRequests.get(peerRequirement.ident.identHash)?.children.set(finalDescriptor.descriptorHash, peerRequest);
2468+
parentPeerRequests.get(peerRequirement.ident.identHash)?.children.set(virtualizedDescriptor.descriptorHash, peerRequest);
24712469
}
24722470
}
24732471

@@ -2483,76 +2481,63 @@ function applyVirtualResolutionMutations({
24832481
for (const fn of [...firstPass, ...secondPass])
24842482
fn();
24852483

2486-
let stable: boolean;
2487-
do {
2488-
stable = true;
2489-
2490-
for (const [physicalLocator, virtualDescriptor, virtualPackage] of newVirtualInstances) {
2491-
const otherVirtualInstances = miscUtils.getMapWithDefault(allVirtualInstances, physicalLocator.locatorHash);
2492-
2493-
// We take all the dependencies from the new virtual instance and
2494-
// generate a hash from it. By checking if this hash is already
2495-
// registered, we know whether we can trim the new version.
2496-
const dependencyHash = hashUtils.makeHash(
2497-
...[...virtualPackage.dependencies.values()].map(descriptor => {
2498-
const resolution = descriptor.range !== `missing:`
2499-
? allResolutions.get(descriptor.descriptorHash)
2500-
: `missing:`;
2501-
2502-
if (typeof resolution === `undefined`)
2503-
throw new Error(`Assertion failed: Expected the resolution for ${structUtils.prettyDescriptor(project.configuration, descriptor)} to have been registered`);
2504-
2505-
return resolution === top ? `${resolution} (top)` : resolution;
2506-
}),
2507-
// We use the identHash to disambiguate between virtual descriptors
2508-
// with different base idents being resolved to the same virtual package.
2509-
// Note: We don't use the descriptorHash because the whole point of duplicate
2510-
// virtual descriptors is that they have different `virtual:` ranges.
2511-
// This causes the virtual descriptors with different base idents
2512-
// to be preserved, while the virtual package they resolve to gets deduped.
2513-
virtualDescriptor.identHash,
2514-
);
2515-
2516-
const masterDescriptor = otherVirtualInstances.get(dependencyHash);
2517-
if (typeof masterDescriptor === `undefined`) {
2518-
otherVirtualInstances.set(dependencyHash, virtualDescriptor);
2519-
continue;
2520-
}
2521-
2522-
// Since we're applying multiple pass, we might have already registered
2523-
// ourselves as the "master" descriptor in the previous pass.
2524-
if (masterDescriptor === virtualDescriptor)
2525-
continue;
2526-
2527-
allPackages.delete(virtualPackage.locatorHash);
2528-
allDescriptors.delete(virtualDescriptor.descriptorHash);
2529-
allResolutions.delete(virtualDescriptor.descriptorHash);
2530-
2531-
accessibleLocators.delete(virtualPackage.locatorHash);
2484+
for (const locatorHash of dedupeCandidates) {
2485+
// Remove locatorHash here so that if a dependency is deduped, it will be
2486+
// deduped again when added to the dedupe candidates
2487+
dedupeCandidates.delete(locatorHash);
25322488

2533-
const dependents = allVirtualDependents.get(virtualDescriptor.descriptorHash) || [];
2534-
const allDependents = [parentPackage.locatorHash, ...dependents];
2489+
const virtualPackage = allPackages.get(locatorHash)!;
25352490

2536-
allVirtualDependents.delete(virtualDescriptor.descriptorHash);
2491+
// We take all the dependencies from the new virtual instance and
2492+
// generate a hash from it. By checking if this hash is already
2493+
// registered, we know whether we can trim the new version.
2494+
const dependencyHash = hashUtils.makeHash(
2495+
structUtils.devirtualizeLocator(virtualPackage).locatorHash,
2496+
...Array.from(virtualPackage.dependencies.values(), descriptor => {
2497+
const resolution = descriptor.range !== `missing:`
2498+
? allResolutions.get(descriptor.descriptorHash)
2499+
: `missing:`;
25372500

2538-
for (const dependent of allDependents) {
2539-
const pkg = allPackages.get(dependent);
2540-
if (typeof pkg === `undefined`)
2541-
continue;
2501+
if (typeof resolution === `undefined`)
2502+
throw new Error(`Assertion failed: Expected the resolution for ${structUtils.prettyDescriptor(project.configuration, descriptor)} to have been registered`);
25422503

2543-
if (pkg.dependencies.get(virtualDescriptor.identHash)!.descriptorHash !== masterDescriptor.descriptorHash)
2544-
stable = false;
2504+
return resolution === top ? `${resolution} (top)` : resolution;
2505+
}),
2506+
);
25452507

2546-
pkg.dependencies.set(virtualDescriptor.identHash, masterDescriptor);
2547-
}
2508+
const masterLocator = allVirtualInstances.get(dependencyHash);
2509+
if (typeof masterLocator === `undefined`) {
2510+
allVirtualInstances.set(dependencyHash, virtualPackage);
2511+
continue;
2512+
}
25482513

2549-
for (const peerRequirement of parentPeerRequirements.values()) {
2550-
if (peerRequirement.provided.descriptorHash === virtualDescriptor.descriptorHash) {
2551-
peerRequirement.provided = masterDescriptor;
2552-
}
2514+
// Change every descriptor that is resolving to the virtual package to
2515+
// resolve to the master locator instead, then discard the virtual
2516+
// package
2517+
const masterResolutions = miscUtils.getSetWithDefault(allVirtualResolutions, masterLocator.locatorHash);
2518+
for (const descriptorHash of allVirtualResolutions.get(virtualPackage.locatorHash) ?? []) {
2519+
allResolutions.set(descriptorHash, masterLocator.locatorHash);
2520+
masterResolutions.add(descriptorHash);
2521+
}
2522+
allPackages.delete(virtualPackage.locatorHash);
2523+
accessibleLocators.delete(virtualPackage.locatorHash);
2524+
dedupeCandidates.delete(virtualPackage.locatorHash);
2525+
2526+
const dependents = allVirtualDependents.get(virtualPackage.locatorHash);
2527+
if (dependents !== undefined) {
2528+
const masterDependents = miscUtils.getSetWithDefault(allVirtualDependents, masterLocator.locatorHash);
2529+
for (const dependent of dependents) {
2530+
// A dependent of the virtual package is now a dependent of the
2531+
// master package
2532+
masterDependents.add(dependent);
2533+
2534+
// Virtual packages that depended on the deduplicated package would
2535+
// get a different dependency hash now, so we need to deduplicate
2536+
// them again
2537+
dedupeCandidates.add(dependent);
25532538
}
25542539
}
2555-
} while (!stable);
2540+
}
25562541

25572542
for (const fn of [...thirdPass, ...fourthPass]) {
25582543
fn();

0 commit comments

Comments
 (0)