One-way bindings of arrays behave inconsistently when a default value is set in the constructor

[donmccurdy]

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

Bug.

What is the current behavior?

One-way bindings of arrays behave inconsistently when a default value is set in the constructor.

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).

Minimal JSFiddle example.

childCtrl.list2 is set but childCtrl.list1 never is. The changes argument to $onChanges(changes) contains both values, however.

What is the expected behavior?

Perhaps I made a terrible mistake by trying to set default values (list1=[] and list2=[]) in my child controller's constructor. Probably that's my fault, and list1 being empty is expected. But the problem did take me longer to track down because some previous code (like list2) had worked just fine.

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

I'd have expected list1 and list2 to behave the same way here. If setting a default in the constructor is bad and should never be done, then maybe no change is needed.

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

v1.5.8, v1.5.9

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

Thanks!

[dcherman]

The current implementation in Angular has been assigning bindings to the controller prior to your constructor being called; you were overwriting the binding that was passed in.

Once #15095, this behavior will be optionalized and you can opt-in to having the bindings initialized later, so you can safely set defaults in the constructor. That and since you weren't specifying your one-way bindings as optional, any defaults you did set would be overwritten even if no binding was provided.

[gkalpak]

@dcherman, I think this is a different problem (which is also highlighted by the difference in behavior between list1 and list2. I will add a more detailed answer soon.

[gkalpak]

Actually, I meant there is not noly that. What @dcherman mentioned above explains why the list1 binding is overwritten by the default value. But there is more going on (that causes list2 to be set to the "correct" value).

[gkalpak]

So, the fact that list2 shows correctly is (kind of) a bug 😃

The main takeaway (for now) should be that if you want to set default values, you should set them either in the first call of $onChanges or in the $onInit hook (in both cases only if there are no values present):

// In child
this.$onInit = function $onInit() {
  if (!this.list1) this.list1 = [];
  if (!this.list2) this.list2 = [];
};

For (self) reference and the curious:

When we initialize the bindings on the child controller (which happens before actually instantiating the controller), we assign the initial values for list1/list2 to the controller instance (here) - yes, there is an instance even if we haven't instantiated the controller yet (don't ask 😃) - collect the initial changes for list1/list2 (here) and set up a watch on them (here). The collected initial changes will be used to fire an initial $onChanges call.
At this point, child.list1 == child.list2 == [1, 2, 3].

Then, we instantiate the controller (i.e. run the constructor in the context of the instance), so the default values overwrite the alreayd set bindings.
At this point, child.list1 == child.list2 == [].

Later, when the watcher fires for the first time, it will compare the current values with those initial values:

• If they are the same (in the === sense), it will do nothing. This is what happens with list1. It remains at [].
• If they have changed, it will assign the new values to the child controller (and schedule an $onChanges). This is what happens to list2. It gets re-assigned to [1, 2, 3].

At this point, child.list1 == []; child.list2 == [1, 2, 3].

The reason why initialList1BindingValue === list1FromFirstWatch is that it refers to parent.list, which hasn't changed.
The reason why initialList2BindingValue !== list2FromFirstWatch is that the list2 binding is a literal expression, which means that a new array will be created with every digest. (The new array will contain the exact same elements, but it wil be a different array object.) Thus the === comparison fails.

Note, subsequent watch evaluations will not run into the same issue, because we set up a deep watcher for literals, but it seems that we missed that on the comparison here. <-- This is the bug I referred to earlier.

[gkalpak]

We should probably change this line like this:

-if (oldValue === initialValue) return;
+if (parentGet.literal ? equals(oldValue, initialValue) : oldValue === initialValue) return;

(But get scared when I think what we might break 👿 )

[donmccurdy]

Thanks for looking into this! I appreciate the detailed answer. The bug was harmless enough, so leaving it alone seems fine to me.

[gkalpak]

There is no such thing as "harmless bug" 😛
(Submitted a fix in #15123.)

[donmccurdy]

Awesome, thanks! 😄