Fix yarn upgrade <package> command to store version in lockfile (#1691

)

__tests__/commands/add.js

@@ -1,6 +1,6 @@
 /* @flow */
 
-import {getPackageVersion, createLockfile, explodeLockfile, run as buildRun} from './_install.js';
+import {getPackageVersion, createLockfile, explodeLockfile, run as buildRun, runInstall} from './_install.js';
 import {Add} from '../../src/cli/commands/add.js';
 import {Reporter} from '../../src/reporters/index.js';
 import * as constants from '../../src/constants.js';
@@ -10,7 +10,6 @@ import Lockfile from '../../src/lockfile/wrapper.js';
 import {run as check} from '../../src/cli/commands/check.js';
 import Config from '../../src/config.js';
 import * as fs from '../../src/util/fs.js';
-import {runInstall} from './_install.js';
 import assert from 'assert';
 import semver from 'semver';
 
@@ -43,26 +42,32 @@ test.concurrent('install with arg', (): Promise<void> => {
 
 test.concurrent('install with --dev flag', (): Promise<void> => {
   return runAdd({dev: true}, ['[email protected]'], 'add-with-flag', async (config) => {
+    const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));
     const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
 
+    assert(lockfile.indexOf('[email protected]:') === 0);
     assert.deepEqual(pkg.devDependencies, {'left-pad': '1.1.0'});
     assert.deepEqual(pkg.dependencies, {});
   });
 });
 
 test.concurrent('install with --peer flag', (): Promise<void> => {
   return runAdd({peer: true}, ['[email protected]'], 'add-with-flag', async (config) => {
+    const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));
     const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
 
+    assert(lockfile.indexOf('[email protected]:') === 0);
     assert.deepEqual(pkg.peerDependencies, {'left-pad': '1.1.0'});
     assert.deepEqual(pkg.dependencies, {});
   });
 });
 
 test.concurrent('install with --optional flag', (): Promise<void> => {
   return runAdd({optional: true}, ['[email protected]'], 'add-with-flag', async (config) => {
+    const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));
     const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
 
+    assert(lockfile.indexOf('[email protected]:') === 0);
     assert.deepEqual(pkg.optionalDependencies, {'left-pad': '1.1.0'});
     assert.deepEqual(pkg.dependencies, {});
   });
