Do not override Backbone.sync

[rubennorte]

I think Backbone adapters shouldn't modify the original sync function since it can be a important source of incompatibilities with other libs. It could just provide the sync function to be used on the selected models and collections (or manually overriding the default function if we want to).

Here's an example of a better approach IMO:

var LocalModel = Backbone.Model.extend({
  sync: Backbone.localforage.sync,
  offlineStore: new Backbone.localforage.Store('name')
});

Any ideas?

[tofumatt]

If it’s that simple that works for me. I don’t like the override we do but I haven’t ever built a big Backbone app with multiple sync adapters, so I’m a bit out of my depth on what the best practice is.

[rubennorte]

I'm sure the current implementation works for most users, but with a little change it can work for everyone. It just occurred to me a simpler usage:

var LocalModel = Backbone.Model.extend({
  sync: Backbone.localforage.sync('storename') // Use a store for the model
});

var OtherLocalModel = Backbone.Model.extend({
  sync: Backbone.localforage.sync() // Use the model's collection store
});

I can send a pull request if there's consensus about this.

[tofumatt]

Oh wow, I really like that. That would be amazing 👍

[magalhas]

I like that as well.

[tofumatt]

Fixed in ff38f31

[magalhas]

I'll fix the Backbone example according to this changes.

[tofumatt]

I already did, no worries :-)

Matthew Riley MacPherson (Sent from mobile)

On Feb 22, 2014, at 17:32, José Magalhães notifications@github.com wrote:

I'll fix the Backbone example according to this changes.

—
Reply to this email directly or view it on GitHub.