Moshe Shmukler
Moshe Shmukler

Reputation: 1300

Correct promise chaining

I am working on a Chrome extension where I need to contact from the page extension code with background script using Chrome messages. This adds a fair level asynch fiddling to my code. The general purpose is to process text macros ie parse text, using data stored in the database accessible from the background script.

at some point, I have:

findMacro(key)
.then(result => {
   processMacro(key, result);
}};

The processor roughly looks like:

function processMacro(shortcut, text) {
  text = macroBuilder(text);
}

macroBuilder function processes various types of macros, pushing processed text to an array then running join(''). My problem is that, I need to support nested macros and when there are such macros, I call findMacro again, which inside does chrome sendMessage to the background process. The processor does something like:

function macroBuilder(text) {
  let pieces = [];
  macroProcessorCode(text).forEach(res => {
    res.visit({
      text(txt, ctx) {
        pieces.push(txt);
      },
      dates(obj, ctx) {
        pieces.push(processDates(obj))
      },
      ...
      nested(obj,ctx) {
        let fragments = [];
        fragments.push(
          findMacro(obj.name)
          .then(res => {
            return macroBuilder(res);
          })
        );
        console.log('promised results:', fragments);
        Promise.all(fragments)
        .then(fragments => {
          console.log('resolved results:', fragments);
          pieces.push(fragments);
        });
      }
    });
  });
  return pieces.join('');
}

For some reason my function returns before resolving, so promised results happens before it returns, and resolved results after. In short, I return from the code processing text with the result, before nested macros are processed. This only happens with nested, other types of macros are processed correctly.

Any ideas?

Upvotes: 1

Views: 65

Answers (1)

T.J. Crowder
T.J. Crowder

Reputation: 1074295

macroBuilder creates a bunch of promises but never does anything with them. Instead, it needs to wait for them and return its own promise that will resolve/reject based on the promises for the pieces/fragments.

This is somewhat off-the-cuff and probably needs tweaking, but should get you going the right direction. See *** comments:

function macroBuilder(text) {
    // *** An array for all the promises below
    const piecePromises = [];
    macroProcessorCode(text).forEach(res => {
        res.visit({
            text(txt, ctx) {
                pieces.push(txt);
            },
            dates(obj, ctx) {
                pieces.push(processDates(obj))
            },
            //...
            nested(obj, ctx) {
                let fragments = [];
                fragments.push(
                    findMacro(obj.name)
                    .then(res => macroBuilder) // *** Note we can just pass `macroBuilder` directly
                );
                console.log('promised results:', fragments);
                // *** Add this promise to the array
                pieces.push(Promise.all(fragments)
                    // (*** This whole then handler can be removed
                    // once you don't need the logging anymore;
                    // just push the result of Promise.all)
                    .then(fragments => {
                        console.log('resolved results:', fragments);
                        return fragments;
                    })
                );
            }
        });
    });
    // *** Wait for all of those and then join; ultimate result
    // is a promise that resolves with the pieces joined together
    return Promise.all(piecePromises).then(pieces => pieces.join(''));
}

At that point, there's not much point to processMacro; it would just look like this (note that it returns a promise):

function processMacro(shortcut, text) {
  return macroBuilder(text);
}

...unless there's something you do with shortcut that you haven't shown.

Assuming you need processMacro, you'd call it like this if you're propagating the promise to the caller:

return findMacro(key)
.then(result => processMacro(key, result));

...or like this if you're not propagating the promise:

findMacro(key)
.then(result => processMacro(key, result))
.catch(err => {
   // Deal with the fact an error occurred
});

Since one of the rules of promises is that you either propagate the promise or handle errors from it.

Upvotes: 1

Related Questions