[bug] yarn leaves failed modules in node_modules

[MarshallOfSound]

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

When installing an optionalDependency that fails in a "install" script (such as an incompatible native dependency) the module folder is left behind in the node_modules directory

If the current behavior is a bug, please provide the steps to reproduce.

Make a new package.json that depends on the "mouse-forward-back" module as an optional dependency and run yarn on a windows machine

What is the expected behavior?

The optionalDependency should fail to install and not be present in the node_modules folder

Please mention your node.js, yarn and operating system version.

Windows 10, Yarn 0.16.1, Node.JS 6.3.1

[hpurmann]

I don't have a windows machine right here so I tried to install a windows-only dependency on macOS.

I can confirm this behaviour. Tried with

{
  "name": "yarn-1328",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "optionalDependencies": {
    "edge_case": "https://github.com/paddyb/edge_case"
  }
}

With the current master (73ea5b4), yarn even tries to build native modules.

yarn install v0.18.0-0
info No lockfile found.
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
warning edge_case@1.0.0: The platform "darwin" is incompatible with this module.
info "edge_case@1.0.0" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...
[1/2] ⠂ edge: gyp ERR!
[-/2] ⠂ waiting...
[-/2] ⠂ waiting...
[-/2] ⠂ waiting...
success Saved lockfile.
✨  Done in 3.09s.

The old yarn 17.0 (installed via brew) quits a bit earlier.

yarn install v0.17.0
info No lockfile found.
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
warning edge_case@1.0.0: The platform "darwin" is incompatible with this module.
info "edge_case@1.0.0" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...
success Saved lockfile.
✨  Done in 2.38s.

Both leave all dependencies of paddyb/edge_case in the node_modules folder though.

[danielkcz]

I do confirm this the case of chokidar module and its optional dependency fsevents which fails on Windows, yet it's still kept in node_modules folder adding to its size. For a bigger projects with several different versions of this module it can even add extra 30MB there. Not that it's huge issue, merely optimization, but still.

[sebmck]

Fixed via #2116.