Depended module config() and run() blocks being executed multiple times

[duncangrist]

I've been playing around with ocLazyLoad for a while now and I've noticed something that I didn't expect.

It seems that if you register a module statically as a dependency on your application module, and then add a dependency to that same module from a lazy-loaded module, the depended module essentially runs twice; it's config() and run() blocks execute each time you lazy load a module which depends on it.

Is this expected behaviour?

Plunkr which reproduces the issue - observe the console.log output and then click the button.
http://plnkr.co/edit/IKVZAB7omm2nlmpsK96a

Note that I'm not setting the reconfig flag...

[ocombe]

If you don't use reconfig, it definitely shouldn't run those blocks twice.
Thanks for the plunkr, it will help.

I'll take a look at it next week (sorry I'm at Paris for ng-europe, so I
can't take a look at it before).

[duncangrist]

Thanks for your quick response!

I've managed to distill my issue down to such a simple plunkr that I've been convinced for ages that I must be doing something wrong.

Anyway, enjoy ng-europe :)

[duncangrist]

I can fix this issue by changing the following code:

var newModule = regModules.indexOf(moduleName) === -1;
moduleFn = angular.module(moduleName);
if(newModule) { // new module
    regModules.push(moduleName);
    register(providers, moduleFn.requires, params);
    runBlocks = runBlocks.concat(moduleFn._runBlocks);
}
invokeQueue(providers, moduleFn._invokeQueue, moduleName, params.reconfig);
invokeQueue(providers, moduleFn._configBlocks, moduleName, params.reconfig); // angular 1.3+

to this

var newModule = regModules.indexOf(moduleName) === -1;
moduleFn = angular.module(moduleName);
if(newModule) { // new module
    regModules.push(moduleName);
    register(providers, moduleFn.requires, params);
    runBlocks = runBlocks.concat(moduleFn._runBlocks);
    invokeQueue(providers, moduleFn._invokeQueue, moduleName, params.reconfig);
    invokeQueue(providers, moduleFn._configBlocks, moduleName, params.reconfig); // angular 1.3+
}

which leads me to ask, why would you want to re-run the _invokeQueue or _configBlocks on an already loaded module anyway? I'm sure there's a reason, or you wouldn't have introduced the reconfig parameter!

Incidentally, I was also having the same issue as detailed here: #58 (directives firing twice), and the above change stopped that from happening too.

[ocombe]

Initially it was made this way because you don't know the dependencies until you analyze the invoke queue.
Let's say that you add a new service to an existing module, and that this service requires new uninvoked services, we need to run the invoke queue to make sure that those services are instantiated.
Now that I try to explain it, I'm thinking that this might not be a real issue, and that angular will instantiate the service when it is needed, not in the invoke queue.
I need to recheck this and see if I extrapolated and imagined behaviors that aren't really needed.
I'll let you know what I find.

PS: I really need to add unit tests for all those special cases because I keep forgetting them (or add more comments)

[ocombe]

Fixed in 0.3.9, thanks for the plunkr it helped me narrow the problem down to a simple coding mistake that led to big consequences: the app module was not detected and its dependencies (it includes the config/run blocks) were not listed as already loaded !

[duncangrist]

Thanks for the fix, it does indeed fix the issue in the Plunkr. Unfortunately I don't think it solves #58, at least for me.

I will try and create another plunkr encapsulating the problem that I'm seeing, but I think it reaches beyond simply having directives firing more than once. I think it's a fundamental problem with having behaviour that reruns the invoke queue and config blocks, even if angular has already loaded the module (and would have therefore already run these).

I say this because when I apply my diff above, which explicitly doesn't rerun the config blocks or invoke queue if angular has already registered the module, it solves both #77 and #58 for me.

[ocombe]

The problem from #58 that is not solved with this fix is " Multiple directives [uiInputText, uiInputText] asking for template on: < div ................ >" ?
Any chance to have a plunkr for that problem ? :)

[duncangrist]

Yes, that's the one - multiple directives (of the same name) trying to attach to the same element.
In my app it's being caused by the fact that the module that registers the directive is being invoked multiple times and therefore the same directive name is registered multiple times and therefore different instances of the same directive are all trying to be linked to the same element.

I think that somewhere along the line, new $injector instances are being created which is allowing directives of the same name and module to coexist. This is just a hunch at this time, though.

I'm working on a Plunkr, but I'm running out of time today. It's proving hard functionality to extract because I've got ocLazyLoad plugged into a ES6 module loader in my app and I'm not sure whether that's part of the picture or not.

I have some more time to look into it on Friday.

[ocombe]

Ok no problem, take your time.

That's the kind of bug that my fix should prevent.
It is allowed by Angular to register multiple directives on the same element, because you could bind a directive on input elements for example (by naming it input) and it should work, even if there already is a native directive input in angular.
But what my fix prevents is registering the same directive from the same module twice.
That's why I don't understand why it doesn't work in your case, unless you register the same directive in different modules ? Or maybe I don't detect it as a dependency from the correct module... I'll find out once you can show me that plunkr.

[duncangrist]

Although I haven't managed to create a plunkr showing the directive issue, I have manage to get one together exhibiting an issue which I believe is related.

http://plnkr.co/edit/2ZAbNpJX2skTMblvSlCY

More information in the Plunkr itself.