Can't npm shinkwrap because of extraneous deps

[ntr-808]

Side effect of the osx post-install script.

Thoughts feelings emotions?

---

npm ERR! Error: Problems were encountered
npm ERR! Please correct and try again.
npm ERR! extraneous: fsevents@0.1.6 /Users/nathanr/webclient/node_modules/chokidar/node_modules/fsevents
npm ERR! extraneous: recursive-readdir@0.0.2 /Users/nathanr/webclient/node_modules/chokidar/node_modules/recursive-readdir
...
npm ERR! System Darwin 11.4.2
npm ERR! command "node" "/usr/local/bin/npm" "shrinkwrap"
npm ERR! node -v v0.10.24
npm ERR! npm -v 1.3.21

[paulmillr]

@izs any idea what's this? We have a postinstall script that installs deps only on os x

[paulmillr]

@isaacs

[es128]

I think adding --save-optional to the npm command in the postinstall script would be the best bet.

[nrako]

oh good idea!

[kleinsch]

Agreed with @es128 . Running postinstall script on a Mac adds packages that aren't in package.json, which breaks npm shrinkwrap. Either the dependencies need to be in package.json (which sounds like a bad call based on discussion to #94 ) or they need to be added to it as part of the postinstall script.

[vjpr]

+1. Breaks shrinkwrap for me too. The post install script is a bad idea.

A post install message should be shown to the user that they need to install these libraries manually in their app.

Or when the module is first used check for the library and throw an error that they need to install libs.

There should also be a fallback if possible.

Workaround

npm install fsevents recursive-readdir --save and rm -rf node_modules/chokidar/node_modules

[es128]

At this point, I agree that we have not been successful in finding a clean way to install fsevents conditionally, but I think it should remain possible for individual users to enable it. Even with --save-optional it appears #95 will still be an issue. OTOH, being able to easily enable fsevents may be a good solution for users like the one at brunch/brunch#783.

I suggest switching the postinstall to something arbitrary, like fsevents, and provide instructions in the readme and possibly in the console during install for users to manually npm run fsevents if they want these features. Dependents of chokidar like karma and brunch, if they decide they want to, could also mention this in their docs and/or set up their own npm script so users don't have to dig down into node_modules for it.

If I find the time today I'll prepare a PR.

[jagill]

I would love some way to be able to instal fsevents when useful, and remove it when not. It dramatically improves performance on OS X, but we also need to make builds on other architectures.

It seems safer to not have it by default, but allow OS X users to install it and use it.

[quicksnap]

We're having a build issue with the 0.1.6 version of fsevents that was resolved in 0.2.0. I was hoping to shrinkwrap, but I was brought here.

Would love to avoid forking chokidar for this little issue!

[vojtajina]

@paulmillr can you push this to NPM?

[IgorMinar]

This will still be an issue even with optionalDependencies because of an npm bug: npm/npm#2679 (comment)

(just fyi, I still think that declaring these as optional deps is a good thing)

[startswithaj]

Workaround: If you don't need fsevents and recursive-readdir in your deployment env. Just go into node_modules/chokidar/node_modules and rm -rf those directories. Then do npm_shrinkwrap. if you do need them (if your env is OSX??). Go into node_modules/chokidar/package.json and add them as dependecies.

[IgorMinar]

@startswithaj you have to do this every time you upgrade which is far from ideal. My PR #106 fixes the problem by using optionalDependencies and fsevent 0.2.0 with "os": ["darwin"] declaration in package.json.

[startswithaj]

@IgorMinar totally agree. Nice work on the fix.