node-markdown-spellcheck icon indicating copy to clipboard operation
node-markdown-spellcheck copied to clipboard

Sinon as promised should be a dev dep not an actual dep

Open halkeye opened this issue 6 years ago • 4 comments

It might also be worth hooking up https://www.npmjs.com/package/eslint-plugin-import but it doesn't catch anything in this case cause its a prod dep.

halkeye avatar Jun 22 '19 20:06 halkeye

Looks like appveyor's version is checking graceful-fs maybe? I don't think its an actual error

halkeye avatar Jun 22 '19 20:06 halkeye

I would like this to be merged too, I'm kind of strict but I don't want to have a warning about unsatisfied peer dependencies:

npm WARN [email protected] requires a peer of sinon@1 but none is installed. You must install peer dependencies yourself.

Some arguments as to why this should be merged:

  • Causes unnecessary noise for anyone not using sinon as testing framework.
  • Causes 9 additional packages to be installed, each which may potentially have security issues and will increase installation time.
  • Sinon 2.0.0 (released 2017-03-15, current version 7.5.0) makes this package unnecessary, even the package itself states this:

Sinon 2 added resolves and rejects methods and no longer requires this library.

As for now I will refrain from using this package sadly (because it would really help me) but I'm strict about pulling packages causing these issues. Bit of a shame since it is such an easy fix.

ext avatar Dec 22 '19 14:12 ext

Ping @lukeapage

@ext you can always fork and deploy it to your own namespace on npm so you can easily use it

halkeye avatar Dec 22 '19 19:12 halkeye

Of course, but that puts a burden on me to keep it up-to-date with updates, security advisories, etc. So in practice I don't think I'd be doing anyone a favor by forking.

I really hope @lukeapage will get around to merging or otherwise solving this.

ext avatar Dec 22 '19 21:12 ext