superfeedr / indexeddb-backbonejs-adapter

An indexedDB adapter for Backbonejs
http://blog.superfeedr.com/indexeddb-backbonejs-adapter/
MIT License
248 stars 61 forks source link

Issue with ordering of sync and resolve in callbacks #46

Closed nmartin413 closed 10 years ago

nmartin413 commented 11 years ago

Wonderful work going on here!

However, I think I might have stumbled across a bit of a logical issue. It seems the sync function is resolving it's deferreds before it actually syncs the model up:

var success = options.success;
options.success = function(resp) {
    resolve();
    if (success) success(resp);
    object.trigger('sync', object, resp, options);
};

Which causes something like this to fail:

it("should save a question", function (done) {
    var question = new Model.Exam.Question();
    question.save({ id: 1000 }).then(function () {
        var collection = new Model.Exam.QuestionCollection();
        collection.fetch().then(function () {
            var target = collection.get(1000);
            target.should.be.an('object');
            done();
        });
    });
});

Modifying my local copy of backbone.indexeddb.js to reorder the sync and resolve causes this test to pass. Is this something that should be changed? (I believe the error callback also would be considered...)

julien51 commented 11 years ago

Hum. Did the test fail previously for you?

nmartin413 commented 10 years ago

Yes, without the re-ordering

NadaAldahleh commented 10 years ago

I have the same issue. .done() can get fired before it's actually successful. Re-ordering the two lines as below seems to fix it.

resolve();
if (success) success(resp);
julien51 commented 10 years ago

Please, feel free to submit a Pull Request so I can close that issue :)