Skip to content

Conversation

kjlubick
Copy link

@kjlubick kjlubick commented Oct 9, 2014

First off, thanks for making a great localstorage module. I especially liked that it basically worked with everything, arrays and all, out of the box.

I had a complicated array structure that I wanted to store and as per http://stackoverflow.com/questions/14712089/how-to-deep-watch-an-array-in-angularjs
the advice was to use $watch instead of $watchCollection to deal with the deep comparision that I needed.

It's a relatively small change and I hope my documentation on bind is helpful to others as well.

It will be slower, but allows for binding an array objects.
http://stackoverflow.com/a/14713978/1447621
Documented the newly added feature and the other arguments as well.
@a8m
Copy link
Collaborator

a8m commented Oct 9, 2014

@kjlubick , Thanks.
please add test(s).

@kjlubick
Copy link
Author

kjlubick commented Oct 9, 2014

Added a test. Is that sufficient?

@a8m
Copy link
Collaborator

a8m commented Oct 10, 2014

@kjlubick would you please just merge these 4 commits into only one? thanks.

@kjlubick
Copy link
Author

Couldn't do it in one because of the two merge conflicts.

Conflicts:
	README.md
	src/angular-local-storage.js
@kjlubick
Copy link
Author

Ready to merge

@a8m
Copy link
Collaborator

a8m commented Oct 11, 2014

I think we should always use $watch(..., ..., true) instead of $watchCollection.
Because when we bind a non-primitive value we actually want tס do deep comparison.
@grevory What do you think ?

@kjlubick I think it's kinda messy to do something like that:
ls.bind($scope, 'key', null, 'key', true) instead of ls.bind($scope, 'key')
I dunno, 5 arguments it's too much for me.

@kjlubick
Copy link
Author

5 args is a bit much, I agree. I didn't make it default because the angularjs docs warned of a performance hit - but if it's not doing what we want anyway than we don't have too much of a choice.

@a8m
Copy link
Collaborator

a8m commented Oct 11, 2014

@kjlubick , we can go this way:

$scope.$watch(key, function(newVal) {
  //...
}, isObject($scope[key]));

so, if it's primitive value, we comparing for reference equality. so actually it really cheaper.

@grevory
Copy link
Owner

grevory commented Oct 11, 2014

I like it
On Oct 11, 2014 5:00 PM, "Ariel Mashraki" [email protected] wrote:

@kjlubick https://github.com/kjlubick , we can go this way:

$scope.$watch(key, function(newVal) {
//...}, isObject($scope[key]));

so, if it's primitive value, we comparing for reference equality. so
actually it really cheaper.


Reply to this email directly or view it on GitHub
#144 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants