Skip to content
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

LocalStorage and 0.9.10 Collection update #73

Closed
dlferro opened this issue Feb 19, 2013 · 13 comments
Closed

LocalStorage and 0.9.10 Collection update #73

dlferro opened this issue Feb 19, 2013 · 13 comments

Comments

@dlferro
Copy link

dlferro commented Feb 19, 2013

At the moment it doesn't appear that the LocalStorage extension does anything with the new collection smart update to save back to localStorage. Is there plans to support it or am I missing something?

@jeromegn
Copy link
Owner

I'll need more information.

The new collection smart update? Point me to lines of code, a diff or at least a change log entry.

All tests pass using 0.9.10, so this must be a new feature I'm not aware of. Please explain it :)

@dlferro
Copy link
Author

dlferro commented Feb 19, 2013

Sure.

http://backbonejs.org/#Collection-update.

It could be possible that I'm completely misunderstanding the functionality.

@Krivega
Copy link

Krivega commented Feb 19, 2013

I do not have access to commit to the repository, but the latest version of Backbone requires the following changes:

     if (options && options.success)
-      if (Backbone.VERSION === "0.9.10") {
-        options.success(model, resp, options);
-      } else {
-        options.success(resp);
-      }
+      options.success(resp);

@jeromegn
Copy link
Owner

@Krivega Yes, but some people could still be using 0.9.10... going to leave that for a little while.

@Krivega
Copy link

Krivega commented Feb 19, 2013

Yes, but if use https://github.com/documentcloud/backbone/blob/master/backbone.js (general change in commit jashkenas/backbone@e4c046c )

@Krivega
Copy link

Krivega commented Feb 19, 2013

it would be good for this purpose set a new version of Backbone(0.9.11)

@jeromegn
Copy link
Owner

@Krivega we don't follow master, we follow the stable releases.

@dlferro
Copy link
Author

dlferro commented Mar 21, 2013

With the official reason of 1.0 will there be any plans to support http://backbonejs.org/#Collection-set to sync LocalStorage?

@jeromegn
Copy link
Owner

jeromegn commented Apr 1, 2013

@dlcerva Does it require anything different in the sync method? I don't see it right off the bat.

@jeromegn
Copy link
Owner

Would you mind writing a test for this? I will, but it might take some time before I get around doing it.

Once I have a failing test, the fix should be easy.

@marsch
Copy link

marsch commented Jul 3, 2013

@jeromegn any progress on this?

@jwilm
Copy link

jwilm commented Jul 3, 2013

@jeromegn weighing in that I too am interested in this. Thanks!

In the meantime, I have extended my local storage collection with a save method. My use case is simplified since all of the models are updated and never deleted from the collection.

Backbone.Collection.extend({
  // localStorage, model, etc
  save: function() {
    this.each( function(model) {
      Backbone.localSync("update", model)
    })
  }
})

I just call it manually after set, but listening for add/change/remove events and proxying to save should work for more complicated use cases. You would of course need to add an optional argument which is a model and sync action to alter behavior.

@Krivega
Copy link

Krivega commented Jul 3, 2013

you really need tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants