Skip to content

Commit

Permalink
fix(resolver): Fix --frozen-lockfile flag with duplicate top level re…
Browse files Browse the repository at this point in the history
…solution (yarnpkg#4793)

**Summary**

Fixes yarnpkg#4778.

In this particular issue, the same pattern `babel-runtime@^6.26.0` was [defined in both devDependencies](https://github.com/kompot/yarn-frozen-lockfile-bug/blob/master/package.json#L80) and [resolutions field](https://github.com/kompot/yarn-frozen-lockfile-bug/blob/master/package.json#L130). Since resolutions feature was only intended for nested dependencies, it previously didn't take into account when the same exact pattern existed as a top level dependency. And it happens so that after the package resolver phase, [integrity checker looks at top level patterns](https://github.com/yarnpkg/yarn/blob/master/src/integrity-checker.js#L364-L367), so install failed.

The solution was to add top level raw patterns to resolutions map, and only remove them if they're transitive patterns

**Test plan**

Added a new test case in resolutions

*BEFORE*
<img width="618" alt="screen shot 2017-10-28 at 11 07 52 am" src="https://user-images.githubusercontent.com/18429494/32137218-621a19b8-bbd0-11e7-87a3-4acd43a44a69.png">

*AFTER*
<img width="538" alt="screen shot 2017-10-28 at 11 10 13 am" src="https://user-images.githubusercontent.com/18429494/32137229-9b828f1e-bbd0-11e7-90a2-1464c28fcab4.png">
  • Loading branch information
kaylieEB authored and terra-incognita committed Nov 9, 2017
1 parent 0266424 commit d34a917
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 1 deletion.
13 changes: 13 additions & 0 deletions __tests__/commands/install/resolutions.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* @flow */

import {getPackageVersion, isPackagePresent, runInstall} from '../_helpers.js';
import {ConsoleReporter} from '../../../src/reporters/index.js';

jasmine.DEFAULT_TIMEOUT_INTERVAL = 150000;

Expand All @@ -27,6 +28,18 @@ test.concurrent('install with subtree exact resolutions should override subtree
});
});

test.concurrent('install with --frozen-lockfile with resolutions', async (): Promise<void> => {
const reporter = new ConsoleReporter({});

try {
await runInstall({frozenLockfile: true}, {source: 'resolutions', cwd: 'frozen-lockfile'}, async config => {
expect(await getPackageVersion(config, 'left-pad')).toEqual('1.1.3');
});
} catch (err) {
expect(err.message).not.toContain(reporter.lang('frozenLockfileError'));
}
});

test.concurrent('install with exotic resolutions should override versions', (): Promise<void> => {
return runInstall({}, {source: 'resolutions', cwd: 'exotic-version'}, async config => {
expect(await getPackageVersion(config, 'left-pad')).toEqual('1.1.1');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "project",
"version": "1.0.0",
"dependencies": {
"left-pad": "^1.0.0",
"d2": "file:../d2-1"
},
"resolutions": {
"left-pad": "^1.0.0"
}
}
12 changes: 12 additions & 0 deletions __tests__/fixtures/install/resolutions/frozen-lockfile/yarn.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


"d2@file:../d2-1":
version "1.0.0"
dependencies:
left-pad "^1.0.0"

left-pad@^1.0.0:
version "1.1.3"
resolved "https://registry.yarnpkg.com/left-pad/-/left-pad-1.1.3.tgz#612f61c033f3a9e08e939f1caebeea41b6f3199a"
1 change: 1 addition & 0 deletions src/cli/commands/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ export class Install {

steps.push(async (curr: number, total: number) => {
this.reporter.step(curr, total, this.reporter.lang('resolvingPackages'), emoji.get('mag'));
this.resolutionMap.setTopLevelPatterns(rawPatterns);
await this.resolver.init(this.prepareRequests(depRequests), {
isFlat: this.flags.flat,
isFrozen: this.flags.frozenLockfile,
Expand Down
4 changes: 3 additions & 1 deletion src/package-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,9 @@ export default class PackageResolver {
invariant(resolutionManifest._reference, 'resolutions should have a resolved reference');
resolutionManifest._reference.patterns.push(pattern);
this.addPattern(pattern, resolutionManifest);
this.lockfile.removePattern(pattern);
if (!this.resolutionMap.topLevelPatterns.has(pattern)) {
this.lockfile.removePattern(pattern);
}
} else {
this.resolutionMap.addToDelayQueue(req);
}
Expand Down
6 changes: 6 additions & 0 deletions src/resolution-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ export default class ResolutionMap {
this.config = config;
this.reporter = config.reporter;
this.delayQueue = new Set();
this.topLevelPatterns = new Set();
}

resolutionsByPackage: ResolutionInternalMap;
config: Config;
reporter: Reporter;
delayQueue: Set<DependencyRequestPattern>;
topLevelPatterns: Set<string>;

init(resolutions: ?ResolutionEntry = {}) {
for (const globPattern in resolutions) {
Expand All @@ -55,6 +57,10 @@ export default class ResolutionMap {
this.delayQueue.add(req);
}

setTopLevelPatterns(patterns: Array<string>) {
this.topLevelPatterns = new Set(patterns);
}

parsePatternInfo(globPattern: string, range: string): ?Object {
if (!isValidPackagePath(globPattern)) {
this.reporter.warn(this.reporter.lang('invalidResolutionName', globPattern));
Expand Down

0 comments on commit d34a917

Please sign in to comment.