I would like to hear your thoughts, idea's or feedback on the following code.
Ive added to code to github.
https://github.com/redbullzuiper/angularjs-dynamic-controllers
Usually when you attach a controller to a DOM element using ng-controller="myController"
, you need to include the controller somewhere in your page; for instance:
<script src="src/application/controllers/my-controller.js"></script>
However, when including the script I wrote in the head
, it will traverse across the DOM and search for all ng-controller
attributes.
It will then automatically append the correct controller into the head
of the page. After it's done including all the controllers, it will manually bootstrap the application using:
angular.bootstrap(document, ['module_name']);
This way all controllers are getting loaded dynamically, without the need of including all controllers in your head
. But they will instead only get loaded when called.
Can't create a demo using a fiddle, but a working demo can be found here
The code:
window.onload = function()
{
var promises = [];
var ngApp = document.querySelectorAll('[ng-app]')[0];
var ngControllers = document.querySelectorAll('[ng-controller]');
// Throw warning if angular is not found
if(typeof angular == 'undefined')
{
console.warn("simplify error: Angular not found, operation canceled.");
return;
}
// Throw warning if ng-app directive exists. Script will bootstrap the application manually
if(ngApp)
{
console.warn("Please remove the ng-app directive. `Simplify` will bootstrap the application manually.");
console.warn("This will also most likely fix the 'Uncaught Error: [$injector:modulerr]' error.");
}
// Append the scripts
for(var i = 0;i < ngControllers.length;i++)
{
var promise = new Promise(function(resolve, reject) {
var src = 'src/application/controllers/'+ ngControllers[i].getAttribute('ng-controller') + '.controller.js';
var script = document.createElement('script');
script.setAttribute('src', src);
document.head.appendChild(script);
script.addEventListener('load', function() {
resolve(script);
});
});
// Push promises to array to resolve them all together later on
promises.push(promise);
}
// Resolve all promises then bootstrap the app
// Without the use of promises, the bootstrap will start before all scripts are included
// This results into an erro
Promise.all(promises).then(function() {
angular.bootstrap(document, ['app']);
});
}
1 Answer 1
Feedback
This is an interesting way of loading controllers. I like the application of the promises. It does feel a little weird not to have an ng-app
directive but it makes sense given the nature of your code.
Suggestions
The suggestions below can simplify the code, but may not necessarily be optimal or have desired browser support.
Iterating functionally
One could utilize a functional approach instead of the declarative for
loop:
for(var i = 0;i < ngControllers.length;i++)
Because each iteration pushes a promise into the array promises
, that loop can be replaced by a call to Array.map()
:
var promises = Array.prototype.map.call(ngControllers, function(ngController)
Or .map
could be called on an array after converting the nodeList to an array via Array.from() (but beware of Browser compatibilty on that)
var promises = Array.from(ngControllers).map(function(ngController)
Or for a completely es-6 approach, use the spread operator:
var promises = [...ngControllers].map(function(ngController)
At the end of each iteration, simply return the promise instead of pushing it into the array.
// Push promises to array to resolve them all together later on
return promise;//promises.push(promise);
}); //end the call to .map()
Bear in mind that the functional approach would likely be somewhat slower because a function call is added for each iteration, but for a small number of controllers this should be a negligible difference. If you would like to become more familiar with functional JavaScript, I recommend these exercises.
Use partial(-ly applied) functions for callbacks
Instead of making a closure just to call resolve()
:
script.addEventListener('load', function() { resolve(script); });
Use Function.bind()
(different than (angular.bind()
) to create a partially applied function for each time resolve()
is called.
script.addEventListener('load', resolve.bind(null, script));
Bear in mind that the event listener would pass an event
argument to the callback, but that should be the second argument when script
is bound as the first argument.
The same is true for calling the bootstrap function after all promises have been resolved:
Promise.all(promises).then(function() { angular.bootstrap(document, ['app']); });
Can be simplified like below:
Promise.all(promises).then(angular.bootstrap.bind(null, document, ['app']));
Use addEventListener
instead of onload
This likely wouldn't be an issue in a small application but in case there was other code (potentially multiple functions) that needed to be executed when the page is loaded, it might be simpler just to use window.addEventListener('DOMContentLoaded', function() {...})
instead of window.onload = function() {...};
-
\$\begingroup\$ I'm really happy with the suggestions. Especially the way you suggest to handle the
addEventListerner
. Thats indeed a much simpler approach. I will make these changes in the github project and codepen. I appreciate your time and effort! \$\endgroup\$Red– Red2018年03月29日 17:02:57 +00:00Commented Mar 29, 2018 at 17:02
Explore related questions
See similar questions with these tags.