Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use peerDependencies for baconjs #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ahdinosaur
Copy link
Contributor

hi!

peerDependencies help when multiple plugins want to play together. without peerDeps, it's likely each plugin would have it's own Bacon instance, which is bad.

cheers :)

@raimohanska
Copy link
Contributor

Oh, this seems like a good thing. I'll come back to this when I'm done with Hello World Open :)

@raimohanska
Copy link
Contributor

Hi! Tried to reproduce the issue of "multiple bacons" today, but got stuck with NPM problems.

Current status: created https://github.com/raimohanska/bjq-client-test which fails to install because npm insists on installing all teh devDependencies of bacon.jquery. Don't know why. Can't fix right now. Any help appreciated. This seems related to npm/npm#4660

Btw, Getting stuck with NPM problems seems to be a trend for me nowadays :(

Update: removed npm-shrinkwrap.json from bacon.jquery and now the install problems are gone. So next I'll be back to trying to reproduce the double-bacon issue.

@lautis
Copy link
Member

lautis commented Jun 12, 2014

NPM team has some doubts about usefulness of peerDependencies issues and instead recommend using npm dedupe to avoid multiple copies of a library.

The behaviour of peerDependencies has been changed and they are not anymore installed automatically on npm install. That would make using bacon.model slightly annoying as you'd also have to specify baconjs as dependency even if you didn't directly use any of its functionality.

@DylanLukes
Copy link

I'm not sure if it's more relevant to this issue, or merits its own... But I would suggest exporting the init function rather than applying it to require('baconjs') in Node contexts.

As it stands, it's nearly impossible without modification to use both of, say, bacon.model and bacon.jquery. They just both import a fresh copy of bacon respectively. I'd say exposing the init method (and renaming it to patch or mixin) is a better course.

Generally speaking, automatically patching existing objects on require/import is a bad idea. These semantically include exported new variables, not extend existing ones. That should be the user's responsibility.

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

Successfully merging this pull request may close these issues.

4 participants