CODEWITHSUNDEEP
angularjspromise
user2173353
user2173353

Reputation: 4640

AngularJS promise gets resolved with the wrong value

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?

Upvotes: 1

Views: 260

Answers (1)

Benjamin Gruenbaum
Benjamin Gruenbaum

Reputation: 276306

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.

Upvotes: 2

Related Questions