Skip to content

Commit

Permalink
do not remove pattern from lockfile if it is overridden by resolution (
Browse files Browse the repository at this point in the history
…#5572)

* do not remove pattern from lockfile if it is overridden by resolution

* test the bailout after integrity check instead of changes to the lockfile
  • Loading branch information
MhMadHamster authored and arcanis committed Mar 27, 2018
1 parent 25bd1a7 commit 66a0143
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 10 deletions.
3 changes: 3 additions & 0 deletions .vscode/settings.json
@@ -0,0 +1,3 @@
{
"javascript.validate.enable": false
}
23 changes: 23 additions & 0 deletions __tests__/commands/install/resolutions.js
Expand Up @@ -2,9 +2,14 @@

import {getPackageVersion, isPackagePresent, runInstall} from '../_helpers.js';
import {ConsoleReporter} from '../../../src/reporters/index.js';
import * as fs from '../../../src/util/fs.js';
import {Install} from '../../../src/cli/commands/install.js';
import Lockfile from '../../../src/lockfile';

jasmine.DEFAULT_TIMEOUT_INTERVAL = 150000;

const path = require('path');

test.concurrent('install with simple exact resolutions should override all versions', (): Promise<void> => {
return runInstall({}, {source: 'resolutions', cwd: 'simple-exact'}, async config => {
expect(await getPackageVersion(config, 'a')).toEqual('1.0.0');
Expand Down Expand Up @@ -92,3 +97,21 @@ test.concurrent('install with nested resolutions using flat mode', (): Promise<v
expect(await getPackageVersion(config, 'ansi-regex')).toEqual('1.1.1');
});
});

test.concurrent('install with resolution settings should correctly bailout during the integrity check', (): Promise<
void,
> => {
return runInstall(
{},
{source: 'resolutions', cwd: 'install-with-resolution-should-bailout-during-the-integrity-check'},
async (config, reporter): Promise<void> => {
// remove file
await fs.unlink(path.join(config.cwd, 'node_modules', 'left-pad', 'index.js'));
// run install again
const reinstall = new Install({}, config, reporter, await Lockfile.fromDirectory(config.cwd));
await reinstall.init();
// don't expect file being recreated because install should have bailed out
expect(await fs.exists(path.join(config.cwd, 'node_modules', 'left-pad', 'index.js'))).toBe(false);
},
);
});
7 changes: 7 additions & 0 deletions __tests__/fixtures/install/resolutions/e-1/package.json
@@ -0,0 +1,7 @@
{
"name": "e",
"version": "1.0.0",
"dependencies": {
"left-pad": "^1.0.0"
}
}
@@ -0,0 +1,9 @@
{
"dependencies": {
"left-pad": "1.1.1",
"e": "file:../e-1"
},
"resolutions": {
"e/left-pad": "1.1.1"
}
}
1 change: 0 additions & 1 deletion src/cli/commands/install.js
Expand Up @@ -541,7 +541,6 @@ export class Install {
steps.push((curr: number, total: number) =>
callThroughHook('resolveStep', async () => {
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
3 changes: 0 additions & 3 deletions src/package-resolver.js
Expand Up @@ -629,9 +629,6 @@ export default class PackageResolver {
invariant(resolutionManifest._reference, 'resolutions should have a resolved reference');
resolutionManifest._reference.patterns.push(pattern);
this.addPattern(pattern, resolutionManifest);
if (!this.resolutionMap.topLevelPatterns.has(pattern)) {
this.lockfile.removePattern(pattern);
}
} else {
this.resolutionMap.addToDelayQueue(req);
}
Expand Down
6 changes: 0 additions & 6 deletions src/resolution-map.js
Expand Up @@ -33,14 +33,12 @@ 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 @@ -57,10 +55,6 @@ 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 66a0143

Please sign in to comment.