fix: Controlled multiple files upload list sync (#26612)

* init hooks

* Use raf to sync list

* clean up

components/upload/Upload.tsx

@@ -19,6 +19,7 @@ import defaultLocale from '../locale/default';
 import { ConfigContext } from '../config-provider';
 import devWarning from '../_util/devWarning';
 import useForceUpdate from '../_util/hooks/useForceUpdate';
+import useFreshState from './useFreshState';
 
 export { UploadProps };
 
@@ -47,11 +48,11 @@ const InternalUpload: React.ForwardRefRenderFunction<unknown, UploadProps> = (pr
   const [dragState, setDragState] = React.useState<string>('drop');
   const forceUpdate = useForceUpdate();
 
-  // `fileListRef` used for internal state sync to avoid control mode set it back when sync update.
-  // `visibleFileList` used for display in UploadList instead.
-  // It's a workaround and not the best solution.
-  const fileListRef = React.useRef(fileListProp || defaultFileList || []);
-  const visibleFileList = fileListProp || fileListRef.current;
+  // Refresh always use fresh data
+  const [getFileList, setFileList] = useFreshState<UploadFile<any>[]>(
+    fileListProp || defaultFileList || [],
+    fileListProp,
+  );
 
   const upload = React.useRef<any>();
 
@@ -63,16 +64,8 @@ const InternalUpload: React.ForwardRefRenderFunction<unknown, UploadProps> = (pr
     );
   }, []);
 
