0.23.1 cherry-picking

__tests__/commands/install/integration.js

@@ -157,6 +157,22 @@ test.concurrent("production mode with deduped dev dep shouldn't be removed", asy
   });
 });
 
+test.concurrent("production mode dep on package in dev deps shouldn't be removed", async () => {
+  await runInstall({production: true}, 'install-prod-deduped-direct-dev-dep', async (config) => {
+    expect(
+      (await fs.readJson(path.join(config.cwd, 'node_modules', 'a', 'package.json'))).version,
+    ).toEqual('1.0.0');
+
+    expect(
+      (await fs.readJson(path.join(config.cwd, 'node_modules', 'b', 'package.json'))).version,
+    ).toEqual('1.0.0');
+
+    expect(
+      (await fs.readJson(path.join(config.cwd, 'node_modules', 'c', 'package.json'))).version,
+    ).toEqual('1.0.0');
+  });
+});
+
 test.concurrent('hoisting should factor ignored dependencies', async () => {
   // you should only modify this test if you know what you're doing
   // when we calculate hoisting we need to factor in ignored dependencies in it
@@ -800,3 +816,15 @@ test.concurrent('prunes the offline mirror after pruning is enabled', (): Promis
     expect(await fs.exists(path.join(config.cwd, `${mirrorPath}/dep-b-1.0.0.tgz`))).toEqual(false);
   });
 });
+
+test.concurrent('bailout should work with --production flag too', (): Promise<void> => {
+  return runInstall({production: true}, 'bailout-prod', 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({production: true}, 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);
+  });
+});

__tests__/fixtures/install/bailout-prod/package.json

@@ -0,0 +1,8 @@
+{
+  "dependencies": {
+    "left-pad": "^1.1.3"
+  },
+  "devDependencies": {
+    "is-array": "^1.0.1"
+  }
+}

__tests__/fixtures/install/bailout-prod/yarn.lock

@@ -0,0 +1,11 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+# yarn lockfile v1
+
+
+is-array@^1.0.1:
+  version "1.0.1"
+  resolved "https://registry.yarnpkg.com/is-array/-/is-array-1.0.1.tgz#e9850cc2cc860c3bc0977e84ccf0dd464584279a"
+
+left-pad@^1.1.3:
+  version "1.1.3"
+  resolved "https://registry.yarnpkg.com/left-pad/-/left-pad-1.1.3.tgz#612f61c033f3a9e08e939f1caebeea41b6f3199a"

__tests__/fixtures/install/install-prod-deduped-direct-dev-dep/a/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "a",
+  "version": "1.0.0",
+  "dependencies": {
+    "b": "file:../b"
+  }
+}

__tests__/fixtures/install/install-prod-deduped-direct-dev-dep/b/c/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "c",
+  "version": "1.0.0"
+}

__tests__/fixtures/install/install-prod-deduped-direct-dev-dep/b/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "b",
+  "version": "1.0.0",
+  "dependencies": {
+    "c": "file:c"
+  }
+}

__tests__/fixtures/install/install-prod-deduped-direct-dev-dep/package.json

@@ -0,0 +1,8 @@
+{
+  "dependencies": {
+    "a": "file:a"
+  },
+  "devDependencies": {
+    "b": "file:b"
+  }
+}

__tests__/lifecycle-scripts.js

