Remove optional dependency and have rush install skip optional

common/changes/nickpape-no-optional_2017-05-05-04-06.json

@@ -0,0 +1,15 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/web-library-build",
+      "comment": "Remove unnecessary fsevents optional dependency",
+      "type": "patch"
+    },
+    {
+      "packageName": "@microsoft/gulp-core-build-webpack",
+      "comment": "Remove unnecessary fsevents optional dependency",
+      "type": "patch"
+    }
+  ],
+  "email": "nickpape@microsoft.com"
+}

gulp-core-build-webpack/package.json

@@ -30,8 +30,5 @@
     "@types/source-map": "0.5.0",
     "@types/uglify-js": "2.6.28",
     "@types/webpack": "1.12.36"
-  },
-  "optionalDependencies": {
-    "fsevents": "*"
   }
 }

rush/rush/src/utilities/InstallManager.ts

@@ -64,10 +64,6 @@ export default class InstallManager {
         project.cyclicDependencyProjects, project.packageJson.dependencies);
       InstallManager._addDependenciesToMap(rushConfiguration, directDependencies,
         project.cyclicDependencyProjects, project.packageJson.devDependencies);
-
-      // TODO: VSO 346348 - implement this correctly:
-      // InstallManager._addDependenciesToMap(rushConfiguration, directDependencies,
-      //  project.cyclicDependencyProjects, project.packageJson.optionalDependencies);
     });
 
     const implicitlyPinned: Map<string, string> = new Map<string, string>();
@@ -180,7 +176,22 @@ export default class InstallManager {
       console.log(os.EOL + 'Running "npm install" in ' + npmToolFolder);
 
       // NOTE: Here we use whatever version of NPM we happen to find in the PATH
-      Utilities.executeCommandWithRetry('npm', ['install'], MAX_INSTALL_ATTEMPTS, npmToolFolder);
+
+      // 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.
+      Utilities.executeCommandWithRetry('npm', ['install', '--no-optional'], MAX_INSTALL_ATTEMPTS, npmToolFolder);
 
       // Create the marker file to indicate a successful install
       fsx.writeFileSync(npmToolFlagFile, '');

web-library-build/package.json

@@ -28,8 +28,5 @@
     "gulp": "~3.9.1",
     "gulp-replace": "^0.5.4"
   },
-  "devDependencies": {},
-  "optionalDependencies": {
-    "fsevents": "*"
-  }
+  "devDependencies": {}
 }