Skip to content
This repository has been archived by the owner on Dec 28, 2023. It is now read-only.

drop Mocha peer dep? #114

Closed
boneskull opened this issue Sep 19, 2016 · 6 comments
Closed

drop Mocha peer dep? #114

boneskull opened this issue Sep 19, 2016 · 6 comments

Comments

@boneskull
Copy link
Contributor

karma-mocha won't work without Mocha, of course.

But it won't work without Karma, either.

The idea behind peer dependencies is to help support "plugins". karma-mocha is most certainly not a Mocha plugin. So if anything, Karma should be the peer dependency, right?

But a peer dependency is just a static check, really. I'm no big fan of them. Instead of this, what if karma-mocha had a runtime check for a mocha package? Maybe it already does?

Would you accept a PR that drops the peer dep and/or checks for Mocha at runtime?

(Backstory: Mocha is consuming Karma and karma-mocha to run its own tests. Please see mochajs/mocha#2487 for current state of affairs, and how we're trying to work around this.)

@maksimr
Copy link
Contributor

maksimr commented Sep 19, 2016

@boneskull karma should be in peer dependencies too like in karma-jasmine

But a peer dependency is just a static check, really.

Yes

I'm no big fan of them. Instead of this, what if karma-mocha had a runtime check for a mocha package?

Why? Why we should add additional code to karma-mocha if this already implemented in npm?

Thanks!

@boneskull
Copy link
Contributor Author

boneskull commented Sep 19, 2016

Why? Why we should add additional code to karma-mocha if this already implemented in npm?

Because the implementation of peer deps is problematic. There's a reason npm v3 reduced unmet peer deps to a warning. Users of npm v3 would have to npm install mocha anyway, so why not change the install instructions in README to something like npm install karma karma-mocha mocha?

FWIW, the code would amount to:

try {
  require('mocha');
} catch (err) {
  throw new Error('karma-mocha requires Mocha; install via "npm install mocha"');
}

The point of this ticket is that I can't use karma-mocha to test Mocha itself without either a) forking karma-mocha or b) really lame workarounds. Dropping the peer dep would solve this problem, at little cost to other users.

@maksimr
Copy link
Contributor

maksimr commented Sep 19, 2016

@boneskull thanks, now I understand your pain but how you will resolve problem with require.resolve('mocha')?

@boneskull
Copy link
Contributor Author

NODE_PATH=. karma start

@maksimr
Copy link
Contributor

maksimr commented Sep 20, 2016

@boneskull ok, I have no reason why we can not remove peer dependency.

boneskull added a commit to boneskull/karma-mocha that referenced this issue Sep 27, 2016
…ner#114

- this is a breaking change and should probably be released as v2.0.0
@boneskull
Copy link
Contributor Author

@maksimr ping on #116

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants