Endless loop in watch

[thezuck]

There is an infinite loop caused by a deep watch loop, as can be seen in the animated gif below, we start with a simple object which is watched by MarkerChildModel, through a call to
return _this.handleModelChanges(newValue, oldValue);
this does deep watching using getChanges method

ModelKey.prototype.getChanges = function(now, prev, whitelistedProps) {

c = this.getChanges(now[prop], (prev ? prev[prop] : null));

as can be seen in the animated gif, all properties in the object are watched, including a property called icon which has a field called scaledSize, which looks like so:
X {width: 36, height: 36, L: "px", N: "px"}
however, this object has a prototype which is has a toString function which in turn has a method called disable, which seems to references itself.
Since the getchanges method recursively dives in, it never stops until an exception is fired, repeatedly watching disable. This X type object is a google.maps.Size object, so it's not something I can control, and is probably a commonly used property.
There is no way I found to limit how deep the watch should go, and it doesn't limit itself by default.

The initial model which we create and add to the models array used by gmap markers is:

{
                _id: place._id,
                latitude: place.lat,
                longitude: place.lon,
                options: {
                    icon: {
                        url: iconUrl,
                        size: iconSmallSizeObj
                    }
                },
                titleContent: place.companyName
}

where the issue occurs for icon's property size, because iconSmallSizeObj is defined as
new google.maps.Size(32, 32)
so I assume that to reproduce you need to add this type of object to the model.

Final word: this does not happen immediately when the model is added, there is some race condition or some other unexplained process which causes this to happen, can't figure out the exact route to make this happen but it happens very frequently.

[thezuck]

Here's the resulting exception in case it helps

[thezuck]

Happened on 2.1.5, reproduced on 2.2.0

[nmccready]

Can you make a plnkr valid example?

[nmccready]

Also if you continue posting more code please use github conventions by using backticks.

[nmccready]

Well adding an attribute specifically to modify watch depth would help and it is something that should be done.

[thezuck]

a plnkr is a valid requirement, however, I think that iterating over own properties (by filtering using hasOwnProperty) instead of all properties makes sense as an optimization, and adding watch depth limit also makes sense, regardless of whether or not you can reproduce this bug.

[nmccready]

Yes hasOwnProperty is wise I can add that.

[thezuck]

👍

[thezuck]

amazing job on this one by the way, we are using it for my startup (mapme) and very happy with it!

[nmccready]

cool I have plans down the pipe to make the library faster. Basically I will (probably some people out there who wont like this) add another dependency. There is iterator code in angular-leaflet-directive that is proven to be faster that using Array.forEach and lodash's iterators. So I am thinking of an angular-iterator library. But this is still up in the air. If people want other options I am open to it but leafletIterators works pretty well.

http://jsperf.com/iterators/3

https://github.com/tombatossals/angular-leaflet-directive/blob/master/src/services/leafletIterators.js

Also the _async code could be possible moved into there as well.

[nmccready]

see PR will close on merge