feat($injector): ability to load new modules after bootstrapping #4694
Conversation
Added new method to $injector called loadNewModules. Added corresponding unit tests for new method.
|
Thanks for the PR!
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
|
I have just signed the CLA moments ago, as Craig Leyshan |
Added new method to $injector called loadNewModules. Added corresponding unit tests for new method.
Added unit tests to ensure new services, filters, directives and controllers can all be resolved after calling loadNewModules()
|
Looks great 👍 I like the nice coverage around ensuring that controllers, services, and friends can be defined on the new modules. Can't wait to use it in my own apps :) |
|
+1 I'm building a large web application and breaking it into modules is necessary. I spent most of Saturday writing a lazy-loading router and ended up making an almost identical change to Craig's to get it to work. I was planning to submit a PR today to see if we could get it in Angular, and found this one. I have tested Craig's change and it works perfectly for me too. |
|
This patch is sick. I'm using it to dynamically load modules based on config values. Works perfectly. |
|
+1 great change ! |
|
+1, would love to see this merged |
|
+1 from me! |
|
+1 from me |
|
+1 |
3 similar comments
|
+1 |
|
+1 |
|
+1 |
|
Worked perfectly for me! +1 |
|
+1 |
|
+1 This implementation would allow me to create embed codes that use angular.js, and that can be embedded in angular.js sites. |
|
+1 this is really good. |
|
+1 |
|
this change is too simplistic if we are going to do this then it should be part of some kind of script loader ( #5410 ) so that we can ensure that the right scripts have been loaded no matter how we got to the application state that requires lazily loaded components. |
|
@IgorMinar This change is perfect for what I need, and from the comments it seems to be all that several others need too. I read through ( #5410 ) and while it sounds like this change would probably be needed by it, I don't have a need for #5410. Could this change please be made a prerequisite to #5410 and expedited? You are correct in what you say that a script loader is needed - I use requirejs and a highly customized router to manage this for me. I expect most of the people who +1'd this do too. It would probably be useful to include a minimalist example in the docs to give people a clue as to how it could be used. Perhaps something like... require([ moduleName ], function() {
$injector.loadNewModules([ moduleName ]);
// then do what you need once the module has been loaded
});
|
|
@IgorMinar, @keithbranton says right things. Since AngularJS respects modularity and all its scripts are defined as separate modules (resource, mock...), new AMD loader should also be a module. So it seems that core scripts API (injector) will be changed for AMD loader module needs. The only reason to keep this change undone till the angular loader creation is to keep people away from the custom solutions as long as it is possible. Then AngularJS team can be sure, that there won't be plenty of loader solutions, and most people will use AngularJS solution from the beginning. Do you really care about it? Or where was I wrong? |
|
@stalnuhhin I agree that we want to avoid fragmentation of the methods people use to achieve dynamic loading using AMD with angular. However I also think that people using angular today have deadlines they have to hit, and need a solution that works now, rather than waiting 6-12 months for an ideal solution. They will create their custom solutions anyway. So I would suggest that this change be put in fairly quickly, but that it is not widely advertised or documented, to reduce (mis)use of the facility. |
|
@cleyshan, I'm the one who supports this change to be implemented as quickly as possible. |
|
+1 |
1 similar comment
|
+1 |
|
@IgorMinar I think this fix should be considered out of the context of script file loading. |
4dd5a20 to
998c61c
Compare
|
+1 just watched the 1.4 planning meeting, in which this issue was briefly addressed (https://www.youtube.com/watch?v=Uae9_8aFo-o) I'm guessing this is what the community is really striving for @btford, @IgorMinar, @petebacondarwin we can find many ways to package and load files, that's mostly an app realm problem .. we can even monkey patch the existing router to trigger asynchronously loading ... what we can't really do, with current angular is define a new module after bootstrap please make this a priority for 1.4, please, please, please |
|
+1 2014-12-14 23:51 GMT+01:00 Andre Torgal notifications@github.com:
|
|
Ended up modifying my local copy of angular to have this in place. Why even this was not a default behaviour in a first place? |
Because an API like this, unbounded, makes it very easy to create a dependency graph that's difficult to reason about. See #11015 |
|
+1 Building large applications without this feature is a mess. A must need feature. Hopefully, it arrives in 1.4 :) Looking forward too... |
|
@senthanal - 1.4 is already out the door, we have also got the main goals for 1.5 nailed down. I am tentatively adding this to the 1.6 milestone, which are are yet to start planning. |
|
👍 |
|
I have added this to our next team meeting to discuss... |
|
+1 |
1 similar comment
|
+1 |
|
Any updates on this? |
|
I don't think we discussed it in the end. I'll add to the next meeting... |
|
No rush, it's only 4 years since this was opened |
|
@cleyshan if you still want to do this then we have decided that we would be happy to merge this. |
|
It has now been a week and no reply from @cleyshan - if anyone else wants to take this on and provide a new PR with docs, that would work for us. |
|
@petebacondarwin Simplified snippets of my old code with this fork. Please, let me know if you need extended example. app.service('AmdLoader', function ($injector, $log) {
var bundleName = 'office-1';
var bundleJs = config.bundles[bundleName].js
getScript(bundleJs).done(function () {
onJsReady(bundleName);
$log.info('Script loaded : ' + bundleJs);
});
var onReady = function (bundleName, done) {
var moduleName = bundleName + '.html.bundle';
$log.debug('AMD >> Loading Angular module [' + moduleName + '] ...');
if (config.bundles[bundleName].modules) {
$injector.loadNewModules(config.bundles[bundleName].modules);
$log.debug('AMD >> Children modules loaded >>>');
$log.debug(config.bundles[bundleName].modules);
}
$injector.loadNewModules([moduleName]);
$log.debug('AMD >> Angular module [' + moduleName + '] loaded.');
if (done) done();
};
});Config: {
"bundles": {
"office-1": {
"js": "path/to/office-1.js",
"modules": [
"ui.mask"
]
}
}
} |
Added new method to $injector called loadNewModules.
Added corresponding unit tests for new method.
$injector.loadNewModules can be called any time after bootstrap so that any new angular modules defined after bootstrap can still function correctly. Please see https://groups.google.com/d/msg/angular/w0ZEBz02l8s/YK3H0eMmlxcJ for a detailed justification and explanation of this simple but powerful change.