From 66a0143a753cd4ade1a0fffee2174890d564c129 Mon Sep 17 00:00:00 2001 From: Max Burmagin Date: Tue, 27 Mar 2018 18:06:27 +0300 Subject: [PATCH] do not remove pattern from lockfile if it is overridden by resolution (#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 --- .vscode/settings.json | 3 +++ __tests__/commands/install/resolutions.js | 23 +++++++++++++++++++ .../install/resolutions/e-1/package.json | 7 ++++++ .../package.json | 9 ++++++++ src/cli/commands/install.js | 1 - src/package-resolver.js | 3 --- src/resolution-map.js | 6 ----- 7 files changed, 42 insertions(+), 10 deletions(-) create mode 100644 .vscode/settings.json create mode 100644 __tests__/fixtures/install/resolutions/e-1/package.json create mode 100644 __tests__/fixtures/install/resolutions/install-with-resolution-should-bailout-during-the-integrity-check/package.json diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000000..f9a4c68002 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "javascript.validate.enable": false +} diff --git a/__tests__/commands/install/resolutions.js b/__tests__/commands/install/resolutions.js index 92cb994232..286cf28940 100644 --- a/__tests__/commands/install/resolutions.js +++ b/__tests__/commands/install/resolutions.js @@ -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 => { return runInstall({}, {source: 'resolutions', cwd: 'simple-exact'}, async config => { expect(await getPackageVersion(config, 'a')).toEqual('1.0.0'); @@ -92,3 +97,21 @@ test.concurrent('install with nested resolutions using flat mode', (): Promise => { + return runInstall( + {}, + {source: 'resolutions', cwd: 'install-with-resolution-should-bailout-during-the-integrity-check'}, + async (config, reporter): Promise => { + // 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); + }, + ); +}); diff --git a/__tests__/fixtures/install/resolutions/e-1/package.json b/__tests__/fixtures/install/resolutions/e-1/package.json new file mode 100644 index 0000000000..ce270d0b75 --- /dev/null +++ b/__tests__/fixtures/install/resolutions/e-1/package.json @@ -0,0 +1,7 @@ +{ + "name": "e", + "version": "1.0.0", + "dependencies": { + "left-pad": "^1.0.0" + } +} diff --git a/__tests__/fixtures/install/resolutions/install-with-resolution-should-bailout-during-the-integrity-check/package.json b/__tests__/fixtures/install/resolutions/install-with-resolution-should-bailout-during-the-integrity-check/package.json new file mode 100644 index 0000000000..538bd160d2 --- /dev/null +++ b/__tests__/fixtures/install/resolutions/install-with-resolution-should-bailout-during-the-integrity-check/package.json @@ -0,0 +1,9 @@ +{ + "dependencies": { + "left-pad": "1.1.1", + "e": "file:../e-1" + }, + "resolutions": { + "e/left-pad": "1.1.1" + } +} diff --git a/src/cli/commands/install.js b/src/cli/commands/install.js index 6454bc7e24..a3894575a3 100644 --- a/src/cli/commands/install.js +++ b/src/cli/commands/install.js @@ -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, diff --git a/src/package-resolver.js b/src/package-resolver.js index e2555ebf4a..d62dd5db85 100644 --- a/src/package-resolver.js +++ b/src/package-resolver.js @@ -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); } diff --git a/src/resolution-map.js b/src/resolution-map.js index 12e548dd9b..fbc0af788a 100644 --- a/src/resolution-map.js +++ b/src/resolution-map.js @@ -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; - topLevelPatterns: Set; init(resolutions: ?ResolutionEntry = {}) { for (const globPattern in resolutions) { @@ -57,10 +55,6 @@ export default class ResolutionMap { this.delayQueue.add(req); } - setTopLevelPatterns(patterns: Array) { - this.topLevelPatterns = new Set(patterns); - } - parsePatternInfo(globPattern: string, range: string): ?Object { if (!isValidPackagePath(globPattern)) { this.reporter.warn(this.reporter.lang('invalidResolutionName', globPattern));