Problem with backbone.localstorage on some Androids

[Sergeyshall]

Hi Jerome!

Found problem with methods .find of your backbone.localstorage plugin on few Androids (Android 2.3.5 Samsung Galaxy S2 and some other).
The problem connected with JSON.parse(null) of find method. With null - it cause an error "Uncaught illegal acces".

Could propose a fix with validation before parsing:

// Retrieve a model from this.data by id.
find: function(model) {
var storageData = this.localStorage().getItem(this.name+"-"+model.id);
if (storageData) {
//storageData null cause error of JSON.parse(null) on some androids
return JSON.parse(storageData);
} else {
return null;
}
},

// Return the array of all models currently in storage.
findAll: function() {
return _(this.records).chain()
.map(function(id){
var storageData = this.localStorage().getItem(this.name+"-"+id);
if (storageData) {
//storageData null cause error of JSON.parse(null) on some androids
return JSON.parse(storageData);
} else {
return null;
}
}, this)
.compact()
.value();
},

[garrettlangley]

confirming this is a bug as well.

[jferguson]

Same here. Fix from Sergeyshall resolved the issue for me.

[jeromegn]

Wow, I have no been receiving the email notifications of new issues. I'm sorry if that took a while.

Would you mind submitting a pull request with that code? Maybe even write a test for it :)

[fiznool]

My two cents - I've come across this problem before too, but my suggested approach would be to duck-punch JSON.parse. This way we don't need to add the same null checks every time we are parsing JSON. Pull Request #41 only fixes the case of JSON.parse within this library - what if I need to do JSON.parse in my own code?

For example:

brianleroux/lawnchair#48 (comment)

[nerdgore]

We've come across this issue as well. In our own code it's trivial to implement a null check like value && JSON.parse(value);. To keep the fixing code separate we added jsonData().

find: function(model) {
  // call to this.jsonData instead of JSON.parse
  return this.jsonData(this.localStorage().getItem(this.name+"-"+model.id));
}

findAll: function() {
  return _(this.records).chain()
    .map(function(id){
      return this.jsonData(this.localStorage().getItem(this.name+"-"+id));
    }, this)
    .compact()
    .value();
}

and as above the actual handling is simple

jsonData: function (data) {
  return data && JSON.parse(data);
}

[jeromegn]

@nerdgore If you'd like to submit that as a pull request, it'd be greatly appreciated.