Bug in forEach on chrome

[SimeonC]

This is a bit of an odd bug that's occurred for me in two instances, one I've managed to fix, the other I can't really fix bar hacking into the forEach code in angularjs.

Basically the forEach is also iterating over the "$$hashMap" key which can cause errors in certain situations. I've seen this happen on Chrome at least, but I haven't tested the other browsers to see if it's browser relevant or not.

Issue 1

The first I managed to solve, it occurred in some custom validation code where I had created a directive to dynamically provide a validation object and have it apply onto an ng-Models $validation code.
Because this was used in a scope I guess it added in the $$hashMap key to the object I was assigning to the $validators object and cause a TypeError: string is not a function error when the forEach tried to eval the $$hashMap as a key. Code below (simplified a bit):

directive('ngValidation', function() {
  return {
    restrict: 'A',
    require: ['ngModel', '^form'],
    link: function(scope, element, attrs, ctrls) {
      var ngForm, ngModel, _funcs;
      ngModel = ctrls[0], ngForm = ctrls[1];
      if ((_funcs = scope.$eval(attrs.ngValidation)) != null) {
        if (_funcs.$validators) {
          angular.extend(ngModel.$validators, _funcs.$validators);
        }
        if (_funcs.$asyncValidators) {
          angular.extend(ngModel.$asyncValidators, _funcs.$asyncValidators);
        }
        delete ngModel.$validators.$$hashKey;
        delete ngModel.$asyncValidators.$$hashKey;
      }
      scope.$watch(attrs.ngModel, ngModel.$validate, true);
      return ngForm.$addControl(ngModel);
    }
  };
});

Issue 2

I figure this is the same issue as angular is trying to load a provider called "object:241", which is not defined/requested anywhere in my code and looks very much like a $$hashKey value.

 Error: [$injector:unpr] Unknown provider: object:241Provider <- object:241
http://errors.angularjs.org/1.3.13/$injector/unpr?p0=object%3A241Provider%20%3C-%20object%3A241
    at https://crm-node:3443/assets/angular/angular.js?body=1:63:12
    at https://crm-node:3443/assets/angular/angular.js?body=1:3997:19
    at Object.getService [as get] (https://crm-node:3443/assets/angular/angular.js?body=1:4144:39)
    at https://crm-node:3443/assets/angular/angular.js?body=1:4002:45
    at Object.getService [as get] (https://crm-node:3443/assets/angular/angular.js?body=1:4144:39)
    at https://crm-node:3443/assets/angular-route/angular-route.js?body=1:586:31
    at Object.forEach (https://crm-node:3443/assets/angular/angular.js?body=1:331:20)
    at https://crm-node:3443/assets/angular-route/angular-route.js?body=1:584:23
    at processQueue (https://crm-node:3443/assets/angular/angular.js?body=1:13189:27)
    at https://crm-node:3443/assets/angular/angular.js?body=1:13205:27

I think this is caused by the following code (simplified down again):

TicketControllers = angular.module('tickets', [], [
  '$routeProvider', '$locationProvider', 'EditControllerResolve', function($routeProvider, $locationProvider, EditControllerResolve) {
    $routeProvider.when('/:ticketID', {
      templateUrl: '/ticket-partials/edit',
      controller: 'EditController',
      resolve: EditControllerResolve
    });
    return $locationProvider.html5Mode(true);
  }
]);

TicketControllers.constant('EditControllerResolve', {
  ticket: [
    '$http', '$q', function($http, $q) {
      var deferred;
      deferred = $q.defer();
      $http.get("/...").success(function(data) {
        return deferred.resolve(data);
      });
      return deferred.promise;
    }
  ],
  metadata: [
    '$htttp', '$q', '$route', function($http, $q, $route) {
      var deferred;
      deferred = $q.defer();
      $http.get("/...").success(function(data) {
        return deferred.resolve(data);
      });
      return deferred.promise;
    }
  ]
});

TicketControllers.controller('EditController', [function($scope) { ...

[pkozlowski-opensource]

@SimeonC could you please narrow id down a bit more, use JavaScript and put a minimal reproduction scenario into plunker or similar on-line tool? Otherwise it is rather hard to pin-point what is going wrong, exactly... Thnx!

[SimeonC]

I'll do my best, I've converted it to JS now - I'll see if I can manage to reproduce it in a plnkr. But it is tricky to reproduce in the first place!!

[pkozlowski-opensource]

@SimeonC of course, please comment / re-open as soon as you've got reduced reproduction scenario! Closing for now since we can't take any action but yeh, will look into it as soon as you've got something we can work with.

[gaultier]

Got this problem as well, only on Chrome with minified version (tested with Firefox, IE11, and Edge, on Linux and Windows).

I make an http request with $resource and iterate with angular.forEach (bug also occurs with _.forEach from lodash) over the result returned by the promise.

The thing is, some keys like $resolved are iterated over, which creates errors because I have in the same object being iterated over, $resolved: true, as well as myKey: myObject. My code expects the value to be an object but in this case it is true.

Does anyone have insight on how/why this happens, only in Chrome (every OS) and with the minified version?

A simplified version looks like:

// myResource is a $resource
myResource.get().$promise
.then(function(result) {
// result is an object whose keys are strings and values are object (it's a dictionary of objects)
// For example: result = {'joe': {id: 1, name: 'joe'}, 'jim': {id: 2, name: 'jim'}}
  angular.forEach(result, function(value) { // key-value pair {$resolved: true} will be weirdly iterated over
    value.newProperty = 'toto'; // Crashes because true is not an object and thus we cannot add a property to it
  });
});

[gkalpak]

@gaultier, this is pretty weird (the fact that it happens with the minified version only and only on Chrome).
Since this is closed (and we don't actually know if the two problems are related), you should open a new issue and provide all necessary details (e.g. version of Angular and browser). Ideally, you should create a live demo (e.g. using CodePen, plnkr etc).

[caitp]

Based on the v8 issue that was filed in regards to this, it looks like you could get your expected non-throwing behaviour by changing value.newProperty = 'toto'; to Object(value).newProperty = 'toto';

When value is a primitive, this will prevent it from throwing (by making the receiver an Object). In all other cases, it's unobservable and makes no difference.

Note that trying to assign a property to the primitive this way won't actually do anything, but it wouldn't do anything in the other browsers you list anyways, so you should be safe there.

That said, it's probably better for you to just test if it is an object before trying to assign the property.

Also, as noted on the v8 bug, the behaviour is much more consistent between Chrome, FireFox and other vendors now.

[gaultier]

I opened a issue identifying and describing the problem:
#14460