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

react-loadermixin needs to be a regular dependency, not devDependency #13

Closed
Pomax opened this issue May 5, 2015 · 5 comments
Closed

Comments

@Pomax
Copy link

Pomax commented May 5, 2015

react, react-loadermixin and xtend are required by the imageloader code, and should be normal dependencies, rather than devDependencies.

@lettertwo
Copy link
Contributor

Hmm, well xtend is a dependency, while both react and react-loadermixin are peer dependencies. What specific problem have you run in to? I don't think we can make react a hard dependency, since multiple versions of react cannot co-exist (see this also) in the same app.

@Pomax
Copy link
Author

Pomax commented May 5, 2015

For react, that's true, good point. But, for react-loadermixin, I don't think it is. By its very nature people are already going to have react installed, but loadermixin is super specific and none of the React projects I'm working on/with at the moment make use of it, only your mixin, so "npm install react-imageloader" yields a "broken" mixin without manually installing a mixin that serves no purpose other than to make yours work. I'd still recommend moving the loadermixin to the regular dependency list, instead.

@lettertwo
Copy link
Contributor

Interesting. AFAIK npm should auto-install peer dependencies if they are missing (at least, until 3.x is released). Is it possible you're running an edge version of npm?

@Pomax
Copy link
Author

Pomax commented May 5, 2015

hm, deleted node_modules, pruned package.json, reran it, and you're right: it installed just fine. I guess that means this can be closed, sorry to bother and thanks for the reponse!

@Pomax Pomax closed this as completed May 5, 2015
@lettertwo
Copy link
Contributor

👍 thanks for providing the sanity check! We may still have to do something when the next version of npm is released, since peer dependencies will stop auto-installing then…

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

No branches or pull requests

2 participants