AngularJS promise gets resolved with the wrong value

Question

I was planning on making a service that caches server response data in AngularJS and this is what I did:

function addDataCachingServiceToModule(module) {
   module.factory('myDataCaching', function ($q, itemRepository) {

    var categoriesWithNewItems = undefined;

    function getCategoriesWithNewItems(date) {
        var deferred = $q.defer();
        if (!categoriesWithNewItems) {
            return itemRepository.getCategoriesWithNewItems(date)
                .then(function (res) {
                if (res.data.Data.Success) {
                    categoriesWithNewItems = res;
                    deferred.resolve(res);
                } else {
                    deferred.reject(res);
                }
            });
        } else {
            deferred.resolve(categoriesWithNewItems);
        }
        return deferred.promise;
    }

    function resetCategoriesWithNewItems() {
        categoriesWithNewItems = undefined;
    }

    return {
        getCategoriesWithNewItems: getCategoriesWithNewItems,
        resetCategoriesWithNewItems: resetCategoriesWithNewItems
    };
});
}

To my shock, it seems that while this works normally, when I try to use it like this:

myDataCaching.getCategoriesWithNewItems(date)
    .then(function(res2) {
            // res2 = undefined here !!!
    });

..I get undefined instead of the data that I pass in in deferred.resolve(res);.

I have debugged this and it calls the 1st deferred.resolve(res); in my service with valid data, but in my then() I get undefined instead.

It never passes through any of the other resolve()/reject() calls.

Does anyone have any idea what's wrong here?

why using two return statement, try removing return from 'return itemRepository.getCategoriesWithNewItems(date)' — Anant, Sep 22 '14 at 09:52
@Anant Oh my...! Totally missed that... That was the problem. :( — user2173353, Sep 22 '14 at 09:53

score 2 · Accepted Answer · edited May 23 '17 at 12:29

So, Anant solved your missing return issue. Let's discuss how you won't have it any more in the first place :)

Promises chain, your current code has the deferred anti pattern which we'd rather avoid. You're creating excess deferred which you shouldn't. Your life can be a lot easier:

function addDataCachingServiceToModule(module) {
   module.factory('myDataCaching', function ($itemRepository) {

    var cats = null; // prefer null for explicit lack

    function getCategoriesWithNewItems(date) {
        // are we sure we don't want to cache by date? 
        return cats = (cats || itemRepository.getCategoriesWithNewItems(date))
    }

    function resetCategoriesWithNewItems() {
        categoriesWithNewItems = null;
    }

    return {
        getCategoriesWithNewItems: getCategoriesWithNewItems,
        resetCategoriesWithNewItems: resetCategoriesWithNewItems
    };
});
}

So, what did we do here:

Cache the promise and not the value to avoid race conditions. In your original version making multiple requests before the first one returned and then updating the cache multiple times which would have made deleting not work.
Avoid creating an excess deferred, this is faster and cleaner too.

Nice! :) I have already changed mush of the code concerning your notice : `cache by date` ;) — user2173353, Sep 22 '14 at 14:00
Caching the promise didn't pass through my mind. :) Thanks. :) — user2173353, Sep 22 '14 at 14:10
If it makes you feel any better, I've done it tens of times and it didn't pass my mind either today when I wrote similar code today (in Swift). So your question probably saved me quite some time too :) — Benjamin Gruenbaum, Sep 22 '14 at 14:17

AngularJS promise gets resolved with the wrong value

1 Answers1