Need to clone objects before storing in storage object

[odedbd]

Hi,

First let me thank you for the wonderful little tool you've created, very useful!

I have run into a minor issue, which can be easily fixed (I'm no git user, so I didn't create a pull req for this).

in set:

    set: function(key, value){
        _checkKey(key);
        if(_XMLService.isXML(value)){
            value = {_is_xml:true,xml:_XMLService.encode(value)};
        }
        _storage[key] = value;
        _save();
        return value;
    }

If the value is an object, and it is later changed, the _storage cache will reflect those changes (while the localStorage won't). This is an issue, since the stored object may (and in my application does) acquire non-json-serializable properties (such as jQuery wrapped elements). when ever some other key is changed and the entire _storage object needs to be serialized, things break.

my fix was quite simple, perhaps there's a better way to go about this-

set: function(key, value){
        var clone;
        _checkKey(key);
        if(typeof(value) === 'object'){
            clone = $.extend(true, {}, value);
        }else{
            clone=value;
        }
        if(_XMLService.isXML(clone)){
            clone= {_is_xml:true,xml:_XMLService.encode(clone)};
        }
        _storage[key] = clone;
        _save();
        return clone;
}

Please note that this uses jQuery's extend function explicitly, I have no idea whether this could roll with other libraries. This works fine for me, so I hope it might help others as well.

Thanks again, o

[andris9]

Hi,

As jStorage supports many different libraries besides jQuery (Prototype, MooTools or any other with JSON support) then using jQuery specific methods is not acceptable.

There are some issues that can't be solved by the library. For example when an object includes a DOM node as a property, then JSON serialization in Chrome breaks big time. It is not feasible for jStorage to walk the entire node tree of an object to check if its properties can be saved or not, as it indroduces severe overhead for larger objects. Then there's circular structures etc. Adding such complexity to jStorage isn't probably the best thing to do.

[odedbd]

Hi Andris,

I totally understand about the jQuery specific method, as I have written in my issue.

The main point here is not so much serialization of methods/nodes. In fact, it isn't event related to working with localStorage, strictly speaking. It's simply the way the internal object _storage keeps, for object, references to the actual objects passed to the function. this is a problem, since this may lead to mismatches between what's in the localStorage and what's in the _storage object. In case that the object passed to set has changed and no set was called yet, _storage will reflect this change, while localStorage would not.

I understand why you consider that this cannot be solved in the manner I did within the library, but I would like to suggest two other options that might better suite:

1) just document this issue, and make it clear to users of the library that they are responsible in cloning (or not changing) objects that they set as values in order to prevent this. this is a non-trivial bug to debug, since it usually comes back to bite you only when you are trying to set some other key, which may not even be an object, but just triggers a save, which tries to serialize a previously set object which changed after setting, and had non jsonable properties added to it.

2) it is also possible to simply not save objects into the _storage cache in the first place, so that they are always read from the localStorage. this can have serious performance costs, so I guess it is not preferable.

As for myself, I intend to wrap up the set function with some facade that will implement this logic (using jquery's extend), since it will solve my specific issue. I think that my option 1) here is a good idea, and I hope you might agree, for the sake of future users of the library.

Thanks again for this excellent piece of code that helps make my life as a developer a little easier :)

[andris9]

I think I came up with a solution which satisfies everybody. The value is cloned on save by de/jsonizing it. This also fixes the issue of function values and object methods in saved data being accessible after save but not after page refresh. a84c1aa2900b7cf2126729a71e2f8d461177f2ec

[odedbd]

This sounds like a great idea. I will let you know if I run into any trouble with it.

Thanks. for your work and for your patience.