JQuery async .each returns empty promises

Question

Problem

Hi friends,

I am having the problem that the object promises is returning me as empty []. While the console.log I have documented shows me the data.

If there is a way to solve the problem, I will appreciate it.

const getALLMovies = async(page: number = 1): Promise<IMovies[]> =>{
  const res = await axios.get(`${BASE_URL}peliculas/${page}`);
  const body = await res.data;
  const $ = load(body);
  const promises: IMovies[] = [];  

$('body div div div div div div div div div div div div#default-tab-1 div.Posters a').each(async(index , element) =>{
    const $element = $(element);
    const id = $element.attr('href').replace(BASE_URL , '').trim();
    const title = $element.find('div.listing-content p').text().trim();
    const poster = $element.find('img').attr('src');
    const extra = await contentHandler(id);
    promises.push({
      //id: id || null,
      title: title || null,
      poster: poster || null,
      year:   extra[0].year || null,
      genres: extra[0].genres || null,
      rating: extra[0].rating || null,
      synopsis: extra[0].synopsis || null,
      authors:  extra[0].authors || null,
      director: extra[0].director || null,
      writers:  extra[0].writers || null,
      country:  extra[0].country || null,
      releaseDate: extra[0].releaseDate || null,
    })
    //console.log(JSON.stringify(promises , null , 2)) --> The data is shown here.

  });

  return Promise.all(promises);
};

getALLMovies()
  .then(res =>{
    console.log(res.map(x => x)) --> empty object
  })

Please consider providing a [mcve] as described in [ask]; what you're describing doesn't sound possible given the code above, but of course I can't test it myself so ‍♂️. Good luck! — jcalz, Feb 19 '20 at 16:48
I have update my answer to include an example that achieves your desired result. — Zze, Feb 19 '20 at 20:32

score 2 · Answer 1 · edited Feb 20 '20 at 01:38

2

The issue you are having is a race condition. Your .each handler schedules some asynchronous code, namely it has to await the results of contentHandler. As such, when getALLMovies is called promises is still an empty array, and that array won't be fully populated until all calls to contentHandler complete. I.e. for each element, the result of contentHandler is queued up (4 times in the example below).

var promises = [];

function timeout(ms) {
  return new Promise(resolve => setTimeout(resolve, ms));
}

async function go() {
  $("div").each(async() => {
    await timeout(1500);
    console.log("now");
    promises.push("example");
  })
  console.log(promises);
}

go();

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

This is how I would alter your code to achieve your desired result:

var promises = [];

function timeout(ms) {
  return new Promise(resolve => setTimeout(resolve, ms));
}

async function go() {
  console.log("go");

  promises = $("div").map(async () => {
    await timeout(1500);
    console.log ("now");
    return "value"; 
  });

  return Promise.all(promises);
}

go().then(() => console.log(promises.length));

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

Also a side note... body div div div div div div div div div div div div in your selector is likely irrelevant due to the inclusion of #default-tab-1 which is a higher order or specificity.

edited Feb 20 '20 at 01:38

benbotto

2,291
1
20
32

answered Feb 19 '20 at 19:09

Zze

18,229
13
85
118

Sorry to be pedantic, @Zze, but `async` doesn't move the execution to a new thread. The handler is added to the message queue: https://developer.mozilla.org/en-US/docs/Web/JavaScript/EventLoop – benbotto Feb 19 '20 at 21:25
It's also using the Explicit Promise Construction Anti-Pattern: https://stackoverflow.com/a/23803744/3977276 Create that "new Promise" is not necessary. – benbotto Feb 19 '20 at 21:36
@avejidah Yes, I will update my answer in regards to your first comment. The second comment is not applicable to this example as `go()` is not awaiting the result of internal promises firing, and is in fact in line with the suggested / accepted answer. Please feel free to copy this answer to your own and post if you feel I am incorrect. You would be correct if all it was doing was returning `timeout`, but the OP will be doing other logic in this section. – Zze Feb 19 '20 at 21:50
1

I think the second is applicable. `promises = $("div").map(async () => { await timeout(1500); return "value"; })` avoids the explicit promise construction and results in the same resolution. – benbotto Feb 19 '20 at 21:55
@avejidah ahhhh I see what you are saying, ok. Updated - thanks. – Zze Feb 19 '20 at 21:59
1

Hah! Well that's close, but not quite. Now `resolve` won't be defined. Just return "value" and it will be wrapped in a Promise instance automatically. (E.g. `[].map(async () => {return "value"})`) – benbotto Feb 19 '20 at 22:00
The reason I pointed this out is that by using the aforementioned anti-pattern, failures in the inner asynchronous method (`timeout` in your example) are not propagated. E.g. the code that calls `go()` cannot `.catch` errors that happen in `timeout.` – benbotto Feb 19 '20 at 22:04
@avejidah Thanks, this has been interesting, I did not know that it implicitly wraps it. Also that is interesting with the error propagation. – Zze Feb 19 '20 at 22:05

score 0 · Answer 2 · answered Feb 19 '20 at 21:11

Thanks @Zze,

With your example I have managed to show all the data.

SOLUTION

const getALLMovies = async(page: number = 1) =>{
  const res = await axios.get(`${BASE_URL}peliculas/${page}`);
  const body = await res.data;
  const $ = load(body);
  const promises: IMovies[] = [];  

  //const actual = page;
  //const lastPage = parseInt($('body div.app div.layout ul.pagination li').eq(4).find('a').attr('href').match(/\d/g).join("") , 10);
  //const pagination = {
  //  actual: actual,
  //  last: lastPage
  //}

  const promise = $('body div#default-tab-1 div.Posters a').map((index , element) => new Promise(async(resolve) =>{
    const $element = $(element);
    const id = $element.attr('href').replace(BASE_URL, '').trim();
    const title = $element.find('div.listing-content p').text().trim();
    const poster = $element.find('img').attr('src');
    const extra = await contentHandler(id);
    promises.push({
      //id: id || null,
      title: title || null,
      poster: poster || null,
      year: extra[0].year || null,
      genres: extra[0].genres || null,
      rating: extra[0].rating || null,
      synopsis: extra[0].synopsis || null,
      authors: extra[0].authors || null,
      director: extra[0].director || null,
      writers: extra[0].writers || null,
      country: extra[0].country || null,
      releaseDate: extra[0].releaseDate || null,
    });
    resolve(promises)
  }));


  return Promise.all(promise.toArray());
};

getALLMovies()
  .then(res =>{
    console.log(JSON.stringify(res , null , 2))
  })

That's using the Explicit Promise Construction Anti-Pattern: https://stackoverflow.com/a/23803744/3977276 Creating that "new Promise" is not necessary. — benbotto, Feb 19 '20 at 21:36

JQuery async .each returns empty promises

2 Answers2