The use case for this is a list of files and each one has a button to remove it.
Then we can click in many files.
Let's say we have 2 files, if we click in both at the end we will have 1 file instead of 0 files in the list.
I have no access to open a PR, then I'll paste it here:
commit 2cc0c8791faf0ef8c33e8cef1adc8090d398d7b0 (HEAD -> fix/remove-multiple-files-concurrently)
Author: Osman Pontes <osmanpontes@gmail.com>
Date: Wed Dec 14 15:11:43 2022 -0300
fix multiple removeFile calls
The issue was caused by outdated "files" variable. This is due to how
closure works. Each removeFile starts with 2 files in "files". Then when
we remove the file, one file remains. Then when the second removeFile
file is called, one file remains. The solution is using functional
updates, which makes sure we are calculating state over the previous one
instead of some outdated value due to closure.
Reference:
1. https://reactjs.org/docs/hooks-reference.html#functional-updates
---
src/lib/useFileUpload.ts | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lib/useFileUpload.ts b/src/lib/useFileUpload.ts
index 820cd3c..d0d48d2 100644
--- a/src/lib/useFileUpload.ts
+++ b/src/lib/useFileUpload.ts
@@ -88,9 +88,9 @@ export const useFileUpload = (): useFileUploadHook => {
}
if (typeof file === 'string') {
- setFilesState(files.filter((_file: File) => _file.name !== file));
+ setFilesState((previousFiles) => previousFiles.filter((_file: File) => _file.name !== file));
} else {
- setFilesState(files.filter((_file: File, i) => i !== file));
+ setFilesState((previousFiles) => previousFiles.filter((_file: File, i) => i !== file));
}
},
[files],
commit 763bbfd3697c2ce199c0b8642a0b3f236c4c7bc1
Author: Osman Pontes <osmanpontes@gmail.com>
Date: Wed Dec 14 15:07:20 2022 -0300
added failing tests for multiple removeFile
I added failing tests to prove it's failing before I submit the fix.
When calling removeFile many times some files are not removed. The use
case is add 2 files and have a button to remove each. These buttons call
removeFile. At the end of the operations we have 1 file listed, instead
of zero.
---
src/__tests__/useFileUpload.test.ts | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/src/__tests__/useFileUpload.test.ts b/src/__tests__/useFileUpload.test.ts
index da68d3d..0c14a51 100644
--- a/src/__tests__/useFileUpload.test.ts
+++ b/src/__tests__/useFileUpload.test.ts
@@ -8,6 +8,10 @@ const file = new File(['foo'], 'foo.txt', {
type: 'text/plain',
});
+const barFile = new File(['bar'], 'bar.txt', {
+ type: 'text/plain',
+});
+
const defineProperty = (obj, key, value) => {
Object.defineProperty(obj, key, { writable: false, value });
};
@@ -74,6 +78,54 @@ describe('useFileUpload', () => {
expect(result.current.files).toHaveLength(0);
});
+ /**
+ * Remove multiple files by name concurrently.
+ */
+ it('removes multiple files by name concurrently', () => {
+ const { result } = renderHook(() => useFileUpload());
+ const event = new Event('');
+
+ defineProperty(event, 'currentTarget', { files: [file, barFile] });
+
+ act(() => {
+ result.current.setFiles(event, 'a');
+ });
+
+ expect(result.current.files).toHaveLength(2);
+
+ act(() => {
+ ['foo.txt', 'bar.txt'].forEach((fileName) => {
+ result.current.removeFile(fileName);
+ });
+ });
+
+ expect(result.current.files).toHaveLength(0);
+ });
+
+ /**
+ * Remove multiple files by index concurrently.
+ */
+ it('removes multiple files by index concurrently', () => {
+ const { result } = renderHook(() => useFileUpload());
+ const event = new Event('');
+
+ defineProperty(event, 'currentTarget', { files: [file, barFile] });
+
+ act(() => {
+ result.current.setFiles(event, 'a');
+ });
+
+ expect(result.current.files).toHaveLength(2);
+
+ act(() => {
+ [1, 0].forEach((fileIndex) => {
+ result.current.removeFile(fileIndex);
+ });
+ });
+
+ expect(result.current.files).toHaveLength(0);
+ });
+
/**
* Remove all files.
*/
The use case for this is a list of files and each one has a button to remove it. Then we can click in many files. Let's say we have 2 files, if we click in both at the end we will have 1 file instead of 0 files in the list. I have no access to open a PR, then I'll paste it here: