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

about maintenance and peerDependencies #17

Closed
fibo opened this issue Jul 14, 2016 · 4 comments
Closed

about maintenance and peerDependencies #17

fibo opened this issue Jul 14, 2016 · 4 comments

Comments

@fibo
Copy link

fibo commented Jul 14, 2016

Hi,

I had the same idea about packaging OrbitControls, and I created https://github.com/fibo/three-orbitcontrols, then I discovered this package (I could not find it on npm at a first search).

I have just copyed the original module, imported three.js, and exported it using commonjs. Since I have done only the minimum required to make it importable, I think it will be easier to update it, in fact three.js is now at r78 and, in particular, OrbitControl.js was updated 2 weeks ago mrdoob/three.js@e51d15c

I used peerDependencies, instead of passing three.js in the exported method, cause I think it will be easier to keep the package updated.

Do you think it is a good approach? If yes, I could do a pull request here, so we could keep only one package, instead of publish mine on npm.

Tell me what do you think, if you consider I can collaborate.

@bsergean
Copy link
Collaborator

My opinion is that only one package on NPM is the right thing to do to avoid confusion. I was given ownership rights but unfortunately I'm not doing three.js / js (was doing coffee script !) daily.

@mattdesl is the real owner of this package so I'll leave it up to him.

@fibo, my take on this is that if you are motivated and ready to keep the package up to date, you should become one of the owner of this git repo.

@fibo
Copy link
Author

fibo commented Jul 15, 2016

@bsergean it is ok for me to maintain this package, but, only if its real development is done by original three.js contributors and this package do the minimum modifications to put it on npm.

Then there is a thread about three.js modularization (since 2014 :) and when it ends, my proposal is to give back this package to mrdoob, since maybe he will want to manage it with lerna or some other tool.

@mattdesl if this roadmap is ok for you, I can do the rest and keep this package updated, otherwise I will proceed with the one I created. I prefer the first option, please let me know.

@mattdesl
Copy link
Owner

Both modules handle the THREE global differently. With modular THREE right now there's are a few ways as I see it, with their own pros/cons:

  • Direct dependency on three — not a good solution IMHO, will lead to version woes for the end-user and expects three.js to be part of the bundle
  • Peer dependency on three — can avoid some versioning woes but peer deps are never really good, they cause all sorts of headaches and confusion for many end-users, and ultimately don't solve a whole lot
  • Wrapping things in a closure and forcing the user to pass their own THREE instance — i.e. what this module does. It provides the most flexibility since it doesn't assume THREE is in global scope, and it works whether or not the library is in your bundle or a separate script tag. It's a bit cumbersome to type and harder to pull upstream updates
  • Just expecting THREE to be globally accessible, and not handling version differences. This is probably the easiest way to keep the library updated but also forces the end-user to structure their code in a specific way (including THREE in global scope before requiring modules like this)

It's hard to say which is best, and maybe having multiple modules to choose from isn't such a bad thing for end-users.

@fibo
Copy link
Author

fibo commented Jul 17, 2016

I agree, so I will use peer deps on a separate package, maybe including all controls.

It is true that peer deps can lead to some headache cause they are not installed automatically, but, usually a user that requires Orbit control already has three.js as a dependency. I will keep updated the required version with the upstream and I need really few modifications (which probably I can automate) to make the module exportable, so I think using peer deps is ok.

Using a closure is a nice solution, and I wanted to use this package, but it is not updated to latest version.

Anyway, I think you should not fix bugs on Orbit control issues, but, update from the upstream and invite users to open an issue on three.js official repository, you are doing a task twice.

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

3 participants