ioBroker.js-controller icon indicating copy to clipboard operation
ioBroker.js-controller copied to clipboard

RFC: Install user-defined dependencies centrally, instead of letting adapters handle them

Open AlCalzone opened this issue 1 year ago • 9 comments

"Manually" installing dependencies in a sub-package of our "install" project (/opt/iobroker/package.json) once worked fine, but increasingly collides with what npm does while managing the dependencies. We already stopped doing a separate npm install inside the adapter directories in https://github.com/ioBroker/ioBroker.js-controller/pull/1339, but some adapters still do this, leading to weird issues.

I therefore propose we get rid of this too and do the installation in a central location, adding the dependencies to our root package.json. To avoid conflicts, we need to fake-namespace these packages, which npm supports using the npm: protocol. Npm supports this since 6.9.0 (https://github.com/npm/cli/pull/3):

// package.json
"dependencies": {
  //... all adapters etc., as usual
  // namespaced user-defined dependencies of iobroker.javascript, instance 0:
  "@iobroker-javascript.0/axios": "npm:[email protected]", // Here javascript.0 has [email protected] as its dependency
  "@iobroker-javascript.0/alcalzone-pak": "npm:@alcalzone/[email protected]", // scoped dependencies need to be escaped. I propose omitting the leading @ and replacing / with - (_ would also be fine)
  // ... likewise for other adapters/instances
}

This has the downside that the adapters need to be able to redirect the require to the correct package name, e.g. javascript.0 would have to require @iobroker-javascript.0/axios instead of just axios, but we can expose a utility for this too.

The upside would be that versioning these dependencies would become a bit more manageable, and another adapter installing the same one would not accidentally cause a conflict. This way we can also support multiple instances of javascript have different versions of the same package, since the fake scopes are instance-specific. In addition, maybe the adapters wouldn't even have to store their dependencies separately anymore, but could rather enumerate them from the root package.json.

AlCalzone avatar Sep 20 '22 12:09 AlCalzone

I'm not sure how this would work for node-red ... here we have no control over the application itself and we only start it as extra process

Apollon77 avatar Sep 20 '22 14:09 Apollon77

see https://github.com/ioBroker/ioBroker/issues/512 : When we change this we also need to prevent concurrent runs.

Other question: can we not define an own "npm root folder "for each adapzter and see keep these adapter relevant npm trees separated and make (somehow ;-) ) sure that the relevant local stuff uses the special ones instead of the omnes fropm adapte rby somehow manipulating resolve logic?

Apollon77 avatar Mar 21 '24 12:03 Apollon77

see ioBroker/ioBroker#512 : When we change this we also need to prevent concurrent runs.

Other question: can we not define an own "npm root folder "for each adapzter and see keep these adapter relevant npm trees separated and make (somehow ;-) ) sure that the relevant local stuff uses the special ones instead of the omnes fropm adapte rby somehow manipulating resolve logic?

So basically the proposal from Dominic but each adapter has a separate folder? What should the advantage be compared to the proposed solution? Reducing risk of conflicts? Still with two js instances, they have the same folder or you mean one "root" folder per instance?

foxriver76 avatar Mar 21 '24 13:03 foxriver76

I would mean one per instance to really separate them ... in theory one JS adapter could use [email protected] and a second instance [email protected] ... what to access? How that is handled correctly? ;-) I know edge cases, but if we now plan to restructure it then why not in such a way - amd I think node-rted also actually does exactly this

Apollon77 avatar Mar 21 '24 13:03 Apollon77

indeed sounds reasonable, need to play around a bit when time has come. For sure want to have this in next controller

foxriver76 avatar Mar 21 '24 13:03 foxriver76

npm root folder "for each adapzter

Soo, use npm workspaces and have each adapter installation be a workspace?

AlCalzone avatar Mar 21 '24 15:03 AlCalzone

I think what @Apollon77 wanted to achieve is separated packages for each adapter to manage the dependencies of this instance, so no global workspace root which would then conflict again, but depending on what npm does internally this is likely not that optimal.

So as I understood e.g. like /opt/iobroker/node_modules/iobroker.javascript.0.dependency-mgmt then place package json there with the packages installed by javascript instance 0 and so on. Also with the approach like from initial post

// package.json
"dependencies": {
  //... all adapters etc., as usual
  // namespaced user-defined dependencies of iobroker.javascript, instance 0:
  "@iobroker-javascript.0/axios": "npm:[email protected]", // Here javascript.0 has [email protected] as its dependency
  "@iobroker-javascript.0/alcalzone-pak": "npm:@alcalzone/[email protected]", // scoped dependencies need to be escaped. I propose omitting the leading @ and replacing / with - (_ would also be fine)
  // ... likewise for other adapters/instances
}

foxriver76 avatar Mar 25 '24 12:03 foxriver76

Putting custom folders inside node-modules is just asking for trouble. Besides I don't know how we would even tell npm to install there.

If we changed the installation method so that the root directory was a workspace root and each adapter a workspace, then npm should automatically install the dependencies in the correct location. But this would mean we can't just npm install iobroker.adaptername in the root, which is probably too breaking.

I think the idea from the OP is the most compatible one, even if it's a bit weird.

AlCalzone avatar Mar 25 '24 14:03 AlCalzone

Yes I am with you initial approach is best.

foxriver76 avatar Mar 25 '24 15:03 foxriver76