Updating PR #190 (ModelSync.Local) with Shifter

[clarle]

This is an update/refactor to #190, which has been a stale PR for a while.

It's a few bug fixes I've received from the Gallery version of ModelSync.Local since originally submitting that PR, more complete unit tests, as well as updated Model documentation. I've also updated it to play nicely with Shifter, since the originally PR was submitted while still using the old builder.

I'll be at the roundtable tomorrow, if there's time to discuss it. Thanks!

[jenny]

Per discussion at 1/3/13 Roundtable, can you add a test for handling full local storage?

[clarle]

@ericf, is expected behavior when localStorage is full to throw an error, or to continue paging in memory?

[ericf]

@clarle good question. I feel that throwing an error in code running in production is something that we should avoid. This is a good candidate for following prior-art. What do Backbone and Ember Data do in this case?

[clarle]

Backbone.localStorage (https://github.com/jeromegn/Backbone.localStorage) doesn't have a test case for this scenario, opting to leave it to the developer to not have that happen.

Ember Data's localStorage adapter (https://github.com/rpflorence/ember-localstorage-adapter) triggers a QUOTA_EXCEEDED_ERR event, which allows the developer to supply a callback to properly handle it. I think I like this approach, since it gives the most flexibility.

[ericf]

Yeah, I like Ember Data's approach too, providing the hook will allow the developer to decide and we don't have to do something destructive by default.

[ericf]

@clare were you still looking into this? Let me know if you have time to implement the exceeding quota exception.

[clarle]

I can get to it during the weekend. Sorry for the delay, it's been busy lately.

[ericf]

No worries, thanks.

[clarle]

Sorry for the delay on this, and I understand if it doesn't make it in for the PR feature-complete deadline.

There's no way I could find to fully simulate a DOMException, which the QUOTA_EXCEEDED_ERR is one of, but I added a test case that generates an exception during the _save method, which writes the data into localStorage. I can't think of any other possible way to generate a real QUOTA_EXCEEDED_ERR without actually filling up localStorage.

This particular test case should be enough to represent that scenario though.

[ericf]

Thanks @clarle! I'll be reviewing this for inclusion in 3.9.0, finally :)

[clarle]

I haven't attached the build files, but let me know if you need anything else. Cheers!

[ericf]

Working on merging this into dev-3.x now…

[ericf]

Spent some time trying to get this merged in, but the tests are failing in IE6–8. I have to hold off on merging until that is resolved.

[clarle]

I understand, and I'll see what I can do to get it in for the next cycle. Sorry for taking up your time, and I'll do more testing on the lower IE versions as soon as I can.

[ericf]

@clarle we can coordinate on it. IE 9 & 10 worked great, IE 8 has some weird issue I could figure out what was causing it, and IE 6 had a lot of test failures. I also have some other minor changes locally. On Tuesday morning (Monday is a holiday for us) I'll issue a pull request to you with my tweaks.

[clarle]

@ericf Sorry if you're busy, but could I look at the tweaks sometime? I've been spinning up some IE VMs, and I see the problems that you were talking about.

[clarle]

Closed due to staleness for now.