Search code examples
javascriptpromiseglobalmemoization

Is returning the same Promise to multiple callers wrong?


I created an AngularJS service for handling and caching requests to a given resource.

Once the request has already been completed, cachedRequest() returns Promise.resolve(), so that any .then() s connected can be immediately triggered.

However, if the request is not completed, but already initiated, cachedRequest() returns the same global Promise it created on the first call. Is it wrong? Does it create any memory leaks?

The purpose of the global Promise is to return to all the calls made before request completion at the same time. It doesn't look wrong to me.

The code works without errors:

// The request function which should only be called once
function request(callback) { console.log("Doing stuff only once..."); setTimeout(callback, 5000); }

// Holds global request status (completed/not completed)
var requestCompleted = false;
// Holds global promise
var returnPromise = null;

// The interface for the request function
function cachedRequest() {

  // Request results already available
  // Returning Promise.resolve() to trigger .then()s
  if(requestCompleted){
    return Promise.resolve(requestCompleted);

  // Request result not available (not initiated or not completed)
  }else{
    // Request was initiated
    if(returnPromise){
      // IS THIS WRONG?
      return returnPromise;
    
    // Request not initiated
    }else{
      // Creates and stores the promise 
      returnPromise = new Promise(function (resolve, reject) {
        request(function () {
          requestCompleted = true;
          resolve(requestCompleted);
        });
      });
      // Returns the promise
      return returnPromise;
    }
  }
}


/////////////////

function doRequestAndPrint(n) {
  console.log("Request " + n);
  cachedRequest()
    .then((value)=>{
      console.log("Returned " + n);
    })
}

//////////////////////////////

doRequestAndPrint(1);

setTimeout(()=>{doRequestAndPrint(2)}, 2000);

setTimeout(()=>{doRequestAndPrint(3)}, 10000);


Solution

  • Is it wrong?

    No. Caching promises is a good practice.

    Does it create any memory leaks?

    No. Of course it keeps the promise result in memory, but that's on purpose not a leak.

    Once the request has already been completed, cachedRequest() returns Promise.resolve()

    This is totally superfluous. It should just return the returnPromise like it already did since the request was initiated. Storing the requestCompleted in addition to the returnPromise just makes your code much more complicated.