1

There is a mistake in the following code but I can't figure it out. I believed that when I do Promise.all(...).then, all promises should be finished at that point, but it does not look like it. . The code just grabs a few jsons from an api, then adds a few html elements to the DOM. But, somehow those new elements dont register in the .then part of the Promise.all.

let urls = [];

urls.push(someurl);
urls.push(someurl2);

Promise.all(urls.map(url => {
    fetch(url)
        .then(response => {
            return response.json();
        })
        .then(data => {

            ... // do something with the current json and add some <option> elements to the DOM

            let options = document.querySelectorAll('option');
            console.log(options); //is filled with the correct options!


        })
})).then(data => {

    let options = document.querySelectorAll('option');
    console.log(options); // IS EMPTY!!! I expected to see the two sets of the <option> elements

});

The resulting HTML looks like it should on the frontend, but the DOM in .then is not in the state I expect it to be. Because of this I am having issues with the materializecss library, since I cannot fill my select dropdown with elements. It's like they have not been created yet at the point where I initialize the select dropdown in .then . Thanks for the help!

Sebastian Simon
  • 18,263
  • 7
  • 55
  • 75
Morgus Lethe
  • 340
  • 2
  • 9
  • 4
    Your `.map()` callback does not return values. – Pointy Apr 02 '20 at 14:31
  • Since you're using the function body version of an arrow function, you need an explicit `return`. Or use the concise body: `.map(url => fetch(/*...*/).then(/*...*/).then(/*...*/))`. – T.J. Crowder Apr 02 '20 at 14:35
  • I am not a good programmer, I grabbed this technique from a tutorial. What esxactly is the fix? The urls.map will return a an array of fetches, isn't that correct? I expected when all fetches end, the .then will trigger? – Morgus Lethe Apr 02 '20 at 14:36
  • Side note: Your falling prey to the footgun in the `fetch` API. You need to check for success at the HTTP level, more [on my anemic little blog](http://blog.niftysnippets.org/2018/06/common-fetch-errors.html). – T.J. Crowder Apr 02 '20 at 14:36
  • Okay, so far I understand I need an extra return somewhere, but exactly should I be returning? The Promise.all().then does run without it, so I still don't understand... – Morgus Lethe Apr 02 '20 at 14:45
  • 1
    @T.J.Crowder I did not realize you posted the fix already, because I did not see you removed the brackets from the code. So, the answer to my question is: either return the fetch, or don't use curly brackets. Thank you very much! – Morgus Lethe Apr 02 '20 at 15:30
  • Rolled back your edit which included the answer in the question post. Please read the [etiquette for answering your own question](//meta.stackexchange.com/q/17845/289905). Answers belong only in the Answer section, whereas only questions belong in the Question section. – Sebastian Simon Jan 26 '23 at 14:44

3 Answers3

2

You can try to update your map() method to return fetch response like:

Promise.all(urls.map(url => fetch(url).then(resp => resp.json())))
   .then(data => {
      console.log(data);

      let options = document.querySelectorAll('option');
      console.log(options);
   })

Please know that in ES5 we would do something like this:

var multiply = function(x, y) {
  return x * y;
};

and in ES6 we can use the arrow function like:

const multiply = (x, y) => { return x * y };

here, return statement is required otherwise it would return undefined, but the statement inside {...} will still be executed, only the calculated value will not be returned. Thus in your code fetch worked but the last .then didn't.

Also, curly brackets are not required if only one expression is present. So, the previous example could also be written as:

const multiply = (x, y) => x * y;
palaѕн
  • 72,112
  • 17
  • 116
  • 136
  • so in the Promise.all(...).then what will the data contain exactly? An array of fetched jsons? – Morgus Lethe Apr 02 '20 at 14:49
  • This is useful, thank you very much. But I also need to know which url was used to fetch which json... So I prefered handling the jsons one at a time, since I had the url from fetch still accessible. – Morgus Lethe Apr 02 '20 at 14:57
  • Great. I have also added some additional info to help you better understand arrow function and return. In your required scenario, I would say `Promise.all` is not needed then. You can simply use `map()` method like you are doing currently. – palaѕн Apr 02 '20 at 15:02
  • Now that I fixed it I understand what you were trying to say. The most concise way to answer my question is "either return the fetch, or don't use curly brackets, because they are not needed in the example". But I still need the promise.all to know when everything is finished, so I can initialize my select dropdown. – Morgus Lethe Apr 02 '20 at 15:31
1

I don't know exactly why the Promise.all(...).then executed even without the correct return, but this fixed it.

Without the return that you were missing, you were passing Promise.all() an array of undefined values. Since there were no promises in that array, Promise.all() had nothing to wait for so it called its .then() handler immediately. That's why it executed without waiting for completion. You have to pass Promise.all() an array of promises if you want it to wait.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
0

Solution: missing return in front of fetch. I don't know why the .then was running even without this return, but this fixed it. The other solution would be to remove the curly brackets in this example. Everything now works exactly as I expect it.

Morgus Lethe
  • 340
  • 2
  • 9