marcoslin / angularAMD

Facilitate use of RequireJS in AngularJS
http://marcoslin.github.io/angularAMD
MIT License
734 stars 171 forks source link

In processQueue would be nice if first run all configs of all modules then all run blocks #119

Closed csbenjamin closed 9 years ago

csbenjamin commented 9 years ago

If I have a module A and a module B and the run block of A needs something executed in config block of B and the run block of B needs something executed in config block of A, the current angularAMD version will not work. The changes in AngularAMD.prototype.processQueue bellow fixes this

AngularAMD.prototype.processQueue = function () {
    checkBootstrapped();

    if (typeof alt_angular === 'undefined') {
        throw new Error('Alternate angular not set.  Make sure that `enable_ngload` option has been set when calling angularAMD.bootstrap');
    }

    // Process alternate queue in FIFO fashion
    function processRunBlock(block) {
        //console.info('"' + item.name + '": executing run block: ', run_block);
        run_injector.invoke(block);
    }

    for (var i=0;i<alternate_queue.length;i++) {
        var item = alternate_queue[i],
            invokeQueue = item.module._invokeQueue,
            y;

        // Setup the providers define in the module
        // console.info('invokeQueue: ', invokeQueue);
        for (y = 0; y < invokeQueue.length; y += 1) {
            var q = invokeQueue[y],
                provider = q[0],
                method = q[1],
                args = q[2];

            // Make sure that provider exists.
            if (app_cached_providers.hasOwnProperty(provider)) {
                var cachedProvider;
                if (provider === '$injector' && method === 'invoke') {
                    cachedProvider = config_injector;
                } else {
                    cachedProvider = app_cached_providers[provider];
                }
                // console.info('"' + item.name + '": applying ' + provider + '.' + method + ' for args: ', args);
                cachedProvider[method].apply(null, args);
            } else {
                // Make sure that console exists before calling it
                if ( window.console ) {
                    window.console.error('"' + provider + '" not found!!!');
                }
            }
        }

        /*
         As of AngularJS 1.3.x, the config block are now stored in a new _configBlocks private
         variable.  Loop through the list and invoke the config block with config_injector
         */
        if (item.module._configBlocks) {
            var configBlocks = item.module._configBlocks;

            // console.info('configBlock: ', configBlocks);
            for (y = 0; y < configBlocks.length; y += 1) {
                var cf = configBlocks[y],
                    cf_method = cf[1],
                    cf_args = cf[2];

                config_injector[cf_method].apply(null, cf_args);
            }
        }

    }

    //after we have executed all config blocks, we finally execute the run blocks
    while (alternate_queue.length) {
        var item = alternate_queue.shift();
        if (item.module._runBlocks) {
            angular.forEach(item.module._runBlocks, processRunBlock);
        }
        /*
        Clear the cached modules created by alt_angular so that subsequent call to
        angular.module will return undefined.
        */
        alternate_modules = {};

    }

};
csbenjamin commented 9 years ago

For example, the https://github.com/fraywing/textAngular component. There, there is a module textAngular that have a value taTools which is a object. Still in this module, there is a config block with this code:

// clear taTools variable. Just catches testing and any other time that this config may run multiple times...
angular.forEach(taTools, function(value, key){ delete taTools[key]; });

This module requires a module textAngularSetup, which have a run block that fills the taTools object.

As textAngularSetup is a dependence of textAngular, it needs to be executed first. But after it fills the taTools object, the config block of textAngular module will clear the taTools object, which is not expected.

marcoslin commented 9 years ago

@csbenjamin sorry for delay but it has been a horrible 2 weeks of deadlines for me. Would you consider a pull request? It would be great if you add a unit test to it as well.

However, I do realise that angularAMD's unit test is a bit convoluted so I will try to add the unit test if you do the first part. That will allow me to take a closer look as well.

Many thanks.

csbenjamin commented 9 years ago

@marcoslin I can do a pull request. But tell me if there is a reason for alternate_modules = {}; is inside the while.

marcoslin commented 9 years ago

I created alternate_modules so that module can be returned within a ngload but clear it after it has been loaded. For example:

myModule.js

angular.modue('myModule', []);
...

var mod = angular.module('myModule');
mod.factory(...);

In this case, var mod needs to be populated but after myModules.js has been loaded, call to angular.module('myModule') should return undefined. This was done mostly to catch the intermodule dependencies where a module is created in one file and optional features are added by another.

csbenjamin commented 9 years ago

So, we can put alternate_modules = {}; after the loops instead inside them. I'm asking about this because now we will have two loops. One for config blocks and another for run blocks. In my code above I put alternate_modules = {}; inside the loop of run blocks. But I think we can put outside the loops. Am I right?

csbenjamin commented 9 years ago

@marcoslin I did a pull request without test. here: https://github.com/marcoslin/angularAMD/pull/125