remove peerDependencies

[boneskull]

A missing peerDependency will be reduced to a warning with npm v3.0.0, which is now in beta.

This package requires istanbul, among other things, to be installed.

It just so happens that a fork of istanbul (istanbul-harmony) works better than the harmony branch of istanbul itself.

The istanbul-harmony package uses the same executable, node_modules/.bin/istanbul. This means they cannot coexist; if istanbul-harmony is installed, and grunt-mocha-istanbul is installed thereafter, the presence of the peer dependency creates a conflict, and npm will fail (let me know if you'd like me to paste logs).

This is a non-issue in npm v3.0.0, because it does not attempt to install this package's peerDependencies.

README.md mentions that you may use something else like ibrik if you wish, but ibrik does not use the same executable. Using ibrik doesn't stop this package from installing istanbul anyway (and checking for its presence), even if it will remain unused--which this package should stop doing anyway.

If the intent is to let the user use whatever packages (or forks!) they wish, then:

• the peerDependencies property of package.json should be removed
• explicit checks in index.js for the dependencies should be removed
• at execution time, catch errors in the case of a non-existent mocha, grunt, or istanbul, and display a friendly warning message
• update README.md to instruct users to explictly install the three dependencies

I can provide a PR if you wish.

[pocesar]

sounds reasonable.
also the ibrik example needs to be complemented, it requires the scriptPath to be set to require.resolve('ibrik/bin/ibrik')
send me a PR, I'll merge it. the https://github.com/pocesar/grunt-mocha-istanbul/blob/master/tasks/index.js#L10-L22 part should be refactored to check scriptPath and the resolved mocha dependency before it continues with the task (shouldn't be done in the index.js outuside of the task. done so, it's able to warn / fail dependending on the user command line options, like --force. right now, the pipeline will be broken because of that if the peerDepedency isn't met).
Please also clarify that it needs to use require.resolve('istanbul-harmony/lib/cli') for the ES6+ coverage

[boneskull]

Please also clarify that it needs to use require.resolve('istanbul-harmony/lib/cli') for the ES6+ coverage

I'm not sure if we should do this--it's likely to become out-of-date information as istanbul proper gets up to speed.

[voxpelli]

I think I disagree with this change. peerDependencies are not going away – they are just not being enforced anymore. Not fulfilling a peer dependency will result in a warning one can chose to either address or ignore.

Relevant discussion where the npm authors specifically state that peerDependencies are here to stay: npm/npm#6565 (comment)

I think especially the grunt peer dependency should be kept but also the others. The peer dependency points out what API version of what modules one expects and if someone decides to fulfill that dependency in another way than using the standard dependency, then they can safely ignore any warnings about the missing peer dependency.

For those using the standard modules it's rather good to get a warning if eg. a new grunt or mocha version haven't been tested with this module and thus may be incompatible with it or if they have forgotten to install one of them – and by keeping the peerDependencies declaration, that's exactly what they will get.

[boneskull]

@voxpelli Has a reasonable argument in the case of grunt, but the fact of the matter is that if I have both grunt-mocha-istanbul and istanbul-harmony in my devDependencies, npm install will fail in npm <v3.

npm ERR! Linux 3.13.0-40-generic
npm ERR! argv "/home/travis/.nvm/versions/io.js/v2.4.0/bin/iojs" "/home/travis/.nvm/versions/io.js/v2.4.0/bin/npm" "install"
npm ERR! node v2.4.0
npm ERR! npm  v2.13.0
npm ERR! path ../istanbul/lib/cli.js
npm ERR! code EEXIST
npm ERR! errno -17
npm ERR! syscall symlink
npm ERR! EEXIST: file already exists, symlink '../istanbul/lib/cli.js' -> '/home/travis/build/digsjs/digs-local/node_modules/.bin/istanbul'
File exists: ../istanbul/lib/cli.js
Move it away, and try again. 
npm ERR! Please include the following file with any support request:
npm ERR!     /home/travis/build/digsjs/digs-local/npm-debug.log

Part of the selling point of this package is the scriptPath option, which should allow me to use whatever istanbul-compatible coverage tool I want. If istanbul as a peerDependency implies istanbul must be installed, which is plainly not true.

I could go either way with mocha being a peerDependency. If users were allowed to specify Mocha's executable in addition to Istanbul's, then it shouldn't be.

I'm happy to restore grunt as a peerDependency in #40. This is, after all, a grunt plugin. It's not a mocha plugin nor is it an istanbul plugin...

[boneskull]

@voxpelli It almost sounds like what you want is optionalPeerDependencies 😄

[voxpelli]

@boneskull The real solution would be the replace or provide that eg. Composer supports: https://getcomposer.org/doc/04-schema.md#replace and https://getcomposer.org/doc/04-schema.md#provide

Only that one can one package really tell that it provides the same API as another real or virtual package (virtual packages being packages that has no implementation itself but rather fully relies on other packages to provide the implementation of the agreed API, as I understand it)

I don't think there's any plan for such an addition to npm though.

And speaking of optionalDependencies – they're not that optional as far as I understand. They will always be installed unless the installation fails for some reason, like eg. the current platform not being supported for a native module. So it's kind of hard to provide choice currently even if one uses something like that – considering the current logic of "optionality".

Anyhow – adding grunt sounds like a good minimal.