@@ -109,6 +114,9 @@ test.concurrent('add should ignore cache', (): Promise<void> => {
 
 test.concurrent('add should not make package.json strict', (): Promise<void> => {
   return runAdd({}, ['left-pad@^1.1.0'], 'install-no-strict', async (config) => {
+    const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));
+
+    assert(lockfile.indexOf('left-pad@^1.1.0:') >= 0);
     assert.deepEqual(
       JSON.parse(await fs.readFile(path.join(config.cwd, 'package.json'))).dependencies,
       {
@@ -121,6 +129,9 @@ test.concurrent('add should not make package.json strict', (): Promise<void> =>
 
 test.concurrent('add --save-exact should not make all package.json strict', (): Promise<void> => {
   return runAdd({saveExact: true}, ['[email protected]'], 'install-no-strict-all', async (config) => {
+    const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));
+
+    assert(lockfile.indexOf('[email protected]:') === 0);
     assert.deepEqual(
       JSON.parse(await fs.readFile(path.join(config.cwd, 'package.json'))).dependencies,
       {
@@ -215,6 +226,9 @@ test.concurrent('add with new dependency should be deterministic', (): Promise<v
       const lockFileWritten = await fs.readFile(path.join(config.cwd, 'yarn.lock'));
       const lockFileLines = explodeLockfile(lockFileWritten);
       assert.equal(lockFileLines.length, 11);
+      assert(lockFileLines.indexOf('mime-db@~1.0.1:') >= 0);
+      assert(lockFileLines.indexOf('[email protected]:') >= 0);
+      assert(lockFileLines.indexOf('[email protected]:') >= 0);
 
 
       const mirror = await fs.walk(path.join(config.cwd, mirrorPath));
@@ -518,3 +532,13 @@ test.concurrent('add should put a git dependency to mirror', (): Promise<void> =
     },
   );
 });
+
+test.concurrent('add should store latest version in lockfile', (): Promise<void> => {
+  return runAdd({}, ['max-safe-integer'], 'latest-version-in-lockfile', async (config) => {
+    const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));
+    const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
+
+    assert(lockfile.indexOf('max-safe-integer@^1.0.1:') === 0);
+    assert.deepEqual(pkg.dependencies, {'max-safe-integer': '^1.0.1'});
+  });
+});

__tests__/commands/upgrade.js

@@ -1,6 +1,7 @@
 /* @flow */
 
 import {Reporter} from '../../src/reporters/index.js';
+import {explodeLockfile} from './_install.js';
 import {run as upgrade} from '../../src/cli/commands/upgrade.js';
 import * as fs from '../../src/util/fs.js';
 import * as reporters from '../../src/reporters/index.js';
@@ -86,10 +87,10 @@ test.concurrent('throws if lockfile is out of date', (): Promise<void> => {
 
 test.concurrent('works with no arguments', (): Promise<void> => {
   return runUpgrade({}, [], 'no-args', async (config): ?Promise<void> => {
-    const lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock'));
+    const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));
     const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
 
-    assert(lockfile.indexOf('left-pad-1.0.0.tgz') === -1);
+    assert(lockfile.indexOf('left-pad@^1.0.0:') === 0);
     // the below test passes when it should fail
     // manifest doesn't get updated when ran without args
     assert.deepEqual(pkg.dependencies, {'left-pad': '^1.0.0'});
@@ -98,11 +99,11 @@ test.concurrent('works with no arguments', (): Promise<void> => {
 
 test.concurrent('works with single argument', (): Promise<void> => {
   return runUpgrade({}, ['max-safe-integer'], 'single-package', async (config): ?Promise<void> => {
-    const lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock'));
+    const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));
     const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
 
-    assert(lockfile.indexOf('left-pad-1.0.0.tgz') >= 0);
-    assert(lockfile.indexOf('max-safe-integer-1.0.0.tgz') === -1);
+    assert(lockfile.indexOf('left-pad@^1.0.0:') >= 0);
+    assert(lockfile.indexOf('max-safe-integer@^1.0.1:') >= 0);
     assert.equal(pkg.dependencies['left-pad'], '^1.0.0');
     assert.notEqual(pkg.dependencies['max-safe-integer'], '^1.0.0');
   });
@@ -111,12 +112,12 @@ test.concurrent('works with single argument', (): Promise<void> => {
 test.concurrent('works with multiple arguments', (): Promise<void> => {
   return runUpgrade({}, ['left-pad', 'max-safe-integer'], 'multiple-packages',
     async (config): ?Promise<void> => {
-      const lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock'));
+      const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));
       const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
 
-      assert(lockfile.indexOf('left-pad-1.0.0.tgz') === -1);
-      assert(lockfile.indexOf('max-safe-integer-1.0.0.tgz') === -1);
-      assert(lockfile.indexOf('is-negative-zero-1.0.0.tgz') >= 0);
+      assert(lockfile.indexOf('left-pad@^1.1.3:') >= 0);
+      assert(lockfile.indexOf('max-safe-integer@^1.0.1:') >= 0);
+      assert(lockfile.indexOf('is-negative-zero@^1.0.0:') >= 0);
       assert.notEqual(pkg.dependencies['left-pad'], '^1.0.0');
       assert.notEqual(pkg.dependencies['max-safe-integer'], '^1.0.0');
       assert.equal(pkg.dependencies['is-negative-zero'], '^1.0.0');
@@ -126,8 +127,11 @@ test.concurrent('works with multiple arguments', (): Promise<void> => {
 
 test.concurrent('respects dependency type', (): Promise<void> => {
   return runUpgrade({}, ['left-pad@^1.1.3'], 'respects-dependency-type', async (config): ?Promise<void> => {
+    const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));
     const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
 
+    assert(lockfile.indexOf('max-safe-integer@^1.0.0:') >= 0);
+    assert(lockfile.indexOf('left-pad@^1.1.3:') >= 0);
     assert.deepEqual(pkg.dependencies, {'max-safe-integer': '^1.0.0'});
     assert.deepEqual(pkg.devDependencies, {'left-pad': '^1.1.3'});
   });
@@ -136,11 +140,21 @@ test.concurrent('respects dependency type', (): Promise<void> => {
 test.concurrent('respects --ignore-engines flag', (): Promise<void> => {
   return runUpgrade({ignoreEngines: true}, ['[email protected]'], 'respects-ignore-engines-flag',
     async (config): ?Promise<void> => {
-      const lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock'));
+      const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));
       const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
 
-      assert(lockfile.indexOf('[email protected]') >= 0);
+      assert(lockfile.indexOf('[email protected]:') >= 0);
       assert.deepEqual(pkg.dependencies, {hawk: '0.10'});
     },
   );
 });
+
+test.concurrent('upgrades from fixed version to latest', (): Promise<void> => {
+  return runUpgrade({}, ['max-safe-integer'], 'fixed-to-latest', async (config): ?Promise<void> => {
+    const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));
+    const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
+
+    assert(lockfile.indexOf('max-safe-integer@^1.0.1:') === 0);
+    assert.deepEqual(pkg.dependencies, {'max-safe-integer': '^1.0.1'});
+  });
+});

__tests__/fixtures/add/latest-version-in-lockfile/package.json

@@ -0,0 +1,3 @@
+{
+  "dependencies": {}
+}

__tests__/fixtures/upgrade/fixed-to-latest/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "max-safe-integer": "1.0.0"
+  }
+}

src/cli/commands/add.js

@@ -98,6 +98,8 @@ export class Add extends Install {
 
   async savePackages(): Promise<void> {
     const {exact, tilde} = this.flags;
+    // hold only patterns for lockfile
+    const patterns = [];
 
     // fill rootPatternsToOrigin without `excludePatterns`
     await Install.prototype.fetchRequestFromCwd.call(this);
@@ -155,9 +157,13 @@ export class Add extends Install {
       }
       this.resolver.addPattern(newPattern, pkg);
       this.resolver.removePattern(pattern);
+
+      // push new pattern to the lockfile
+      patterns.push(newPattern);
     }
 
     await this.config.saveRootManifests(manifests);
+    await this.saveLockfileAndIntegrity(patterns);
   }
 }