How to properly concatenate promises in different functions-CodePudding

I'm a newbie and trying to figure out something in Javascript that should be simple. I have 2 functions let's say

function play1(){
  Promise.resolve()
    .then(() => put('A', 1000))
    .then(() => put('B', 1000))
}

function play2(){
  Promise.resolve()
    .then(() => put('C'), 1000)
    .then(() => put('D'), 1000)
}

I need a third function so that it executes sequentially A, B, C, D What I've tried so far with no luck:

function playAllSequentially(){
  
  Promise.resolve()
    .then(() => play1())
    .then(() => play2())
}

but this doesn't get the job done, of course I could do

Promise.resolve()
    .then(() => put('A', 1000))
    .then(() => put('B', 1000))
    .then(() => put('C', 1000))
    .then(() => put('D', 1000))

but that is not the idea

in case it matters the content of put() is

function put(text, duration){
    $('#txtRemarks').text(text);
    delay(duration);
}

Thanks in advance

CodePudding user response：

It sounds like delay returns a promise it fulfills after a period of time. But put is completely ignores that promise, so it doesn't wait. Similarly, play1 and play2 don't return anything, so there's no reason for whatever is calling them to wait either. You need to return the promise chain. (Separately, you have the closing ) in the wrong place in the calls to put in play2.)

See the marked changes (but keep reading):

function play1() {
    return Promise.resolve() // <=== return
        .then(() => put("A", 1000))
        .then(() => put("B", 1000));
}
function play2() {
    return Promise.resolve() // <=== return
        .then(() => put("C", 1000))  // <=== parens
        .then(() => put("D", 1000)); // <=== parens
}

function playAllSequentially() {
    return Promise.resolve() // <=== return
        .then(() => play1())
        .then(() => play2());
}

function put(text, duration){
    $('#txtRemarks').text(text);
    return delay(duration); // <=== return
}

playAllSequentially();

function delay(duration) {
    return new Promise((resolve) => {
        setTimeout(resolve, duration);
    });
}

<div id="txtRemarks"></div>

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>

But, that code has the explict promise creation anti-pattern. You don't need those Promise.resolve() calls if put, play1, and play2 return promises:

function play1() {
    return put("A", 1000)
        .then(() => put("B", 1000));
}
function play2() {
    put("C", 1000)
        .then(() => put("D", 1000));
}

function playAllSequentially() {
    return play1()
        .then(() => play2());
}

function put(text, duration){
    $('#txtRemarks').text(text);
    return delay(duration); // <===
}

playAllSequentially();

function delay(duration) {
    return new Promise((resolve) => {
        setTimeout(resolve, duration);
    });
}

<div id="txtRemarks"></div>

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>

But, here in 2022, that's not how I'd write this code. Instead, I'd use async functions:

async function play1() {
    await put("A", 1000);
    await put("B", 1000);   // Maybe a return here, if you want play1
                            // to return the result of `put`
}
async function play2() {
    await put("C", 1000);
    await put("D", 1000);   // See above
}

async function playAllSequentially() {
    await play1();
    await play2();          // See above
}

async function put(text, duration){
    $('#txtRemarks').text(text);
    await delay(duration);
}

playAllSequentially();

function delay(duration) {
    return new Promise((resolve) => {
        setTimeout(resolve, duration);
    });
}

<div id="txtRemarks"></div>

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>

Finally, in all of the above the functions never reject their promises, but real world functions usually can, so playAllSequentially (or whatever calls it) should handle rejection.

CodePudding user response：

You need to make sure all the functions play1, play2 etc to return promises, so that the then chain waits for returned promise's resolve callback.

function play1(){
  return Promise.resolve()
    .then(() => put('A', 1000))
    .then(() => put('B', 1000))
}

Promise.resolve()
  .then(() => play1())
  // Since play1() returns a promise, following `then` would wait for it's resolve callback
  .then(() => ...)

CodePudding user response：

The problem with your code is that your functions aren't returning the Promises. You have 2 really easy fixes here:

1. Return the Promises manually

All you need to do is something like:

function play1(){
  return Promise.resolve()
    .then(() => put('A', 1000))
    .then(() => put('B', 1000))
}

function play2(){
  return Promise.resolve()
    .then(() => put('C'), 1000)
    .then(() => put('D'), 1000)
}

Also presumably you need to return delay as well in put depending on how delay works

function put(text, duration){
    $('#txtRemarks').text(text);
    return delay(duration);
}

Summary

You should always be returning your promises or else you will end up with tons of hanging promises in memory that may or may not get executed when you want them to.

Example

In this code:

const fn = () => {
    const promise = fetch("https://some-url");
    for (let i = 0; i < 1000000000000; i  ) {
        doSomeExpensiveTask();
    }
}

The promise isn't going to resolve before the for loop. The promise should get resolved after all of your imperative code but maybe not as there might be a lot of repainting to be done or something. The only way to know when your promises are resolved is by using the patterns mentioned.

2. Use `async` `await`

The more idiomatic way of doing this in javascript is to rewrite your functions as async functions and then await the promises

async function play1(){
  await put('A', 1000);
  await put('B', 1000);
}

async function play2(){
  await put('C');
  await put('D');
}

The delay function:

async function put(text, duration){
    $('#txtRemarks').text(text);
    await delay(duration);
}

Then you could change your usage to be:

async function playAllSequentially() {
    await play1();
    await play2();
}

although returning would also work here. You can mix and match these patterns as the async/await is just syntactic sugar for what I showed before.

CodePudding user response：

I executed the script and the errors are two. 1. You called the function delay which is undefined in javascript, maybe you refer to setTimeout(). 2. You defined the function put with two parameters, but here: put('C'), 1000) put('D'), 1000) you called put with 1 parameter, in this example 'c' and 'd'. So the answer is yes, playAllSequentially() works if you fix these errors. In my case I executed the code with Node.js and works correctly.

1. Return the Promises manually

Summary

Example

2. Use async await

2. Use `async` `await`