@@ -32,28 +32,28 @@ async function execCommand(cmd: string, packageName: string, env = process.env):
 
 test('should add the global yarnrc arguments to the command line', async () => {
   const stdout = await execCommand('cache dir', 'yarnrc-cli');
-  expect(stdout.replace(/\\/g, '/')).toMatch(/^\/tmp\/foobar\/v[0-9]+\n$/);
+  expect(stdout.replace(/\\/g, '/')).toMatch(/^(C:)?\/tmp\/foobar\/v[0-9]+\n$/);
 });
 
 test('should add the command-specific yarnrc arguments to the command line if the command name matches', async () => {
   const stdout = await execCommand('cache dir', 'yarnrc-cli-command-specific-ok');
-  expect(stdout.replace(/\\/g, '/')).toMatch(/^\/tmp\/foobar\/v[0-9]+\n$/);
+  expect(stdout.replace(/\\/g, '/')).toMatch(/^(C:)?\/tmp\/foobar\/v[0-9]+\n$/);
 });
 
 test('should not add the command-specific yarnrc arguments if the command name doesn\'t match', async () => {
   const stdout = await execCommand('cache dir', 'yarnrc-cli-command-specific-ko');
-  expect(stdout.replace(/\\/g, '/')).not.toMatch(/^\/tmp\/foobar\/v[0-9]+\n$/);
+  expect(stdout.replace(/\\/g, '/')).not.toMatch(/^(C:)?\/tmp\/foobar\/v[0-9]+\n$/);
 });
 
 test('should allow overriding the yarnrc values from the command line', async () => {
   const stdout = await execCommand('cache dir --cache-folder /tmp/toto', 'yarnrc-cli');
-  expect(stdout.replace(/\\/g, '/')).toMatch(/^\/tmp\/toto\/v[0-9]+\n$/);
+  expect(stdout.replace(/\\/g, '/')).toMatch(/^(C:)?\/tmp\/toto\/v[0-9]+\n$/);
 });
 
 // Test disabled for now, cf rc.js
