alekseykulikov / backbone-offline

[Deprecated] Allows your Backbone.js app to work offline
MIT License
720 stars 56 forks source link

Duplicate Add Event #57

Open ghost opened 11 years ago

ghost commented 11 years ago

I'm using collections which persist in memory - I.E. not recreated each time I use them.

However, when I fetch a new model from the server, the collection gets 2 add events.

Flow seems to be:

  1. call fetch().
  2. get JSON from server.
  3. save to Localstorage.
  4. collection add event.

Then do something else, and then..

5 call fetch() again. 6 collection add event.

So you get another add event, even though there is no new data.

If you call fetch() a 3rd time, you don't get another add event. I.E. you get the event precisely twice.

Further information :

The model in step 4. has a 'cid', but it doesn't have an 'sid', or internal guid.

The model in step 6. does.

However, at step 3, the object in localstorage does have an 'sid'.

So - the following seems to be happening...

a) download model from server b) save to localstorage with new attributes c) add model with server attributes.

Maybe it should add to collection with the same attributes that are saved to localstorage?

Happy to fix this myself. Please direct me to the relevant functions in the source.

Best,

Dalip

ghost commented 11 years ago

I have found the cause of this behavior.

After an ajax 'read', items not yet in memory are passed to @collection.items.create(item, *args)

I.E. the create() method on the backbone collection. The following then happens:

  1. model created from item
  2. model added to collection with server attributes.
  3. ( collection add event )
  4. model.save()
  5. Model.save() triggers an Offline.sync() 'update'.
  6. model gets a local guid, and is saved to local-storage.

I'm not familiar with the framework, so may be missing something. But I thought create + PUT were for newly created entities, not all entities on the server (when they are first fetched).

Here - the purpose seems to be a way to get a 'cid' assigned by the collection, before saving the model to localstorage. I'm gussing this is a hack to avoid calling collection._prepareModel() from outside the collection.

But I can't yet think of a way to fix the 'double add-event' bug without changing the control flow.

Calling _prepareModel() outside collection is bad practice. But if you were flexible on this, would solve bug, and control flow would be much simpler + more intuitive.

One way to make this 'better' practice might be to implement Backbone.Offline using a special Collection subclass, and make a public gateway to _prepareModel().

I personally find this a nice way to implement Offline in my project anyway. I have an OnlineCollection class:

var OfflineCollection = Backbone.Collection.extend({

    storageName: 'default',

    initialize: function() {

        console.log('setting up storage for '+ this.storageName)

        this.storage = new Offline.Storage( this.storageName, this );

    }

});

Nice for DRY - just put storageName in subclass.

But maybe you guys want to touch as little of the Backbone prototypes as possible - which is understandable. Any other suggestions to fix this?

In short run I have deadline, so may have to make my own non-standard branch. In mid term, very much look forward to discussion - I'm happy to contribute to this project!

Dalip

paynecodes commented 11 years ago

This sounds very similar to an issue I'm having where the success method of particular fetch is called twice when the collection is not in localStorage. This in turn, causes an error for my control flow situation because it returns an empty collection on success the first time, then later returns the populated collection after the success magic has already happened with an empty collection.