From 7f82986a7e43010dcbd49335d696959732d11ddd Mon Sep 17 00:00:00 2001
From: Peter Bacon Darwin <pete@bacondarwin.com>
Date: Mon, 27 Apr 2015 22:17:05 +0100
Subject: [PATCH 1/2] fix(ngOptions): use watchCollection not deep watch of
 ngModel

---
 src/ng/directive/ngOptions.js      | 2 +-
 test/ng/directive/ngOptionsSpec.js | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/ng/directive/ngOptions.js b/src/ng/directive/ngOptions.js
index 80390c4e5c1c..f9715a27d3f3 100644
--- a/src/ng/directive/ngOptions.js
+++ b/src/ng/directive/ngOptions.js
@@ -515,7 +515,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
       // We also need to watch to see if the internals of the model changes, since
       // ngModel only watches for object identity change
       if (ngOptions.trackBy) {
-        scope.$watch(attr.ngModel, function() { ngModelCtrl.$render(); }, true);
+        scope.$watchCollection(attr.ngModel, function() { ngModelCtrl.$render(); });
       }
       // ------------------------------------------------------------------ //
 
diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js
index 97b58dee9c20..93ee40545951 100644
--- a/test/ng/directive/ngOptionsSpec.js
+++ b/test/ng/directive/ngOptionsSpec.js
@@ -901,8 +901,7 @@ describe('ngOptions', function() {
 
       // Update the properties on the object in the selected array, rather than replacing the whole object
       scope.$apply(function() {
-        scope.selected[0].id = 20;
-        scope.selected[0].label = 'new twenty';
+        scope.selected[0] = {id: 20, label: 'new twenty'};
       });
 
       // The value of the select should change since the id property changed

From e6ab5ab29b8c63433fe0d96964d95e69cc89f26f Mon Sep 17 00:00:00 2001
From: Peter Bacon Darwin <pete@bacondarwin.com>
Date: Tue, 28 Apr 2015 12:09:25 +0100
Subject: [PATCH 2/2] fix(ngOptions): use watchCollection not deep watch of
 ngModel

Using a deep watch caused Angular to enter an infinite recursion in the
case that the model contains a circular reference.  Using `$watchCollection`
instead prevents this from happening.

This change means that we will not re-render the directive when there is
a change below the first level of properties on the model object. Making
such a change is a strange corner case that is unlikely to occur in practice
and the directive is not designed to support such a situation. The
documentation has been updated to clarify this behaviour.

This is not a breaking change since in 1.3.x this scenario did not work
at all. Compare 1.3.15: http://plnkr.co/edit/zsgnhflQ3M1ClUSrsne0?p=preview
to snapshot: http://plnkr.co/edit/hI48vBc0GscyYTYucJ0U?p=preview

Closes #11372
Closes #11653
Closes #11743
---
 src/ng/directive/ngOptions.js      | 20 +++++--
 test/ng/directive/ngOptionsSpec.js | 86 +++++++++++++++++++++++++++++-
 2 files changed, 99 insertions(+), 7 deletions(-)

diff --git a/src/ng/directive/ngOptions.js b/src/ng/directive/ngOptions.js
index f9715a27d3f3..5f67c950e7e1 100644
--- a/src/ng/directive/ngOptions.js
+++ b/src/ng/directive/ngOptions.js
@@ -31,11 +31,21 @@ var ngOptionsMinErr = minErr('ngOptions');
  * be nested into the `<select>` element. This element will then represent the `null` or "not selected"
  * option. See example below for demonstration.
  *
- * <div class="alert alert-warning">
- * **Note:** By default, `ngModel` compares by reference, not value. This is important when binding to an
- * array of objects. See an example [in this jsfiddle](http://jsfiddle.net/qWzTb/). When using `track by`
- * in an `ngOptions` expression, however, deep equality checks will be performed.
- * </div>
+ * ## Complex Models (objects or collections)
+ *
+ * **Note:** By default, `ngModel` watches the model by reference, not value. This is important when
+ * binding any input directive to a model that is an object or a collection.
+ *
+ * Since this is a common situation for `ngOptions` the directive additionally watches the model using
+ * `$watchCollection` when the select has the `multiple` attribute or when there is a `track by` clause in
+ * the options expression. This allows ngOptions to trigger a re-rendering of the options even if the actual
+ * object/collection has not changed identity but only a property on the object or an item in the collection
+ * changes.
+ *
+ * Note that `$watchCollection` does a shallow comparison of the properties of the object (or the items in the collection
+ * if the model is an array). This means that changing a property deeper inside the object/collection that the
+ * first level will not trigger a re-rendering.
+ *
  *
  * ## `select` **`as`**
  *
diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js
index 93ee40545951..2998ad7aa577 100644
--- a/test/ng/directive/ngOptionsSpec.js
+++ b/test/ng/directive/ngOptionsSpec.js
@@ -776,6 +776,54 @@ describe('ngOptions', function() {
     });
 
 
+    it('should re-render if an item in an array source is added/removed', function() {
+      createSelect({
+        'ng-model': 'selected',
+        'multiple': true,
+        'ng-options': 'item.id as item.label for item in arr'
+      });
+
+      scope.$apply(function() {
+        scope.selected = [10];
+      });
+      expect(element).toEqualSelectValue([10], true);
+
+      scope.$apply(function() {
+        scope.selected.push(20);
+      });
+      expect(element).toEqualSelectValue([10, 20], true);
+
+
+      scope.$apply(function() {
+        scope.selected.shift();
+      });
+      expect(element).toEqualSelectValue([20], true);
+    });
+
+
+    it('should handle a options containing circular references', function() {
+      scope.arr[0].ref = scope.arr[0];
+      scope.selected = [scope.arr[0]];
+      createSelect({
+        'ng-model': 'selected',
+        'multiple': true,
+        'ng-options': 'item as item.label for item in arr'
+      });
+      expect(element).toEqualSelectValue([scope.arr[0]], true);
+
+      scope.$apply(function() {
+        scope.selected.push(scope.arr[1]);
+      });
+      expect(element).toEqualSelectValue([scope.arr[0], scope.arr[1]], true);
+
+
+      scope.$apply(function() {
+        scope.selected.pop();
+      });
+      expect(element).toEqualSelectValue([scope.arr[0]], true);
+    });
+
+
     it('should support single select with object source', function() {
       createSelect({
         'ng-model': 'selected',
@@ -1041,7 +1089,7 @@ describe('ngOptions', function() {
       }).not.toThrow();
     });
 
-    it('should setup equality watches on ngModel changes if using trackBy', function() {
+    it('should re-render if a propery of the model is changed when using trackBy', function() {
 
       createSelect({
         'ng-model': 'selected',
@@ -1063,7 +1111,7 @@ describe('ngOptions', function() {
 
     });
 
-    it('should not setup equality watches on ngModel changes if not using trackBy', function() {
+    it('should not re-render if a property of the model is changed when not using trackBy', function() {
 
       createSelect({
         'ng-model': 'selected',
@@ -1084,6 +1132,40 @@ describe('ngOptions', function() {
       expect(element.controller('ngModel').$render).not.toHaveBeenCalled();
     });
 
+
+    it('should handle options containing circular references (single)', function() {
+      scope.arr[0].ref = scope.arr[0];
+      createSelect({
+        'ng-model': 'selected',
+        'ng-options': 'item for item in arr track by item.id'
+      });
+
+      expect(function() {
+        scope.$apply(function() {
+          scope.selected = scope.arr[0];
+        });
+      }).not.toThrow();
+    });
+
+
+    it('should handle options containing circular references (multiple)', function() {
+      scope.arr[0].ref = scope.arr[0];
+      createSelect({
+        'ng-model': 'selected',
+        'multiple': true,
+        'ng-options': 'item for item in arr track by item.id'
+      });
+
+      expect(function() {
+        scope.$apply(function() {
+          scope.selected = [scope.arr[0]];
+        });
+
+        scope.$apply(function() {
+          scope.selected.push(scope.arr[1]);
+        });
+      }).not.toThrow();
+    });
   });