NathanaelA / nativescript-localstorage

NativeScript LocalStorage API
Other
75 stars 32 forks source link

The getItem function may return incorrect values #5

Closed pitAlex closed 7 years ago

pitAlex commented 7 years ago

Hi,

I see you are doing this:

      getItem: function (name) {
            if (localStorageData[name]) {
                return localStorageData[name];
            }
            return null;
        }

However if localStorageData[name] is 0, false or "" then getItem(name) will return null. You should change it something like this:

      getItem: function (name) {
            if (localStorageData.hasOwnProperty(name)) {
                return localStorageData[name];
            }
            return undefined; // null can also be intentionally set so its best to just give "undefined"
        }

Update: In fact I don't see any harm in just doing :

      getItem: function (name) {
            return localStorageData[name];
        }

If the key doesn't exist the result will be "undefined".

NathanaelA commented 7 years ago

Good catch!

terreb commented 7 years ago

I hit the issue related to this. I need to store an empty string value. getItem returns null however should return ''.

DanielKucal commented 7 years ago

I think the result of getting not-existing variable should be null, just like in web browsers. There is no way for local- and sessionStorage to return undefined... @terreb, have you tried to JSON.stringify() the data before save and then JSON.parse() to get the desired value? This is how frontend libraries handle it. If you are writing Angular app, then you can also try my lib ngx-store when #9 will be merged.

NathanaelA commented 7 years ago

@DanielKucal - Your are correct, null is what is supposed to be returned when it doesn't exist. I will update the getItem function to return null rather than undefined. Thanks.

terreb commented 7 years ago

@DanielKucal, why would I need to JSON.stringify a string?

DanielKucal commented 7 years ago

@terreb, in general you should not, but there was a bug about it and I think I've also noticed some issue with it recently... It was like some value was changing after site reload. However I agree that localStorage.setItem('test', ''); localStorage.getItem('test')) should return "".

NathanaelA commented 7 years ago

The newest published version has both this fix (null instead of undefined). And your patch for Storage object..