WIP: TODO notes

Author: IgorMinar

src/Angular.js

@@ -440,6 +440,7 @@ function isDefined(value){return typeof value !== 'undefined';}
  * @param {*} value Reference to check.
  * @returns {boolean} True if `value` is an `Object` but not `null`.
  */
+// TODO(perf): use !== null to avoid conversion???
 function isObject(value){return value != null && typeof value === 'object';}
 
 
@@ -832,6 +833,7 @@ function shallowCopy(src, dst) {
   if (isArray(src)) {
     dst = dst || [];
 
+    // TODO(perf): how about slice?
     for (l = src.length; i < l; i++) {
       dst[i] = src[i];
     }

src/jqLite.js

@@ -253,8 +253,10 @@ function jqLiteClone(element) {
 function jqLiteDealoc(element, onlyDescendants){
   if (!onlyDescendants) jqLiteRemoveData(element);
 
+  // TODO(perf): do we need to check length here? if so, cache childNodes
   if (element.childNodes && element.childNodes.length) {
     // we use querySelectorAll because documentFragments don't have getElementsByTagName
+    // TODO(perf): since we need to slice the live list, shoudn't we just use querySelectorAll all the time?
     var descendants = element.getElementsByTagName ? sliceArgs(element.getElementsByTagName('*')) :
                     element.querySelectorAll ? element.querySelectorAll('*') : [];
     for (var i = 0, l = descendants.length; i < l; i++) {
@@ -307,6 +309,7 @@ function jqLiteRemoveData(element, name) {
       }
       jqLiteOff(element);
     }
+    // TODO(perf): delete vs set to undefined?
     delete jqCache[expandoId];
     element.ng339 = undefined; // don't delete DOM expandos. IE and Chrome don't like it
   }

src/ng/compile.js

@@ -911,6 +911,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
           : $compileNodes;
 
         if (transcludeControllers) {
+          // TODO(perf): use Object.create(null) for transcludeControllers and then `for in`
           var names = Object.keys(transcludeControllers);
           var i = names.length;
           var name;
@@ -1061,6 +1062,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
 
         var clone = transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn);
         if (scopeCreated) {
+          // TODO(perf): do we really need this? looks bogus + tries to registers listeners on
+          //             boundary comment needs in repeater
           clone.on('$destroy', function() { transcludedScope.$destroy(); });
         }
         return clone;
@@ -1670,6 +1673,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
           var transcludeControllers;
 
           // no scope passed
+          // TODO(perf): arguments.length is slow, check if cloneAttachFn is truthy instead
+          // http://jsperf.com/isundefined-vs-arguments-length
           if (arguments.length < 2) {
             cloneAttachFn = scope;
             scope = undefined;
@@ -2075,6 +2080,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
       if (parent) {
         parent.replaceChild(newNode, firstElementToRemove);
       }
+
+      // TODO(perf): what are we doing with this document fragment? could we return it for cloning?
       var fragment = document.createDocumentFragment();
       fragment.appendChild(firstElementToRemove);
       newNode[jqLite.expando] = firstElementToRemove[jqLite.expando];

src/ng/directive/input.js

@@ -1188,6 +1188,7 @@ function radioInputType(scope, element, attr, ctrl) {
 
   ctrl.$render = function() {
     var value = attr.value;
+    // TODO(perf): update checked only when match since only one radio button can be checked at a time?
     element[0].checked = (value == ctrl.$viewValue);
   };
 
@@ -1966,6 +1967,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
       modelValue = modelValue();
     }
 
+    // TODO(perf): why not move this to the action fn?
     // if scope model value and ngModel value are out of sync
     if (ctrl.$modelValue !== modelValue &&
         (isUndefined(ctrl.$$invalidModelValue) || ctrl.$$invalidModelValue != modelValue)) {

src/ng/directive/ngBind.js

@@ -59,6 +59,7 @@ var ngBindDirective = ['$compile', function($compile) {
         // We are purposefully using == here rather than === because we want to
         // catch when value is "null or undefined"
         // jshint -W041
+        // TODO(perf): consider setting textContent directly
         element.text(value == undefined ? '' : value);
       });
     }

src/ng/directive/ngRepeat.js

@@ -341,6 +341,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
           arrayLength = collectionKeys.length;
 
           // locate existing items
+          // TODO(perf): don't reset nextBlockOrder.length ???
           length = nextBlockOrder.length = collectionKeys.length;
           for (index = 0; index < length; index++) {
             key = (collection === collectionKeys) ? index : collectionKeys[index];
@@ -386,6 +387,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
             key = (collection === collectionKeys) ? index : collectionKeys[index];
             value = collection[key];
             block = nextBlockOrder[index];
+            // TODO(perf): simplify previousBlockBoundaryNode calculation
             if (nextBlockOrder[index - 1]) previousNode = getBlockEnd(nextBlockOrder[index - 1]);
 
             if (block.scope) {
@@ -406,8 +408,10 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
               // new item which we don't know about
               $transclude(function ngRepeatTransclude(clone, scope) {
                 block.scope = scope;
+                // TODO(perf): could we move this into the template element and have it cloned with it?
                 // http://jsperf.com/clone-vs-createcomment
                 clone[clone.length++] = ngRepeatEndComment.cloneNode();
+                // TODO(perf): support naked previousNode in `enter`?
                 $animate.enter(clone, null, jqLite(previousNode));
                 previousNode = clone;
                 // Note: We only need the first/last node of the cloned nodes.

src/ng/directive/ngSwitch.js

@@ -156,6 +156,7 @@ var ngSwitchDirective = ['$animate', function($animate) {
           selectedScopes[i].$destroy();
           previousElements[i] = selected;
           $animate.leave(selected, function() {
+            // TODO(perf): this schedules rAF even without animations which dealocs the elements again via previousElements[i].remove() above;
             previousElements.splice(i, 1);
           });
         }

src/ng/httpBackend.js

@@ -1,6 +1,7 @@
 'use strict';
 
 function createXhr(method) {
+  // TODO(ie): remove
     //if IE and the method is not RFC2616 compliant, or if XMLHttpRequest
     //is not available, try getting an ActiveXObject. Otherwise, use XMLHttpRequest
     //if it is available

src/ng/parse.js

@@ -308,6 +308,7 @@ Lexer.prototype = {
       token.constant = true;
     } else {
       var getter = getterFn(ident, this.options, this.text);
+      // TODO(perf): consider exposing the getter reference
       token.fn = extend(function(self, locals) {
         return (getter(self, locals));
       }, {

src/ng/rootScope.js

@@ -529,6 +529,7 @@ function $RootScopeProvider(){
               oldValue.length = oldLength = newLength;
             }
             // copy the items to oldValue and look for changes.
+            // TODO(perf): consider caching oldValue[i] and newValue[i]
             for (var i = 0; i < newLength; i++) {
               bothNaN = (oldValue[i] !== oldValue[i]) &&
                   (newValue[i] !== newValue[i]);
@@ -540,6 +541,7 @@ function $RootScopeProvider(){
           } else {
             if (oldValue !== internalObject) {
               // we are transitioning from something which was not an object into object.
+              // TODO(perf): Object.create(null) + don't use hasOwnProperty for oldValue
               oldValue = internalObject = {};
               oldLength = 0;
               changeDetected++;
@@ -700,7 +702,10 @@ function $RootScopeProvider(){
                   watch = watchers[length];
                   // Most common watches are on primitives, in which case we can short
                   // circuit it with === operator, only when === fails do we use .equals
+                  // TODO(perf): is this if needed?
                   if (watch) {
+                    // TODO(perf): the last typeof number + NaN checks are executed for all counter watches
+                    //             can we skip it or at least remember if the lastValue is NaN
                     if ((value = watch.get(current)) !== (last = watch.last) &&
                         !(watch.eq
                             ? equals(value, last)
@@ -709,6 +714,7 @@ function $RootScopeProvider(){
                       dirty = true;
                       lastDirtyWatch = watch;
                       watch.last = watch.eq ? copy(value, null) : value;
+                      // TODO(perf): initialize all watches via async queue?
                       watch.fn(value, ((last === initWatchVal) ? value : last), current);
                       if (ttl < 5) {
                         logIdx = 4 - ttl;
@@ -811,6 +817,7 @@ function $RootScopeProvider(){
         this.$$destroyed = true;
         if (this === $rootScope) return;
 
+        // TODO(perf): $$listenerCount should be Object.create(null), then use `for in`
         forEach(this.$$listenerCount, bind(null, decrementListenerCount, this));
 
         // sever all the references to parent scopes (after this cleanup, the current scope should