Get rid of peer dependency on loopback-datasource-juggler

[bajtos]

LoopBack has a peer dependency on loopback-datasource-juggler. As explained in an older comment:

Before peerDependency is adopted, we end up with the following tree:

  myapp 
    --> loopback --> loopback-datasource-juggler
    --> loopback-connector-mongodb --> loopback-datasource-juggler

Having multiple instances of juggler is a problem because then there are multiple instances of the global Model registry.

I have looked at several connectors (mysql, postgresql, oracle, rest, soap) and found the following usages of loopback-datasource-juggler:

• Mysql connector calls juggler.ModelBuilder.registerType to register a custom type Point. The call is made from initialize.
• All SQL connectors are extending juggler.BaseSQL

---

I am proposing to make the following changes:

• Move BaseSQL and Connector (which is super class of BaseSQL) to a new module (e.g. loopback-connector-base or loopback-connector). Modify connectors to use this new module instead of juggler.
• To preserve backwards compatibility, juggler should probably require loopback-connector-base and expose BaseSQL and Connector classes. This can be removed in the next major version.
• Modify the signature of initialize function from initializeDataSource(dataSource, callback) to initializeDataSource(dataSource, ModelBuilder, callback).

The new dependency graph would look like this:

myapp 
  --> loopback --> loopback-datasource-juggler -> loopback-connector-base
  --> loopback-connector-mongodb --> loopback-connector-base

/to @ritch @Raymond what's your opinion on this? I am happy to implement these changes once I get green light from you.

Note: if we are going to release juggler 2.x then it should wait until this change is made, as it allows us to drop juggler dependency on connector base.

---

Connector check list:

• mysql (v1.3.0)
• mongodb (v1.3.0)
• rest (v1.2.1)
• soap (v1.1.0)
• postgresql (v1.1.0)
• oracle (v1.4.0)
• mssql (v1.1.0)
• kafka - see Remove peer dep on loopback haio/loopback-connector-kafka#1
• neo4j
• Incorrect dependency on juggler and strong-remoting loopback-connector-remote#5

[sam-github]

Best fix is for the model registry to be provided to a connector/juggler when it is instantiated, so there is no need to mis-use peer deps to attempt to guarantee singleton-ness of the juggler (this doesn't work under a number of conditions).

That's an API change, though. For an API compatible fix, I'd suggest doing something like this:

in juggler:index.js:

if(global.JUGGLER) {
if(global.JUGGLER.version sem-compat with require(./package).verssion) {
exports = global.JUGGLER
return;
} else {
error incompatible juggler versions in use
}

... normal export setup

globals.JUGGLER = export;

[raymondfeng]

I believe the ultimate fix is Dependency Injection :-).

Thanks,

---

Raymond Feng
Co-Founder and Architect @ StrongLoop, Inc.

StrongLoop makes it easy to develop APIs in Node, plus get DevOps capabilities like monitoring, debugging and clustering.

On May 22, 2014, at 11:18 AM, Sam Roberts notifications@github.com wrote:

Best fix is for the model registry to be provided to a connector/juggler when it is instantiated, so there is no need to mis-use peer deps to attempt to guarantee singleton-ness of the juggler (this doesn't work under a number of conditions).

That's an API change, though. For an API compatible fix, I'd suggest doing something like this:

in juggler:index.js:

if(global.JUGGLER) {
if(global.JUGGLER.version sem-compat with require(./package).verssion) {
exports = global.JUGGLER
return;
} else {
error incompatible juggler versions in use
}

... normal export setup

globals.JUGGLER = export;

—
Reply to this email directly or view it on GitHub.

[bajtos]

@sam-github Best fix is for the model registry to be provided to a connector/juggler when it is instantiated, so there is no need to mis-use peer deps to attempt to guarantee singleton-ness of the juggler (this doesn't work under a number of conditions).

@raymondfeng I believe the ultimate fix is Dependency Injection

Yes, that's what I am intending to implement.

[bajtos]

I have made the required changes in loopback-connector, loopback-datasource-juggler and loopback-connector-mysql - see the three pull requests listed above this comment.

@raymondfeng or @ritch could you please review?

I'll modify the remaining connectors after we agree on the right implementation.

Note: the change in loopback-datasource-juggler is breaking the API. That should be fine, since we are going to release 2.x soon. However, preserving backwards compatibility is pretty easy. I can modify the PR if we think it's worth it.

[bajtos]

I am closing this issue as done, even though the kafka connector was not updated yet (see the pending pull request haio/loopback-connector-kafka#1). Since the code in kafka connector does not depend on anything from loopback nor loopback-datasource-juggler, its peer dependency on loopback is not a major problem.

[bajtos]

I have reopened the issue. All ground work has been done, but we still need to make the change in loopback's package.json (few minutes of work) and perform a thorough test (that needs more time).