Search code examples
javascriptes6-promise

Using JavaScript Promise All - is this syntax valid?


I have two tables with users, where each id for one user is same in both tables (don't ask why I have two user tables). At some point, I need to filter users from table 1, and if certain condition is true, I store a promise (deleting request) for each user into (let's call it) tableOnePromises. I do the same for table 2. In order to empty table 2, I MUST first empty table one due to some requirements. this is what I did:

let tableOnePromises = [];
let tableTwoPromises = [];

tableOne.forEach(item => {
  if(item.deactivated) {
    const tableOneDeleted = supabase
      .from("table-one")
      .delete()
      .match({id: item.id});

    tableOnePromises.push(tableOneDeleted);

    const tableTwoDeleted = supabase
      .from("table-two")
      .delete()
      .match({id: item.id});

    tableOnePromises.push(tableTwoDeleted);
  }
});

await Promise.all(tableOnePromises).then(() => {
  return Promise.all(tableTwoPromises)
}).catch(err => console.log(err));

Solution

  • Assuming the code using await is inside an async function (or at the top level of a module), the syntax is correct, but it's probably not what I'd use (in general, avoid mixing async/await with explicit callbacks via .then and .catch), and separately it's probably not working quite as you expect (this is borne out by your saying that your code was failing to delete from table-two).

    For any particular id value, your code starts deleting from table-one and then immediately starts deleting from table-two without waiting for the deletion in table-one to complete:

    // STARTS the deletion but doesn't wait for it to finish
    const tableOneDeleted = supabase
      .from("table-one")
      .delete()
      .match({id: item.id});
    // ...
    // Starts deleting from `table-two`, even though the item may still be in `table-one`
    const tableTwoDeleted = supabase
      .from("table-two")
      .delete()
      .match({id: item.id});
    

    Remember that a promise is just a way of observing an asynchronous process; by the time you have the promise, the process it's observing is already underway.¹ So even though you don't wait for the table-two promises until later, you start the table-two deletions immediately.

    ...I MUST first empty table one due to some requirements...

    If by "empty" you mean just that you have to ensure you've done the delete for a particular id on table-one before doing it on table-two, you need to wait for the table-one deletion to be completed before starting the table-two deletion. I'd put that in a function:

    async function deleteItem(id) {
        await supabase
            .from("table-one")
            .delete()
            .match({id});
        await supabase
            .from("table-two")
            .delete()
            .match({id});
    }
    

    Then the code becomes:

    const promises = [];
    for (const {deactivated, id} of tableOne) {
        if (deactivated) {
            promises.push(deleteItem(id));
        }
    }
    await Promise.all(promises); // With the `try`/`catch` if desired
    

    ...or if it's okay to make two passes through the array:

    await Promise.all( // With the `try`/`catch` if desired
        tableOne.filter(({deactivated}) => deactivated)
                .map(({id}) => deleteItem(id))
    );
    

    ¹ "...by the time you have the promise, the process it's observing is already underway." That's the normal case. There is unfortunately a popular document DB library that doesn't start its work on something until/unless you call then on the promise for it. But that's an exception, and an anti-pattern.