I have a simple application. I have an upload which reads multiple images into a hook. From this I would then like to reduce and display each individual image. However, React only ever shows me one image
useEffect(() => {
const CallMe = async () => {
// files contains 2 images
if (files.length > 0) {
return Promise.all(
files.map(async (file, idx) => {
return new Promise(async (resolve, reject) => {
try {
const image = await resizeFile(file[idx]);
console.log(image);
resolve(image);
} catch (error) {
reject(error);
}
});
})
);
}
};
CallMe().then((res) => {
setThumbnails([...thumbnails, res]);
});
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [files]);
{thumbnails.length &&
thumbnails?.map((img, idx) => {
if (img)
return (
//ONLY ONCE IS SHOWN
<img
key={idx}
src={img}
alt="soso"
height={100}
width={100}
></img>
);
})}
CodePudding user response:
The primary problem is here:
setThumbnails([...thumbnails, res]);
Two issues there:
That uses the initial value of
thumbnails
(I'm guessing[]
), not the current one when the async work is complete.res
is an array, not an individual image.Instead, use the callback form so you're working with up-to-date information, and spread out
res
:setThumbnails((previousThumbnails) => [...previousThumbnails, ...res]);
That will also fix the ESLint error you've suppressed. (That error is helpful, I suggest not ignoring it.)
Here are some other issues that you might want to look at:
Your
map
look suspect. You havefiles.map(async (file, idx) => {
...but then you're doing
const image = await resizeFile(file[idx]);
...but
file
is already one element fromfiles
, so unlessfiles
is an array of arrays, you just needfile
notfile[idx]
there. (Even if it is an array of arrays, it seems odd to use the value at index0
of the first one, then the value at index1
of the second one, etc.)You're using
new Promise
within anasync
function. That's the explicit promise creation anti-pattern, there's no reason to do that here.Your current code ignores errors resizing the file other than dumping them out to the console. I've assumed you really want to do that, but once you're using
async
/await
, I'd usetry
/catch
to handle that.
So taking that all together:
useEffect(() => {
const CallMe = async () => {
return Promise.all(files.map(async (file) => {
try {
return await resizeFile(file);
} catch (error) {
// Not really ideal to just ignore errors
// Note this will leave an `undefined` in the returned array
console.log(error);
}
}));
};
CallMe()
.then((res) => {
// Remove blank elements from the array
res = res.filter(Boolean);
// If we have any left, use them
if (res.length) {
setThumbnails((thumbnails) => [...thumbnails, ...res]);
}
})
.catch((error) => {
// ...handle/report error...
});
}, [files]);
A couple of notes on this:
<img
key={idx}
src={img}
alt="soso"
height={100}
width={100}
></img>
Since we know your array will change (if files
changes, which it can if it's from props), using the index as the key will not work correctly; see this from the react docs:
We don’t recommend using indexes for keys if the order of items may change. This can negatively impact performance and may cause issues with component state. Check out Robin Pokorny’s article for an in-depth explanation on the negative impacts of using an index as a key. If you choose not to assign an explicit key to list items then React will default to using indexes as keys.
They don't say it strongly enough there in my view: you will get duplicated display elements or elements that don't get removed from the display, etc. Indexes as keys is only a valid choice for static lists.
Finally, <img ...></img>
is more idiomatically written <img .../>
in JSX. (And you would never write it in HTML at all; img
is a void element.)