Angular.js: Unintended breaking change when passing ngModel as a binding

[frederikprijck]

Note: for support questions, please use one of these channels: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#question. This repository's issues are reserved for feature requests and bug reports.

Do you want to request a feature or report a bug?
bug

What is the current behavior?
The following code throws an error with AngularJS 1.6.2 but not with 1.5.8 (I haven't checked other versions yet):

<form name="example">
    <input name="example[firstName]" ng-model="firstName">
    <validation-confirmed fields="[example['example[firstName]']]"></validation-confirmed>
</form>

The error:

Error: [ng:cpws] Can't copy! Making copies of Window or Scope instances is not supported.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).
Reproduce:
https://plnkr.co/edit/dAqZuQP4e9Gk1wc6c1sw?p=preview (AngularJS 1.6.2 not working)
https://plnkr.co/edit/nuXCMcYoJUGJ4xucmhqC?p=preview (AngularJS 1.5.8 working)

What is the expected behavior?
No error as in the 1.5.x version.

What is the motivation / use case for changing the behavior?

Which versions of AngularJS, and which browser / OS are affected by this issue? Did this work in previous versions of AngularJS? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

Other information (e.g. stacktraces, related issues, suggestions how to fix)

[erikbaan]

Removing the quotes resolves the issue: [example[example[firstName]]]
But in previous angular versions (< 1.5.8), this also works including the quotes.

[gkalpak]

Removing the quotes doesn't solve the problem. It makes the error go away, but the component deoes not work. The reason this breaks is that (since 9e24e774a) we put the scope on the NgModelController instance, so it is no longer possible to copy such instances (which happens under the hood when $watching).

@jbedard, thoughts?

[jbedard]

This is something that always bugs me with Angular(js) DI. You must store injected things on the instance in cases like this even though they are the same or every instance (within that angular app).

I don't think people should be watching the full NgModelController though. But if we want to support it all I can think of is using defineProperty to prevent some things from being copied? Or add a way for objects to provide a copy hook to define their own copy method?

[gkalpak]

I know what you mean, but in this case it is _not_ the same on every instance (that's why you need to store it on the instance) :smiley:

The problem came up when passing the NgModelController instance as a < binding, not explicitly watching it. I would rather avoid this breaking change if possible (in a reasonable way).

#

I just realized this is only a problem if you have an array or object literal. This will work correctly on 1.7.x (thanks to #15301), but not on 1.6.x 😞
It will still work fine on 1.6.x if constructing the array and passing it as binding instead of using an array literal.

[jbedard]

All I can think of is doing Object.defineProperty(this, '$$scope', {value: $scope}) so it is non-enumerable...

[gkalpak]

For the record, here are the alternative solutions we discussed:

• Only store the scope ID on the controller instance, store scopes on a separate object/hash, look up scopes by id.
  _Con: One extra look-up. Slightly bigger change._
• Turn $$scope into a function (e.g. $$getScope()) and access scope via a closure.
  _Con: Kind of beats the purpose of moving methods on NgModelController.prototype to avoid closures._
• Rename $$scope to $$$scope, teach copy() to ignore $$$ prefixed properties.
  _Con: Eww! (Plus some app, somewhere will be broken by this.)_
• Teach copy() to use a $$copy() method if present.
  _Con: Slightly bigger change. (Plus some app, somewhere might be broken by this.)_

[jbedard]

A workaround has been added to 1.6.x. Tests added to 1.7 to confirm that this no longer happens in this specific case (ng-model in an object literal). If you manually deep watch an mg-model it will still occur but I don't think we want to fix that.