Javascript Promise Anti-pattern

Question

I have a Promise, but it is not working as I want because the async call is not finished when I try use the data.

I have been looking here at Promise ant-patterns, and I am sure this is my fix. However, because I am weak at Javascript, I am struggling to implement their suggestion. If anyone can help, I would appreciate it.

My code:

  private findLocalChatsWithLastMessageForChat(): Promise<Mongo.Collection<Chat>> {
    let promise: Promise<Mongo.Collection<Chat>> = new Promise<Mongo.Collection<Chat>>(resolve => {
      let localChatCollection: Mongo.Collection<Chat> = new Mongo.Collection<Chat>(null);
      for (let i: number = 0; i < this.chatsStorageService.chats.length; i++) {
        let chat: Chat = this.chatsStorageService.chats[i];
        let findLastMessageForChatPromise: Promise<Message> = this.chatsStorageService.findLastMessageForChat(chat);
        findLastMessageForChatPromise.then((data) => {
          let message: Message = data;
          chat.lastMessage = message;
          chat.lastMessageCreatedAt = message.createdAt;
          localChatCollection.insert(chat);
        });
      }
      resolve(localChatCollection);
    });
    return promise;
  }

As you can see, the promise returns to the calling function, before the this.chatsStorageService.findLastMessageForChat promises are finished.

Reading here, offers this solution:

function workMyCollection(arr) {
    return q.all(arr.map(function(item) {
        return doSomethingAsync(item);
    }));    
}

However, I don't know how to amend my Typescript code.

Thanks

Steeve Pitis · Accepted Answer · 2016-10-12T09:10:43.657

The problem here is your resolve(localChatCollection) which is resolving your Promise without waiting for previous promises.

You have to store all your Promise in an array and wait them all before resolving.

Note that I don't know TypeScript I let you translate if I'm wrong about the syntax.

 private findLocalChatsWithLastMessageForChat(): Promise<Mongo.Collection<Chat>> {
    let promise: Promise<Mongo.Collection<Chat>> = new Promise<Mongo.Collection<Chat>>(resolve => {
      let localChatCollection: Mongo.Collection<Chat> = new Mongo.Collection<Chat>(null);

      // -----------------
      // ARRAY OF PROMISES 
      let promises: Array<Promise> = [];
      // -----------------

      for (let i: number = 0; i < this.chatsStorageService.chats.length; i++) {
        let chat: Chat = this.chatsStorageService.chats[i];
        let findLastMessageForChatPromise: Promise<Message> = this.chatsStorageService.findLastMessageForChat(chat);

        // -----------------
        // PUSH ALL YOUR PROMISES IN promises ARRAY
        promises.push(findLastMessageForChatPromise);
        // -----------------

        // Binding 'chat' in order to don't loose it.
        findLastMessageForChatPromise.then(function (_chat, data) {
          let message: Message = data;
          _chat.lastMessage = message;
          _chat.lastMessageCreatedAt = message.createdAt;
          localChatCollection.insert(_chat);
        }.bind(null, chat));
      }

      // -----------------
      // WAIT FOR ALL PROMISES BEFORE RESOLVING
      Promise.all(promises).then(function() {resolve(localChatCollection);});
      // -----------------

    });
    return promise;
  }

hum there will be some problem with your `chat` variable. I will update my answer — Steeve Pitis, Oct 12 '16 at 09:08
You still need to avoid the [`Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572)! — Bergi, Oct 12 '16 at 11:13

Javascript Promise Anti-pattern

1 Answers1