-test.skip('should resolve the yarnrc values relative to where the file lives', async () => {
+test('should resolve the yarnrc values relative to where the file lives', async () => {
   const stdout = await execCommand('cache dir', 'yarnrc-cli-relative');
-  expect(stdout.replace(/\\/g, '/')).toMatch(/^(\/[^\/]+)+\/foobar\/hello\/world\/v[0-9]+\n$/);
+  expect(stdout.replace(/\\/g, '/')).toMatch(/^(C:)?(\/[^\/]+)+\/foobar\/hello\/world\/v[0-9]+\n$/);
 });
 
 test('should expose `npm_config_argv` environment variable to lifecycle scripts for back compatibility with npm (#684)',

__tests__/package-hoister.js

@@ -110,7 +110,7 @@ test('Produces valid destination paths for scoped modules', () => {
     _reference: (({}: any): PackageReference),
   }: any): Manifest);
 
-  const info = new HoistManifest(key, parts, pkg, '', false, false);
+  const info = new HoistManifest(key, parts, pkg, '', true, false);
 
   const tree = new Map([
     ['@scoped/dep', info],

circle.yml

@@ -33,6 +33,7 @@ test:
     - yarn check-lockfile
     - yarn build-dist
     - node ./scripts/build-webpack.js
+    - ls -t1 artifacts | head -n2 | while read entry; do node "./artifacts/$entry" --version; done
     - ./scripts/build-deb.sh
     # Test that installing as root works and that it also works
     # behind a user namespace which Circle CI tests are run under

package.json

@@ -28,7 +28,6 @@
     "node-gyp": "^3.2.1",
     "object-path": "^0.11.2",
     "proper-lockfile": "^2.0.0",
-    "rc": "^1.2.1",
     "read": "^1.0.7",
     "request": "^2.81.0",
     "request-capture-har": "^1.2.2",

scripts/build-webpack.js

@@ -56,7 +56,7 @@ const compilerLegacy = webpack({
       exclude: /node_modules/,
       loader: 'babel-loader',
       query: babelRc.env['pre-node5'],
-    }],
+    }]
   },
   plugins: [
     new webpack.BannerPlugin({

src/cli/commands/install.js

@@ -252,6 +252,7 @@ export class Install {
             pattern += '@' + depMap[name];
           }
 
+          // normalization made sure packages are mentioned only once
           if (isUsed) {
             usedPatterns.push(pattern);
           } else {
@@ -357,12 +358,9 @@ export class Install {
       const ref = manifest._reference;
       invariant(ref, 'expected package reference');
 
-      if (ref.requests.length === 1) {
-        // this module was only depended on once by the root so we can safely ignore it
-        // if it was requested more than once then ignoring it would break a transitive
-        // dep that resolved to it
-        ref.ignore = true;
-      }
+      // just mark the package as ignored. if the package is used by a required package, the hoister
+      // will take care of that.
+      ref.ignore = true;
     }
   }
 
@@ -385,7 +383,6 @@ export class Install {
       requests: depRequests,
       patterns: rawPatterns,
       ignorePatterns,
-      usedPatterns,
     } = await this.fetchRequestFromCwd();
     let topLevelPatterns: Array<string> = [];
 
@@ -394,7 +391,7 @@ export class Install {
       await this.resolver.init(this.prepareRequests(depRequests), this.flags.flat);
       topLevelPatterns = this.preparePatterns(rawPatterns);
       flattenedTopLevelPatterns = await this.flatten(topLevelPatterns);
-      return {bailout: await this.bailout(usedPatterns)};
+      return {bailout: await this.bailout(topLevelPatterns)};
     });
 
     steps.push(async (curr: number, total: number) => {

src/package-compatibility.js

@@ -126,6 +126,7 @@ export default class PackageCompatibility {
 
       if (ref.optional) {
         ref.ignore = true;
+        ref.incompatible = true;
 
         reporter.warn(`${human}: ${msg}`);
         if (!didIgnore) {

src/package-hoister.js

@@ -13,9 +13,9 @@ type Parts = Array<string>;
 let historyCounter = 0;
 
 export class HoistManifest {
-  constructor(key: string, parts: Parts, pkg: Manifest, loc: string, isIgnored: boolean, inheritIsIgnored: boolean) {
-    this.isIgnored = isIgnored;
-    this.inheritIsIgnored = inheritIsIgnored;
+  constructor(key: string, parts: Parts, pkg: Manifest, loc: string, isRequired: boolean, isIncompatible: boolean) {
+    this.isRequired = isRequired;
+    this.isIncompatible = isIncompatible;
     this.loc = loc;
     this.pkg = pkg;
 
@@ -28,8 +28,8 @@ export class HoistManifest {
     this.addHistory(`Start position = ${key}`);
   }
 
-  isIgnored: boolean;
-  inheritIsIgnored: boolean;
+  isRequired: boolean;
+  isIncompatible: boolean;
   pkg: Manifest;
   loc: string;
   parts: Parts;
@@ -96,7 +96,7 @@ export default class PackageHoister {
     while (true) {
       let queue = this.levelQueue;
       if (!queue.length) {
-        this._propagateNonIgnored();
+        this._propagateRequired();
         return;
       }
 
@@ -135,18 +135,17 @@ export default class PackageHoister {
 
     //
     let parentParts: Parts = [];
-    let isIgnored = ref.ignore;
-    let inheritIsIgnored = false;
+    const isIncompatible = ref.incompatible;
+    let isRequired = !parent && !ref.ignore && !isIncompatible;
 
     if (parent) {
       if (!this.tree.get(parent.key)) {
         return null;
       }
       // non ignored dependencies inherit parent's ignored status
       // parent may transition from ignored to non ignored when hoisted if it is used in another non ignored branch
-      if (!isIgnored && parent.isIgnored) {
-        isIgnored = parent.isIgnored;
-        inheritIsIgnored = true;
+      if (!isRequired && !isIncompatible && parent.isRequired) {
+        isRequired = true;
       }
       parentParts = parent.parts;
     }
@@ -155,7 +154,7 @@ export default class PackageHoister {
     const loc: string = this.config.generateHardModulePath(ref);
     const parts = parentParts.concat(pkg.name);
     const key: string = this.implodeKey(parts);
-    const info: HoistManifest = new HoistManifest(key, parts, pkg, loc, isIgnored, inheritIsIgnored);
+    const info: HoistManifest = new HoistManifest(key, parts, pkg, loc, isRequired, isIncompatible);
 
     //
     this.tree.set(key, info);
@@ -173,13 +172,13 @@ export default class PackageHoister {
    * Propagate inherited ignore statuses from non-ignored to ignored packages
   */
 
-  _propagateNonIgnored() {
+  _propagateRequired() {
     //
     const toVisit: Array<HoistManifest> = [];
 
     // enumerate all non-ignored packages
     for (const entry of this.tree.entries()) {
-      if (!entry[1].isIgnored) {
+      if (entry[1].isRequired) {
         toVisit.push(entry[1]);
       }
     }
@@ -192,9 +191,9 @@ export default class PackageHoister {
 
       for (const depPattern of ref.dependencies) {
         const depinfo = this._lookupDependency(info, depPattern);
-        if (depinfo && depinfo.isIgnored && depinfo.inheritIsIgnored) {
-          depinfo.isIgnored = false;
-          info.addHistory(`Mark as non-ignored because of usage by ${info.key}`);
+        if (depinfo && !depinfo.isRequired && !depinfo.isIncompatible) {
+          depinfo.isRequired = true;
+          depinfo.addHistory(`Mark as non-ignored because of usage by ${info.key}`);
           toVisit.push(depinfo);
         }
       }
@@ -247,12 +246,14 @@ export default class PackageHoister {
       const existing = this.tree.get(checkKey);
       if (existing) {
         if (existing.loc === info.loc) {
-          // switch to non ignored if earlier deduped version was ignored
-          if (existing.isIgnored && !info.isIgnored) {
-            existing.isIgnored = info.isIgnored;
+          // switch to non ignored if earlier deduped version was ignored (must be compatible)
+          if (!existing.isRequired && info.isRequired) {
+            existing.addHistory(`Deduped ${fullKey} to this item, marking as required`);
+            existing.isRequired = true;
+          } else {
+            existing.addHistory(`Deduped ${fullKey} to this item`);
           }
 
-          existing.addHistory(`Deduped ${fullKey} to this item`);
           return {parts: checkParts, duplicate: true};
         } else {
           // everything above will be shadowed and this is a conflict
@@ -543,7 +544,7 @@ export default class PackageHoister {
       const ref = info.pkg._reference;
       invariant(ref, 'expected reference');
 
-      if (info.isIgnored) {
+      if (!info.isRequired) {
         info.addHistory('Deleted as this module was ignored');
       } else {
         visibleFlatTree.push([loc, info]);

src/package-install-scripts.js

@@ -111,6 +111,7 @@ export default class PackageInstallScripts {
 
       if (ref.optional) {
         ref.ignore = true;
+        ref.incompatible = true;
         this.reporter.warn(this.reporter.lang('optionalModuleScriptFail', err.message));
         this.reporter.info(this.reporter.lang('optionalModuleFail'));
 

src/package-reference.js

@@ -33,6 +33,7 @@ export default class PackageReference {
     this.optional = null;
     this.root = false;
     this.ignore = false;
+    this.incompatible = false;
     this.fresh = false;
     this.location = null;
     this.addRequest(request);
@@ -48,6 +49,7 @@ export default class PackageReference {
   uid: string;
   optional: ?boolean;
   ignore: boolean;
+  incompatible: boolean;
   fresh: boolean;
   dependencies: Array<string>;
   patterns: Array<string>;

src/rc.js

@@ -1,8 +1,8 @@
 /* @flow */
 
+import {dirname, resolve} from 'path';
 import parse from './lockfile/parse.js';
-
-const rc = require('rc');
+import * as rcUtil from './util/rc.js';
 
 // Keys that will get resolved relative to the path of the rc file they belong to
 const PATH_KEYS = [
@@ -14,20 +14,14 @@ const PATH_KEYS = [
 let rcConfCache;
 let rcArgsCache;
 
-const buildRcConf = () => rc('yarn', {}, [], (fileText) => {
+const buildRcConf = () => rcUtil.findRc('yarn', (fileText, filePath) => {
   const values = parse(fileText, 'yarnrc');
   const keys = Object.keys(values);
 
-  // Unfortunately, the "rc" module we use doesn't tell us the file path :(
-  // cf https://github.com/dominictarr/rc/issues/61
-
   for (const key of keys) {
     for (const pathKey of PATH_KEYS) {
-      if (key.replace(/^(--)?([^.]+\.)+/, '') === pathKey) {
-        // values[key] = resolve(dirname(filePath), values[key]);
-        if (!values[key].startsWith('/')) {
-          delete values[keys];
-        }
+      if (key.replace(/^(--)?([^.]+\.)*/, '') === pathKey) {
+        values[key] = resolve(dirname(filePath), values[key]);
       }
     }
   }