davideicardi / live-plugin-manager

Plugin manager and installer for Node.JS
MIT License
225 stars 43 forks source link

fix: circular dependencies result in endless loop + out of memory crash #54

Open petermetz opened 2 years ago

petermetz commented 2 years ago

I tried installing a plugin that has es6-iterator as a dependency which has es5-ext as a dependency which has es6-iterator and so there's a cycle in the dependency graph that I think live-plugin-manager needs a circuit breaker for. It might be that some additional things are needed to make sure it is handled correctly, if anyone has any information about how npm/yarn handle these cases (on top of having the circuit breaker) that would be great to know!

davideicardi commented 2 years ago

Circular requires should be supported, but not sure about circular dependencies. From high level point of view it sounds strange... but probably in someway we can support it.

AS a workaround you can probably install es6-iterator and es5-ext manually and then mark them as a static dependencies? (see staticDependencies options)

petermetz commented 2 years ago

@davideicardi

Circular requires should be supported, but not sure about circular dependencies.

Fair point IMO. The only reason why I'd still consider dealing with it due to wanting to be consistent with the other package managers (both npm and yarn are able to install these dependencies). Of course I also understand 100% that if being consistent in all fronts like that is not part of the vision of the project.

AS a workaround you can probably install es6-iterator and es5-ext manually and then mark them as a static dependencies? (see staticDependencies options)

I had to go another route so I'm unable to test it anymore unfortunately. (I'm using npm's --prefix parameter to install plugins "somewhere else on the file-system")
Based on the above I'm also okay with just closing this of course, if no one else had this issue and I've moved on then I don't want to take up your time!

davideicardi commented 2 years ago

For me we can leave the issue open for the future, so I can monitor it.