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 arbitrary 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 log the contents of all 3 files to console.
Can anyone spot why this isn't working?
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));
}
This creates a local instance of url for each iteration.
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 per iteration. 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