-  React.useEffect(() => {
-    if (fileListProp !== undefined && fileListProp !== fileListRef.current) {
-      fileListRef.current = fileListProp;
-      forceUpdate();
-    }
-  }, [fileListProp]);
-
   const onChange = (info: UploadChangeParam) => {
-    fileListRef.current = info.fileList;
-    forceUpdate();
+    setFileList(info.fileList);
 
     const { onChange: onChangeProp } = props;
     if (onChangeProp) {
@@ -87,7 +80,7 @@ const InternalUpload: React.ForwardRefRenderFunction<unknown, UploadProps> = (pr
     const targetItem = fileToObject(file);
     targetItem.status = 'uploading';
 
-    const nextFileList = fileListRef.current.concat();
+    const nextFileList = getFileList().concat();
 
     const fileIndex = nextFileList.findIndex(({ uid }: UploadFile) => uid === targetItem.uid);
     if (fileIndex === -1) {
@@ -110,7 +103,7 @@ const InternalUpload: React.ForwardRefRenderFunction<unknown, UploadProps> = (pr
     } catch (e) {
       /* do nothing */
     }
-    const targetItem = getFileItem(file, fileListRef.current);
+    const targetItem = getFileItem(file, getFileList());
     // removed
     if (!targetItem) {
       return;
@@ -120,12 +113,12 @@ const InternalUpload: React.ForwardRefRenderFunction<unknown, UploadProps> = (pr
     targetItem.xhr = xhr;
     onChange({
       file: { ...targetItem },
-      fileList: fileListRef.current.concat(),
+      fileList: getFileList().concat(),
     });
   };
 
   const onProgress = (e: { percent: number }, file: UploadFile) => {
-    const targetItem = getFileItem(file, fileListRef.current);
+    const targetItem = getFileItem(file, getFileList());
     // removed
     if (!targetItem) {
       return;
@@ -134,12 +127,12 @@ const InternalUpload: React.ForwardRefRenderFunction<unknown, UploadProps> = (pr
     onChange({
       event: e,
       file: { ...targetItem },
-      fileList: fileListRef.current.concat(),
+      fileList: getFileList().concat(),
     });
   };
 
   const onError = (error: Error, response: any, file: UploadFile) => {
-    const targetItem = getFileItem(file, fileListRef.current);
+    const targetItem = getFileItem(file, getFileList());
     // removed
     if (!targetItem) {
       return;
@@ -149,7 +142,7 @@ const InternalUpload: React.ForwardRefRenderFunction<unknown, UploadProps> = (pr
     targetItem.status = 'error';
     onChange({
       file: { ...targetItem },
-      fileList: fileListRef.current.concat(),
+      fileList: getFileList().concat(),
     });
   };
 
@@ -160,7 +153,7 @@ const InternalUpload: React.ForwardRefRenderFunction<unknown, UploadProps> = (pr
         return;
       }
 
-      const removedFileList = removeFileItem(file, fileListRef.current);
+      const removedFileList = removeFileItem(file, getFileList());
 
       if (removedFileList) {
         file.status = 'removed';
@@ -189,11 +182,13 @@ const InternalUpload: React.ForwardRefRenderFunction<unknown, UploadProps> = (pr
     if (result === false) {
       // Get unique file list
       const uniqueList: UploadFile<any>[] = [];
-      fileListRef.current.concat(fileListArgs.map(fileToObject)).forEach(f => {
-        if (uniqueList.every(uf => uf.uid !== f.uid)) {
-          uniqueList.push(f);
-        }
-      });
+      getFileList()
+        .concat(fileListArgs.map(fileToObject))
+        .forEach(f => {
+          if (uniqueList.every(uf => uf.uid !== f.uid)) {
+            uniqueList.push(f);
+          }
+        });
 
       onChange({
         file,
@@ -212,7 +207,7 @@ const InternalUpload: React.ForwardRefRenderFunction<unknown, UploadProps> = (pr
     onSuccess,
     onProgress,
     onError,
-    fileList: fileListRef.current,
+    fileList: getFileList(),
     upload: upload.current,
     forceUpdate,
   }));
@@ -251,7 +246,7 @@ const InternalUpload: React.ForwardRefRenderFunction<unknown, UploadProps> = (pr
           return (
             <UploadList
               listType={listType}
-              items={visibleFileList}
+              items={getFileList(true)}
               previewFile={previewFile}
               onPreview={onPreview}
               onDownload={onDownload}
@@ -279,9 +274,7 @@ const InternalUpload: React.ForwardRefRenderFunction<unknown, UploadProps> = (pr
       prefixCls,
       {
         [`${prefixCls}-drag`]: true,
-        [`${prefixCls}-drag-uploading`]: fileListRef.current.some(
-          file => file.status === 'uploading',
-        ),
+        [`${prefixCls}-drag-uploading`]: getFileList().some(file => file.status === 'uploading'),
         [`${prefixCls}-drag-hover`]: dragState === 'dragover',
         [`${prefixCls}-disabled`]: disabled,
         [`${prefixCls}-rtl`]: direction === 'rtl',

components/upload/__tests__/upload.test.js

@@ -17,6 +17,15 @@ describe('Upload', () => {
   beforeEach(() => setup());
   afterEach(() => teardown());
 
+  // Mock for rc-util raf
+  window.requestAnimationFrame = (callback) => {
+    window.setTimeout(callback, 16);
+  };
+  window.cancelAnimationFrame = (id) => {
+    window.clearTimeout(id);
+  };
+
+
   // https://github.com/react-component/upload/issues/36
   it('should get refs inside Upload in componentDidMount', () => {
     let ref;
@@ -255,6 +264,7 @@ describe('Upload', () => {
   });
 
   it('should be controlled by fileList', () => {
+    jest.useFakeTimers();
     const fileList = [
       {
         uid: '-1',
@@ -266,8 +276,11 @@ describe('Upload', () => {
     const ref = React.createRef();
     const wrapper = mount(<Upload ref={ref} />);
     expect(ref.current.fileList).toEqual([]);
+
     wrapper.setProps({ fileList });
+    jest.runAllTimers();
     expect(ref.current.fileList).toEqual(fileList);
+    jest.useRealTimers();
   });
 
   describe('util', () => {

components/upload/__tests__/uploadlist.test.js

@@ -26,6 +26,14 @@ const fileList = [
 ];
 
 describe('Upload List', () => {
+  // Mock for rc-util raf
+  window.requestAnimationFrame = callback => {
+    window.setTimeout(callback, 16);
+  };
+  window.cancelAnimationFrame = id => {
+    window.clearTimeout(id);
+  };
+
   // jsdom not support `createObjectURL` yet. Let's handle this.
   const originCreateObjectURL = window.URL.createObjectURL;
   window.URL.createObjectURL = jest.fn(() => '');
@@ -913,4 +921,51 @@ describe('Upload List', () => {
     wrapper.setProps({ showUploadList: false });
     expect(wrapper.exists('.ant-upload-list button.trigger')).toBe(false);
   });
+
+  // https://github.com/ant-design/ant-design/issues/26536
+  it('multiple file upload should keep the internal fileList async', async () => {
+    jest.useFakeTimers();
+
+    const uploadRef = React.createRef();
+
+    const MyUpload = () => {
+      const [testFileList, setTestFileList] = React.useState([]);
+
+      return (
+        <Upload
+          ref={uploadRef}
+          fileList={testFileList}
+          action="https://www.mocky.io/v2/5cc8019d300000980a055e76"
+          multiple
+          onChange={info => {
+            setTestFileList([...info.fileList]);
+          }}
+        >
+          <button type="button">Upload</button>
+        </Upload>
+      );
+    };
+
+    mount(<MyUpload />);
+
+    // Mock async update in a frame
+    const files = ['light', 'bamboo', 'little'];
+
+    /* eslint-disable no-await-in-loop */
+    for (let i = 0; i < files.length; i += 1) {
+      await Promise.resolve();
+      uploadRef.current.onStart({
+        uid: files[i],
+        name: files[i],
+      });
+    }
+    /* eslint-enable */
+
+    expect(uploadRef.current.fileList).toHaveLength(files.length);
+
+    jest.runAllTimers();
+    expect(uploadRef.current.fileList).toHaveLength(files.length);
+
+    jest.useRealTimers();
+  });
 });

components/upload/useFreshState.ts

@@ -0,0 +1,56 @@
+import { useRef, useEffect } from 'react';
+import raf from 'rc-util/lib/raf';
+import useForceUpdate from '../_util/hooks/useForceUpdate';
+
+// Note. Only for upload usage. Do not export to global util hooks
+export default function useFreshState<T>(
+  defaultValue: T,
+  propValue?: T,
+): [(displayValue?: boolean) => T, (newValue: T) => void] {
+  const valueRef = useRef(defaultValue);
+  const forceUpdate = useForceUpdate();
+  const rafRef = useRef<number>();
+
+  // Set value
+  function setValue(newValue: T) {
+    valueRef.current = newValue;
+    forceUpdate();
+  }
+
+  function cleanUp() {
+    raf.cancel(rafRef.current!);
+  }
+
+  function rafSyncValue(newValue: T) {
+    cleanUp();
+    rafRef.current = raf(() => {
+      setValue(newValue);
+    });
+  }
+
+  // Get value
+  function getValue(displayValue = false) {
+    if (displayValue) {
+      return propValue || valueRef.current;
+    }
+
+    return valueRef.current;
+  }
+
+  // Effect will always update in a next frame to avoid sync state overwrite current processing state
+  useEffect(() => {
+    if (propValue) {
+      rafSyncValue(propValue);
+    }
+  }, [propValue]);
+
+  // Clean up
+  useEffect(
+    () => () => {
+      cleanUp();
+    },
+    [],
+  );
+
+  return [getValue, setValue];
+}

package.json

@@ -147,7 +147,7 @@
     "rc-tree": "~3.9.0",
     "rc-tree-select": "~4.1.1",
     "rc-trigger": "~4.4.0",
-    "rc-upload": "~3.3.0",
+    "rc-upload": "~3.3.1",
     "rc-util": "^5.1.0",
     "scroll-into-view-if-needed": "^2.2.25",
     "warning": "^4.0.3"