nglviewer / ngl

WebGL protein viewer
http://nglviewer.org/ngl/
MIT License
656 stars 168 forks source link

Should threejs be changed to a peer dependency? #1021

Open luxaritas opened 4 months ago

luxaritas commented 4 months ago

728, #755, and related issues discuss the use case of using NGL in downstream projects that also need to use threejs as a dependency, eg because NGL is embedded into an existing threejs app or a downstream app wants to extend NGL somehow (eg, adding a custom representation). To further support this use case, I'm wondering if it would make sense to make threejs a peer dependency instead of a regular dependency. This would ensure that only one copy on threejs is in play, and its version is always synced with the one required by NGL.

papillot commented 4 months ago

Honestly, I don't know.... I've spent lots of time recently trying to migrate the Three.js version (still an ongoing project) and realized that this library makes breaking changes in the context of NGL every 5-10 minor version bump.

In your experience, how using peerDependencies instead of dependencies will help solve the problem you are describing? A user who needs an updated version of Three.js, can they make it work with NGL if we are using peerDependencies?

luxaritas commented 4 months ago

In your experience, how using peerDependencies instead of dependencies will help solve the problem you are describing?

Right now, since I have lint rules set up forbidding importing dependencies that are not in my package.json, I have to add three as a dependency to my package.json. If NGL upgrades its three version and I don't, there will be two conflicting versions of three installed at use at the same time. Same if I accidentally change the version and get out of sync with NGL. Making it a peer dependency would enforce that if I add it to my package.json, it has to use the same version of three as NGL, preventing the two from getting out of sync.

A user who needs an updated version of Three.js, can they make it work with NGL if we are using peerDependencies?

That user shouldn't be using an updated version of three since it is liable to break NGL though, right? Or are you arguing for the case of the user creating a separate three app that never interacts with NGL? In the latter case I suppose that would be an issue.

I suppose you could make an argument that you should just import ngl's version without adding it to the main package.json, and in my case that just means adding lint rule exclusions. Though it could still create problems if another dependency is used which also has three as a dependency or peer dependency of a conflicting version