How to avoid the explicit Promise construction antipattern for this concurrent timer that should reset when the promise completes?

Question

I have written a pretty simply script:

{
   waiting: false,

   async handleWaiting(promise, timeout) {
       return new Promise((res, rej) => {
           let loadingStarted = false;

           const timeoutInstance = setTimeout(() => {
               loadingStarted = true;
               this.waiting = true;
           }, timeout);

           const onFinished = () => {
               if (loadingStarted) {
                   this.waiting = false;
               }
               clearTimeout(timeoutInstance);
           }

           promise
               .then((result) => {
                   onFinished();
                   res(result);
               })
               .catch((ex) => {
                   onFinished();
                   rej(ex);
               });
       });
    },

    async searchForTerm(term) {
       this.results = await this.handleWaiting(this.$wire.query(term), 500);
       // do something with the results...
    },
 }

It tiggers the waiting spinner for a search field.

Someone wrote me, that:

Your code has the Explicit Promise construction antipattern! You should use chaining and promise composition instead... Also, a function that returns a promise, but doesn't await anything don't have to be async

I tinkered around with this working code. But I got just error over error!

Can someone help me with this, or at least put me on the right track.

I am not that good with javascript but I am interested in writing it better.

Does this answer your question? [How to return an Ajax result using async/await?](https://stackoverflow.com/questions/48506445/how-to-return-an-ajax-result-using-async-await) — messerbill, Dec 12 '20 at 12:00
Just don't create the extra promise, comment it out and add "return" to the promise.then.. And yes, async is not required if you don't await. — Wiktor Zychla, Dec 12 '20 at 12:03

str · Accepted Answer · 2020-12-12T22:11:37.183

See What is the explicit promise construction antipattern and how do I avoid it? for details. Generally speaking, you (mostly) do not need new Promise when you already have a promise. You can just reuse the existing one (and chain it if necessary).

Your code can be simplied.

Remove the unnecessary new Promise
Reuse and return the existing promise
Remove code duplication and use Promise#finally instead

{
  waiting: false,

  handleWaiting(promise, timeout) {
    let loadingStarted = false;

    const timeoutInstance = setTimeout(() => {
      loadingStarted = true;
      this.waiting = true;
    }, timeout);

    return promise.finally(() => {
      if (loadingStarted) {
        this.waiting = false;
      }
      clearTimeout(timeoutInstance);
    });
  },

  async searchForTerm(term) {
    this.results = await this.handleWaiting(this.$wire.query(term), 500);
    // do something with the results...
  },
}

And you can probably get rid of loadingStarted as well. Is there a reason why you have two state variables for that? You never reset loadingStarted anyway.

{
  waiting: false,

  handleWaiting(promise, timeout) {
    const timeoutInstance = setTimeout(() => {
      this.waiting = true;
    }, timeout);
    return promise.finally(() => {
      this.waiting = false;
      clearTimeout(timeoutInstance);
    });
  },

  async searchForTerm(term) {
    this.results = await this.handleWaiting(this.$wire.query(term), 500);
    // do something with the results...
  },
}

Alternative to `.finally()`: `try { return await promise; } finally { … }` — Bergi, Dec 12 '20 at 14:24
Notice I'm not a big fan of passing promises as arguments to functions either, what might fit better would be passing a callback to return the promise. (Following the [disposer pattern](https://stackoverflow.com/questions/28915677/what-is-the-promise-disposer-pattern) but basically without an explicit resource). — Bergi, Dec 12 '20 at 14:27
"*you can probably get rid of loadingStarted as well. Is there a reason why you have two state variables for that?*" - a reason for that might be multiple concurrent calls to `handleWaiting`. It might be supposed not to happen at all, but still needs to be handled properly if it could occur. Admittedly, the second boolean doesn't actually help at all, a counter (semaphore) would be my suggestion. — Bergi, Dec 12 '20 at 14:30
Ok, It's hilarious what I have to learn in order to write good javascript. What would it look like. When you write the short code above of str with the disposer pattern? — Slowwie, Dec 12 '20 at 17:58

How to avoid the explicit Promise construction antipattern for this concurrent timer that should reset when the promise completes?

1 Answers1

Linked