Fixes for #645 and #632

[wattsbn]

Fixes for #645 and #632

Both fixes are going to decrease performance, but I think the hit is necessary to fix these bugs.
We can use $watchColleciton whenever we start using angular 1.1.4

[roblarsen]

Can you set up a demo so we can look at what sort of performance hit this will inflict?

[wattsbn]

Here is the plunk used for #645: http://plnkr.co/edit/8qNVdI?p=preview
and here it is using my changes: http://plnkr.co/edit/vXk8ql?p=preview

The performance hit is only for when data changes.
So, try toggling between 'Half' and 'All'. There is a noticeable difference for 10,000 items between the two plunks, but its not like its slow. This demonstrates the changes needed to fix #645.

For #632, its a little more complicated.
Instead of doing two object identity (pointer) watches, I changed it to do a single deep object watch.
What this amounts to is the dataWatcher will get called anytime the data array/object changes in anyway including changes that we don't care about. So the performance hit depends on how often the user is changing the data array or the objects in the array.

[swalters]

A deep watch on our collection backing the grid would be a show stopper. Our objects are very complex and we might only show 10 columns in the grid, but have over 100 properties in the objects bound the collection.

I should add that our biggest complaint about the grid so far is performance. Anything that slows it down that is not some optional setting just won't work.

To be realistic with your plunker, add about 15 - 20 columns over 10,000 items. I know that we shouldn't be using 10,000 items in a grid and I don't do that. But most tests concentrate on many rows while I see many columns being a much bigger issue.

[wattsbn]

Alright, maybe the best bet here is to add an option (watchData or updateOnDataChange) for doing watching in general. Have it default to true for the current identity watches and if the user sets it to 'deep', then we do a deep watch. Then when we can do shallow watching, we can just make it strictly true/false.

I added 50 columns and tested out the changes without doing a deep watch and it still looks like it works fine to me.

I will try to update the plunks to showcase rows as well as columns and and the new option to the pull request sometime this weekend.

@swalters, just curious, do any of your usages add/remove multiple items at a time from the grid or is mostly editing the items after the initial load? I ask because both of these bugs seem like fairly big functionality issues and I wondering how I'm the first to find them.

[swalters]

@wattsbn We never actually remove items from the grid, we just flag them as removed. When the rows are physically removed, we are reloading the entire array. It's more of a batch operation where the user makes all of their changes, then saves the changes.

When can we officially support the $watchCollection?

[wattsbn]

$watchCollection was added into AngularJS in version 1.1.4.
This version is still marked as unstable.
Their current stable build is version 1.0.7 and there are 5 other versions before 1.1.4.

So either we wait for it to become a stable version (not sure how long that takes) or start using the unstable version (which is not my call to make).

[swalters]

@wattsbn What if we added an if(isFunction($scope.$watchCollection))? Anyone on 1.1.4 or higher would benefit.

@roblarsen Too much of a hack? It would be a pain to test against older Angular versions.

[roblarsen]

Hi, thanks for this PR. As we move towards releasing 2.0.8 and then soon after moving to the new architecture of 3.0, we're going closing many issues and PRs. This will allow us to make better use of the issue tracker for the project moving forward. We really appreciate your input and hope to do a better job of managing PRs and issues with 3.0.