davideicardi / live-plugin-manager

Plugin manager and installer for Node.JS
MIT License
233 stars 44 forks source link

Dependencies resolve issue #21

Closed shlomisas closed 4 years ago

shlomisas commented 4 years ago

Hi,

I have this scenario: a module/plugin with two dependencies that uses a different (major) versions are override each other and potentially throw an error if major functionality changed.

In my case is jszip (dep of selenium-webdriver) and winston-transport (that is a dep of winston) and winston itself (see below npm ls output)

When Im trying to install my plugin everything is good, but when I tried to require using live-plugin-manager I'm getting the following error:

Cannot find ./writable in plugin readable-stream

I started to debug live-plugin-manager and looks like it requires readable-stream 3.6.0 even for modules that has a different (major) version of this package and it's clear why it's broken now, in 3.6.0 they removed the writable.js file from the root of the module while others still using it.

The output of npm ls readable-stream is this:

$ npm ls readable-stream
package@1.0.0 C:\personal\dev\github\live-plugin-manager-bug-scenario\package
+-- UNMET DEPENDENCY selenium-webdriver@4.0.0-alpha.7
| `-- UNMET DEPENDENCY jszip@3.4.0
|   `-- UNMET DEPENDENCY readable-stream@2.3.7
`-- UNMET DEPENDENCY winston@3.2.1
  +-- UNMET DEPENDENCY readable-stream@3.6.0
  `-- UNMET DEPENDENCY winston-transport@4.3.0
    `-- UNMET DEPENDENCY readable-stream@2.3.7

Here is a POC of my problem: https://github.com/shlomisas/live-plugin-manager-bug-scenario

It sounds quite basic to support multiple versions of the same package, so I wonder: do I do something wrong here?

TU,

Shlomi.

davideicardi commented 4 years ago

Yes, unfortunately for now this scenario is not supported. The problem is that modules are installed just by name, so I can have just one version of it. It is a know limitation and something that I want to change, I have also started working on it some time ago but never finished.

This is already an issue and a draft branch, see #3

davideicardi commented 4 years ago

I'm closing this issue just because it is a duplicate of #3, if you think it is different or if you have something to add feel free to reopen it.