BUG: Singleton (backbone) instances resolved twice

[creynders]

If I'm not mistaken Resolver#resolve is called twice when a singleton instance is created by the resolver.

All singleton constructors are wrapped with _wrapConstructor (which in turn calls context.resolve on the created instance), but _createAndSetupInstance also calls context.resolve on the created instance.

If I'm not mistaken the _wrapConstructor call for singleton classes could be dropped? I think it only makes sense to have it for View classes, no?

[creynders]

Yep, the test confirms the bug. More specifically: when a backbone object (or any object having extend and initialize methods) is wired as a singleton and instantiated its dependencies are resolved twice.

[creynders]

So @geekdave what do you think of

If I'm not mistaken the _wrapConstructor call for singleton classes could be dropped? I think it only makes sense to have it for View classes, no?

Or am I missing something here?

[mmikeyy]

Hmm.. if the call to _wrapConstructor is eliminated for a singleton, then wiring declared in singleton classes will have no effect?

In the application I'm currently converting to Geppeto, I pick an item in a menu, and this puts the selected item's model in a wired object.

In every module called from this module, I just have to wire to the object (always reachable through a chain of parent contexts whose length varies with the depth of the menu structure) to get the chosen item's model, without having to bother carring the info in payload between modules. (once parent contexts are fixed (PR coming, if I can solve my issues with testing...), this works like a charm)

Now one of the sub modules processes item details. So I create a collection as a singleton, and this collection must know what item it pertains to. That's an example of where wiring a singleton is useful.

The detail view is wired to the collection, which is itself wired to the select item model. So I don't have to worry about setting item number in the collection after it's created because it's been injected with wiring (thanks to the wrapped constructor).

Anyway, perhaps I'm the one totally missing something, but apparently the removal of the _wrapConstructor function might break this logic and just make additional coding necessary to compensate. Just my 2 cents!

[creynders]

The _wrapConstructor is only necessary for items that are returned as 'classes', i.e. as constructor functions, from the resolver, which is only the case with views. So no, the removal of _wrapconstructor for singletons won't break their desired behaviour.

[geekdave]

LGTM - Thanks @creynders !

@mmikeyy - Let me know if this gives you any trouble, but it seems that injection into singleton classes still works just fine (but it only happens once instead of twice now)

[creynders]

Aaaaaaaaaaaargh. Just realised there is a problem. If the singleton is a Backbone object with an initialize method, then it can't use any of its dependencies in it, since here the initialize method is called upon instantiation, right before its dependencies are resolved.

I'll look into whether the solution might be to instead wrap every constructor in case they do have a initialize method and this.resolve(instance, wiring); is only called if they don't have an initialize method.

[creynders]

Sorry 'bout the above. Due to the branch already being merged in it created new hashes for commits already merged in, so I deleted the original fix_51 branch, created a new one with its own PR #59 containing only the commit that should fix the above mentioned problem.

I.e. if you accept 59 you'll have a clean history even though it doesn't look like it here.