diff --git a/apps/rush-lib/src/logic/InstallManager.ts b/apps/rush-lib/src/logic/InstallManager.ts index dfe4f9ee9b1..2ad3232f8c9 100644 --- a/apps/rush-lib/src/logic/InstallManager.ts +++ b/apps/rush-lib/src/logic/InstallManager.ts @@ -781,23 +781,6 @@ export class InstallManager { } // Run "npm install" in the common folder - - // NOTE: - // we do NOT install optional dependencies for Rush, as it seems that optional dependencies do not - // work properly with shrinkwrap. Consider the "fsevents" package. This is a Mac specific package - // which is an optional second-order dependency. Optional dependencies work by attempting to install - // the package, but removes the package if the install failed. - // This means that someone running generate on a Mac WILL have fsevents included in their shrinkwrap. - // When someone using Windows attempts to install from the shrinkwrap, the install will fail. - // - // If someone generates the shrinkwrap using Windows, then fsevents will NOT be listed in the shrinkwrap. - // When someone using Mac attempts to install from the shrinkwrap, (as of NPM 4), they will NOT have the - // optional dependency installed. - // - // One possible solution would be to have the shrinkwrap include information about whether the dependency - // is optional or not, but it does not appear to do so. Also, this would result in strange behavior where - // people would have different node_modules based on their system. - const installArgs: string[] = [ 'install' ]; this._pushConfigurationArgs(installArgs, options); @@ -975,7 +958,27 @@ export class InstallManager { */ private _pushConfigurationArgs(args: string[], options: IInstallManagerOptions): void { if (this._rushConfiguration.packageManager === 'npm') { - args.push('--no-optional'); + if (semver.lt(this._rushConfiguration.packageManagerToolVersion, '5.0.0')) { + // NOTE: + // + // When using an npm version older than v5.0.0, we do NOT install optional dependencies for + // Rush, because npm does not generate the shrinkwrap file consistently across platforms. + // + // Consider the "fsevents" package. This is a Mac specific package + // which is an optional second-order dependency. Optional dependencies work by attempting to install + // the package, but removes the package if the install failed. + // This means that someone running generate on a Mac WILL have fsevents included in their shrinkwrap. + // When someone using Windows attempts to install from the shrinkwrap, the install will fail. + // + // If someone generates the shrinkwrap using Windows, then fsevents will NOT be listed in the shrinkwrap. + // When someone using Mac attempts to install from the shrinkwrap, they will NOT have the + // optional dependency installed. + // + // This issue has been fixed as of npm v5.0.0: https://github.com/npm/npm/releases/tag/v5.0.0 + // + // For more context, see https://github.com/Microsoft/web-build-tools/issues/761#issuecomment-428689600 + args.push('--no-optional'); + } args.push('--cache', this._rushConfiguration.npmCacheFolder); args.push('--tmp', this._rushConfiguration.npmTmpFolder); @@ -983,7 +986,6 @@ export class InstallManager { args.push('--verbose'); } } else if (this._rushConfiguration.packageManager === 'pnpm') { - args.push('--no-optional'); args.push('--store', this._rushConfiguration.pnpmStoreFolder); // we are using the --no-lock flag for now, which unfortunately prints a warning, but should be OK @@ -1009,7 +1011,6 @@ export class InstallManager { args.push('--strict-peer-dependencies'); } } else if (this._rushConfiguration.packageManager === 'yarn') { - args.push('--ignore-optional'); args.push('--link-folder', 'yarn-link'); args.push('--cache-folder', this._rushConfiguration.yarnCacheFolder); diff --git a/apps/rush-lib/src/logic/npm/NpmLinkManager.ts b/apps/rush-lib/src/logic/npm/NpmLinkManager.ts index 502512ff14c..d0e7a1bfc8d 100644 --- a/apps/rush-lib/src/logic/npm/NpmLinkManager.ts +++ b/apps/rush-lib/src/logic/npm/NpmLinkManager.ts @@ -296,7 +296,7 @@ export class NpmLinkManager extends BaseLinkManager { throw Error(`The dependency "${dependency.name}" needed by "${localPackage.name}"` + ` was not found in the common folder -- do you need to run "rush install"?`); } else { - console.log(colors.yellow('Skipping optional dependency: ' + dependency.name)); + console.log('Skipping optional dependency: ' + dependency.name); } } } diff --git a/common/changes/@microsoft/rush/rush-install-optional_2018-10-16-03-26.json b/common/changes/@microsoft/rush/rush-install-optional_2018-10-16-03-26.json new file mode 100644 index 00000000000..bfce907dff5 --- /dev/null +++ b/common/changes/@microsoft/rush/rush-install-optional_2018-10-16-03-26.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "comment": "Install optional dependencies, except w/ npm<5.0.0", + "packageName": "@microsoft/rush", + "type": "none" + } + ], + "packageName": "@microsoft/rush", + "email": "elliottsj@users.noreply.github.com" +} \ No newline at end of file