qooxdoo / qooxdoo-compiler

Compiler for Qooxdoo, 100% javascript
MIT License
36 stars 23 forks source link

dependency infection #344

Closed oetiker closed 5 years ago

oetiker commented 5 years ago

Installing eventrecorder contrib v0.9.0 causes all the dependencies of the eventrecorder contrib to be listed in the requirements section of the applications own Manifest.json ... that seems odd as the dependencies of eventrecorder should be its own and not be transmitted up to my application.

cboulanger commented 5 years ago

Absolutely true, this is a bug. @hkollmann we need to look at this.

oetiker commented 5 years ago

it will become more of an issue as further dependencies are introduced ... this will also eventually raise the problem of incompatible versions as qooxdoo does have a global namespace for its classes as far I understand the system

cboulanger commented 5 years ago

So far, with so few contribs, it doesn't matter much, but it will become a real problem that we need to address. NPM can deal with it because there is no global namespace and you can use different versions of the same library since libraries are imported as local symbols. That is a design problem of Qooxdoo itself (here, the library shows its age). Maybe we should look into switching to a module design for qx v7, now that it is supported by all major browsers. But that's a different issue. In the current situation, we might have to resort to enforcing matching version numbers. This is a pain and obviously can lead to dependency hell, but probably the only solution.

johnspackman commented 5 years ago

It has to be enforced version numbers, AFAICR that was the premise in which we entered into this - I'm sure we discussed it at the time?

While it does complicate matters when (if) you want to fork a repo, the simplest (and easiest to understand and debug) is to enforce a flat list of dependent libraries, each with a specific version number.

If the compiler detects incompatible library requirements (or duplicate namespace/classes) this is something that the developer can fix by manually checking out and/or editing compile.json.

I don't see huge a problem with this approach, because IMHO it often does not make sense to have multiple versions of the same contrib (ie as with npm modules) and if there is weirdness/compatibility issues we're limited in what we can do. Certainly I don't think we should waste much time on anything other than a dead-simple, list of libraries and version numbers until we have a bigger eco system (and released 6.0)

oetiker commented 5 years ago

by all means, I was not suggesting to 'solve' the problem of having multiple versions of a contrib in the same project ... but I think it would be great if the compiler complained when the requirements were incompatible with each other.

johnspackman commented 5 years ago

@oetiker sure, sorry I wasn't responding to your original issue (agreeing with it really), just expressing the need to steer away from non-trivial solutions by sticking with a flat list and simple semver tests.

cboulanger commented 5 years ago

This also concerns contrib.json: we get all libraries imported here, even if the Manifest of the importing libraries specifies just one or two of them.