search

sciactive / pnotify

Beautiful JavaScript notifications with Web Notifications support.
https://sciactive.com/pnotify/
Apache License 2.0
3.65k stars 514 forks source link

I think material-design-icons should be removed from peerDependencies (48 MB) #348

Closed bigfarofa closed 5 years ago

bigfarofa commented 5 years ago

Thanks for the plugin,but I think it would be better if material-design-icons were not a dependency. It should be optional.

hperrin commented 5 years ago

peerDependencies are optional.

hperrin commented 5 years ago

I think the problem is that, semantically, peerDependencies is not exactly where they should be. But they also don't belong in optionalDependencies. The problem is that there is no way in package.json to specify "this package has a feature that only works when package x is installed". peerDependencies basically means "this entire package doesn't work unless package x is installed", and (IIRC) optionalDependencies basically means "this package works fully without package x, but it can take advantage of it if it's installed".

The reason I think peerDependencies fits for PNotify is that if a package from there is installed, but it's the wrong version, it will throw an error. If a peer dependency is not installed at all, it will only be a warning. Whereas optionalDependencies will not warn the user at all, and they may wonder why those features aren't working.

I suppose one could make the argument that this ties a dependent package into only certain versions of the peer dependencies even if they aren't using the PNotify features. I'll move these into optionalDependencies for the beta and see if this causes any confusion. If people don't have an issue with it, I can leave them there.

hperrin commented 5 years ago

Seems like there's no way to get NPM to not install optional dependencies without the --no-optional flag. I think this is actually better.