-1

I'm fetching images from my app through js but eslint is warning about using async promise executors. I know that in other cases like this, you could just remove the promise all-together, but not sure if that's applicable here.

Here is the code in question:

async fn() {
    let urls = _.map($('#img-container img'), (img) => $(img).attr('src'));

    urls = await Promise.all(urls.map((url) => new Promise(async (res) => { // FIXME - async promise executor
        const blob = await (await fetch(url)).blob();
        const reader = new FileReader();
        reader.readAsDataURL(blob);
        reader.onload = res;
    })));

    urls = urls.map((u) => u.target.result);
    return Buffer.from(pako.deflate(JSON.stringify(urls))).toString('base64');
}

How could I restructure this to get rid of this pattern? Or should I just dismiss the warning?

Axekan
  • 641
  • 5
  • 11
  • @T.J.Crowder Incorrect duplicate. This is about the promise executor being async. – AKX Sep 18 '22 at 09:46
  • See the [linked question's answers](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it). You **already** have a promise to work with (from `fetch`), so there's no need to create one. Separately: It never makes sense to pass an `async` function into `new Promise`. `new Promise` doesn't do anything with the promise the `async` function returns. Just call the `async` function directly, since (again) it return a promise. (Or in this case, let `map` call it.) – T.J. Crowder Sep 18 '22 at 09:47
  • @AKX - Fixing the explicit creation error fixes the problem either way, but do you have a good dupetarget for the `async` function thing? You **know** there is one. :-) – T.J. Crowder Sep 18 '22 at 09:47
  • @​Axekan - I suspect you're referring to [this error](https://eslint.org/docs/rules/no-async-promise-executor), which the docs do explain. – T.J. Crowder Sep 18 '22 at 09:49

1 Answers1

-1

ESlint is complaining because there shouldn't be a need for an explicit Promise's executor to be async, because it generally means you're doing something wrong (mixing things that are already promisesque with building explicit promises).

You'll have a better time altogether (and ESlint won't complain either) if you refactor things so the callback-style FileReader API is wrapped in its own function that returns a promise (and that executor doesn't need to be async).

function blobToDataURL(blob) {
  return new Promise((res) => {
    const reader = new FileReader();
    reader.readAsDataURL(blob);
    reader.onload = res;
  });
}

async function x() {
  let urls = _.map($("#img-container img"), (img) => $(img).attr("src"));

  const dataUrls = await Promise.all(
    urls.map(async (url) => {
      const blob = await fetch(url).blob();
      return blobToDataURL(blob);
    }),
  );
}
AKX
  • 152,115
  • 15
  • 115
  • 172
  • 1
    [It's a duplicate](https://stackoverflow.com/questions/43036229/is-it-an-anti-pattern-to-use-async-await-inside-of-a-new-promise-constructor). – T.J. Crowder Sep 18 '22 at 09:51