Advice on Javascript Promises script

I’m reading about Promises in Javascript The Definitive Guide by Flannagan 7ed. In the book there is a script which shows how to build a Promise chain dynamically for an arbitray number of URLs. The script is as follows:

       function fetchSequentially(urls) {
        // We'll store the URL bodies here as we fetch them
        const bodies = [];
    
        // Here's a Promise-returning function that fetches one body
        function fetchOne(url) {
            return fetch(url)
                .then(response => response.text())
                .then(body => {
                    // We save the body to the array, and we're purposely
                    // omitting a return value here (returning undefined)
                    bodies.push(body);
                });
        }
    
        // Start with a Promise that will fulfill right away (with value undefined)
        let p = Promise.resolve(undefined);
    
        // Now loop through the desired URLs, building a Promise chain
        // of arbitrary length, fetching one URL at each stage of the chain
       for(url of urls) {
           p = p.then(() => fetchOne(url));
     
       }
     
     
        // When the last Promise in that chain is fulfilled, then the
        // bodies array is ready. So let's return a Promise for that
        // bodies array. Note that we don't include any error handlers:
        // we want to allow errors to propagate to the caller.
        return p.then(() => bodies);
    }


//The script was run as below
//I added the line below to declare the urls array

let urls = ['/data.txt','/readme.txt','/textfile.txt']; 

//the line below is from the book
fetchSequentially(urls).then( bodies => {console.log(bodies)}).catch( e => console.error(e));

I added the let urls line to run the script to fetch 3 text files on my PC.

When the script runs it seems to only fetch the last file ‘textfile.txt’, and it prints out the contents of the third file 3 times in the console. I thought the script would retrieve the contents of all 3 files, add them to the bodies array, and then console log the contents of all 3 files.

Can anyone spot why this isn’t working?

Thanks
John

>Solution :

It looks like this is the section that’s causing problems:

   for(url of urls) {
       p = p.then(() => fetchOne(url));
   }

Here you’re creating a global variable url, and since it’s running asynchronously, fetchOne(url) is using the last instance of it.

Instead you can do something like

   for(let url of urls) {
       p = p.then(() => fetchOne(url));
   }

Which creates a local instance of it.

This sort of style of programming of iterating through arrays asynchronously can introduce subtle errors like this one, so I’d recommend a style that unambiguously creates a new instance. Something like:

urls.forEach(function (url) {
   p = p.then(() => fetchOne(url));
});

Though for this sort of thing with multiple promises, you might just want to do a .map with Promise.all:

return Promise.all(urls.map(fetchOne)); // instead of promise chaining with p