-
Notifications
You must be signed in to change notification settings - Fork 585
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Watch for changes in local storage #166
Comments
Because of the serialisation of the objects, I think it may be useful to have an event or a callback to be able to detect that the binding has been done is from LS to the scope. I explain myself: {
a: 'a',
b: 'b',
myFn: function() {
console.log('test');
}
} After the serialisation, it will be:
So I've lost myFn of the scope which is normal. So if we have an event we can rebind that to the $scope variable and avoid the call to undefined function... What do you think about that? |
Or we could use : http://www.eslinstructor.net/jsonfn/ to avoid this issue ? Don't know what's better.. Maybe this one |
@YouriT Personally, I don't like the idea of broadcasting an event - you have to know to listen for the event (which is not immediately obvious), it may not be possible to simply re-bind the function, the event name is a magic string, etc. I'm not sure it would be possible to serialize functions, other than extremely simple ones (that library you linked to just calls .toString() on the function). I'd say you shouldn't have any unserializable properties on the bound object in the first place. Anyone else want to weigh in on this? |
I agree on the serialisation of functions may be tricky with complex one, that's why I opted for the event broadcasting. Why shouldn't I have that ? On my side it perfectly makes sense because it's context dependant and I don't want to each time inject the data to a function that will then do things in an appropriate form. If you don't like the idea of the broadcasted event, it could be a callback on the bind function and then it would be maybe more straightforward don't you think @DaveWM ? |
I've updated my PR, when local storage is updated it now extends the bound object rather than overwrites it. This should preserve any unserializable properties, and saves you having to wire up events all over the place. How does that sound to you @YouriT ? |
Sounds quite good. Didn't think about that. Thanks ! |
What would happen to deleted properties? |
If you delete a property of the object in local storage, it won't be removed from the javascript object. I can see how this could be confusing, anyone else have any thoughts on this? |
Maybe some markup could help ? I changed anyhow the way it works on my side because it's hard to keep consistency across different pages... I would rather go just for a proper event listening like an additional parameter to the bind function. Something else, I saw that you used addEventListener, don't you wanna use a more cross browser function ? Like this one: function addEventListener(obj, evt, fnc) {
// W3C model
if (obj.addEventListener) {
obj.addEventListener(evt, fnc, false);
return true;
}
// Microsoft model
else if (obj.attachEvent) {
return obj.attachEvent('on' + evt, fnc);
}
// Browser don't support W3C or MSFT model, go on with traditional
else {
evt = 'on' + evt;
if (typeof obj[evt] === 'function') {
// Object already has a function on traditional
// Let's wrap it with our own function inside another function
fnc = (function(f1, f2) {
return function() {
f1.apply(this, arguments);
f2.apply(this, arguments);
};
})(obj[evt], fnc);
}
obj[evt] = fnc;
return true;
}
return false;
}; |
@YouriT Listening for events isn't really the "angular way", you should be able to rely on the binding alone. What exactly are you trying to "keep consistency" of? addEventListener is implemented in IE9, so I don't think we need to worry about this. |
duplicate of #84 |
When you call localStorageService.bind, it would be nice if it then listened for changes to the local storage key it binds to, and updated the bound object (as suggested by @lieryan in #131). This can be done by registering an event listener for the "storage" event. PR incoming.
The text was updated successfully, but these errors were encountered: