5

Block 1:

const promise = new Promise((resolve) => {
  setTimeout(resolve, 100);
});

Block 2:

const promise = (async () => {
  await new Promise(resolve => {
    setTimeout(resolve, 100);
  });
})();

Are the above two blocks equivalent? Any note-worthy differences?

I know it's a highly contrived example and there's not much purpose for Block 2 here. But a situation I have is where I want to create and store a reference to a promise, but the promise executor function needs to use await to get some data. But declaring new Promise(async (resolve) => {}); is considered an anti-pattern. Is block 2 in this situation any better?

UPDATE: Providing a more concrete example of what I'm trying to do:

  export async function getData(request) {
    // De-dupe fetches for the same request and return existing promise.
    if (map.has(JSON.stringify(request))) {
      return map.get(JSON.stringify(request));
    }

    const thePromise = (async () => {
      const [foo, bar] = Promise.all(await getFoo(), await getBar());

      const theData = await getTheData(foo, bar);

      return theData.some.thing ? 'a' : 'b';
    })();

    map.put(JSON.stringify(request), thePromise);
    return thePromise;
  }
kufudo
  • 91
  • 1
  • 4
  • No there is a minimal overhead with async keyword but probably nothing to worry about if you are using [babel](https://babeljs.io/) you can see what it will produce depending on the es versions. I am almost certain that es6 browsers do something similar but to be honest not really sure. – Filip Cordas Jul 27 '19 at 12:50
  • "*the promise executor function needs to use await to get some data*" - no, it doesn't. You're right to identify [passing an `async function` as the executor to `new Promise`](https://stackoverflow.com/q/43036229/1048572) as an anitpattern. Can you please show us the actual code of a concrete case? – Bergi Jul 27 '19 at 13:41
  • My executor needs to make two Async calls to get some data before making another Async call and resolving based on the result of that third call. I could use promise chaining instead of Async/await but I don't see why I should have to mix two patterns. – kufudo Jul 27 '19 at 21:14
  • Added a more concrete example. – kufudo Jul 29 '19 at 00:56
  • @kufudo Thanks. That example is fine (apart from the `Promise.all(await getFoo(), await getBar());`), and it doesn't use the `new Promise` constructor with an executor anywhere. – Bergi Jul 29 '19 at 10:17

3 Answers3

1

In the second approach you can use try and catch blocks, like so:

const promise = (async () => {
  try {
    await new Promise(resolve => {
      setTimeout(resolve, 100);
    });
  } catch(err) {
    // handle error here
  }
})();
biphobe
  • 4,525
  • 3
  • 35
  • 46
cccn
  • 929
  • 1
  • 8
  • 20
0

Not sure if this is what you're referring to when you say

but the promise executor function needs to use await to get some data

but it sounds like you're dealing with some asynchronous call inside of a promise that you need to "wait" for, then resolve with the data that was returned as a result.

So for example,

EDIT: As pointed by @Bergi in the comments below, you probably shouldn't be doing this (wrapping a promise inside of another promise):

// Assuming you're making an API with `axios` to grab some data
const promise = new Promise((resolve, reject) => {
  axios.get('www.example.com/api', {...})
    .then(result => {
      resolve(result)
    })
    .catch(error => {
      reject(error)
    })
})
const callback = (data) => {
  console.log('data', data)
}

// `callback` will get called when the `axios.get` "finishes" (resolves) 
// with some data after it's done fetching, no need to `await` here

promise.then(callback)

Instead, you could do promise chaining, if needed:

// using the same example from above
const handleResponse = (result) => {
  // handle response
}
const handleError = (error) => {
  // handle error
}

axios.get('www.example.com/api', {...})
  .then(handleResponse)
  .then(doSomethingElse)
  .catch(handleError)

// or, if you need to make multiple asynchronous calls use `Promise.all`
const handleResponses = (response) => {
  const [users, books] = response
  // do the rest
}

Promise.all([
  axios.get('www.example.com/api/users'),
  axios.get('www.example.com/api/books')
])
  .then(handleAPIResponses)
  .then(doSomethingElse)
  .catch(handleError)

Similarly, if you're dealing with the "error first" callback pattern

// `fs.readFile` with Node.js
const promise = new Promise((resolve, reject) => {
  fs.readFile('...', (err, data) => {
    if (err) {
      reject(err)
      return
    }
    resolve(data)
  })
})
const callback = (data) => {
  console.log('data', data)
}

// again, `callback` will get called when reading a file is done
// and you get the result back

promise.then(callback)

If you need to make multiple calls inside of a promise and then resolve with the final value, you could do something like the following:

async function promise() {
  try {
    // assuming API.getUserIds() returns a promise
    const userIds = await API.getUserIds(...)
    const users = await API.getUsers(userIds)

    return users
  } catch (err) {
    // ...
  }
}
const callback = (data) => {
  console.log('data', data)
}

// again, `callback` will get called when both API requests 
// resolve with some values and when the above function returns `users`

promise().then(callback)

Personally, I'd stay away from #2, and whatever you're trying to accomplish with that pattern (depending on your case) can be easily done choosing one of the examples above.

goto
  • 4,336
  • 15
  • 20
  • the first example you gave is wrong, in this [example](https://gist.github.com/prithweedas/1e9dd530aeaf411581d8e4296d1571d4) if you run this you will see that you get a promise passed in your call back, your promise executor function won't wait for the `axios.get()` to resolve it will just return that pending Promise and that'll be what gets passed to your then callback – Prithwee Das Jul 27 '19 at 13:03
  • You're right, I fixed it, thanks. Also, your example (as well as my old one) doesn't actually return anything, so the `callback` would never get called, since I never `resolve` nor `reject` (in the original promise). So in your example, `promise.then(callback) // Promise {}` is not a pending promise. – goto Jul 27 '19 at 13:11
  • @goto1 Did you intentionally use the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it) to demonstrate a bad practice? I can't tell. – Bergi Jul 27 '19 at 13:46
  • @Bergi OP was asking for a way to wait for an asynchronous call to finish before resolving the outer promise, so I was just showing different ways that it can be done. You're right though, in the first example it wouldn't make sense to wrap a promise into another promise. I provided a better example that doesn't do that. Thanks. – goto Jul 28 '19 at 22:06
0

Providing a more concrete example of what I'm trying to do

Your usage of the async immediately executed function expression is totally fine here, there's nothing wrong with it. Sure, you could have written the same with a .then() chain, but that wouldn't be exactly as comfortable.

Some minor improvements:

export function getData(request) {
//    ^ no need for `async` when you return a promise anyway
  const requestKey = JSON.stringify(request); // compute only once
  if (map.has(requestKey)) {
    return map.get(requestKey);
  }
  const thePromise = (async () => {
    const [foo, bar] = await Promise.all([getFoo(), getBar()]);
//                     ^^^^^ had `await` keywords in the wrong places
    const theData = await getTheData(foo, bar);
    return theData.some.thing ? 'a' : 'b';
  })();
  map.set(requestKey, thePromise);
  return thePromise;
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • `Promise.all` takes an iterable as an argument, so both `getFoo` and `getBar` should be put into an array, like so: `Promise.all([getFoo(), getBar()])`... I am still confused as to why the `async` call is wrapped with an `iffy` though. Is OP expecting the promise to resolve before it gets stored in map object? It won't work, if so – goto Jul 29 '19 at 12:56
  • @goto1 No, the OP is expecting to get a promise to store in the map (immediately, before resolving) and use `async`/`await` syntax. There's no way to do that without the IIFE. – Bergi Jul 29 '19 at 12:58
  • You could just do `const thePromise = async () => {...}`, then `map.put('...', thePromise)` and when you need to actually use that promise you could just do `const myPromise = getData('...')` then `myPromise().then(...)`... No need for an iffy there, which in my opinion makes it a lot more confusing and hard to understand what's going on, like in this example - https://jsbin.com/gelopibago/edit?js,console – goto Jul 29 '19 at 13:17
  • @goto1 But then you're storing a *function* (confusingly called `thePromise`) in that `Map`, not the actual promise with its value for caching? That misses the entire point of the map. – Bergi Jul 29 '19 at 14:43
  • The naming could change, but using an iffy there is definitely making it less readable... Not sure what's the point of storing a promise instead of the actual value that the promise resolves to... If you're not careful enough, instead of "caching" requests, you could end up making requests every time you retrieve something from the map, which defeats the purpose – goto Jul 29 '19 at 15:33
  • @goto1 The iife makes it work at all. You store the promise so that you are not doing another request for the same resource while the first request is still running. Storing the result value after the request has finished won't achieve that. And yes, making requests every time is just what your code that doesn't use the iife did end up with. – Bergi Jul 29 '19 at 15:37
  • @goto1 Maybe you should refactor the iife into a separate function `makeRequest`, and then call `const thePromise = makeRequest(request)` in `getData`, if that's less confusing. – Bergi Jul 29 '19 at 15:38
  • Yeah, I guess seeing an iffy there makes it less readable to me and I had to try it out myself to make sure I understand what exactly is going on. Not a big fan of iffys I guess lol. Also, there's no `put` method on the map object, `set` is the right method here. – goto Jul 29 '19 at 17:21
  • @goto1 Ah, thanks, I just copied it over from the OPs code and didn't notice. `put` is used in Java... – Bergi Jul 29 '19 at 18:31