refactor(ImageUploader): get complete upload data #6779

Layouwen · 2024-11-10T13:53:35Z

github-actions · 2024-11-10T13:53:46Z

codecov · 2024-11-10T14:02:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.70%. Comparing base (f71756a) to head (ff3fec0).
Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6779      +/-   ##
==========================================
- Coverage   92.70%   92.70%   -0.01%     
==========================================
  Files         335      335              
  Lines        7199     7198       -1     
  Branches     1792     1804      +12     
==========================================
- Hits         6674     6673       -1     
  Misses        490      490              
  Partials       35       35

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

afc163 · 2024-11-11T01:54:17Z

src/components/image-uploader/image-uploader.tsx

@@ -196,7 +196,7 @@ export const ImageUploader = forwardRef<ImageUploaderRef, ImageUploaderProps>(

      setTasks(prev => [...getFinalTasks(prev), ...newTasks])
      const newVal: ImageUploadItem[] = []
-      await Promise.all(
+      await Promise.allSettled(


这个兼容性比起 all 差很多。

这个兼容性比起 all 差很多。

改为 for 循环, 成功的时候 push 值, 失败的话 push null 并在 catch log error 如何

这个解法看起来和之前 Promise.all 里去掉 throw 是差不多意思？

zombieJ · 2024-11-11T03:50:17Z

src/components/image-uploader/image-uploader.tsx

@@ -225,10 +225,10 @@ export const ImageUploader = forwardRef<ImageUploaderRef, ImageUploaderProps>(
                return task
              })
            })
-            throw e
+            console.error(e)


测试用例~~

zombieJ · 2024-11-11T07:11:30Z

src/components/image-uploader/tests/image-uploader.test.tsx

@@ -35,6 +35,19 @@ export async function mockUploadFail() {
  throw new Error('Fail to upload')
 }

+export function mockUploadWithFailure(failOnCount: number) {


这个最好放 get all upload url 测试里定义，内聚一下~

zombieJ · 2024-11-11T07:31:15Z

src/components/image-uploader/tests/image-uploader.test.tsx

+
+    expect(fn.mock.lastCall[0].length).toBe(4)
+
+    function mockUploadWithFailure(failOnCount: number) {


定义放最前面

zombieJ · 2024-11-11T07:41:56Z

src/components/image-uploader/tests/image-uploader.test.tsx

+      jest.runAllTimers()
+    })
+
+    expect(fn.mock.lastCall[0].length).toBe(4)


失败的 2 个要去掉，否则实际上传和 UI 会对不上

zombieJ · 2024-11-11T09:08:14Z

src/components/image-uploader/tests/image-uploader.test.tsx

+      jest.runAllTimers()
+    })
+
+    expect(fn.mock.lastCall[0].length).toBe(3)


对比一下现在的 list file name 是否和 mock input 里的 file list 是对上的~

afc163 · 2024-11-13T02:07:06Z

PR 标题不对。

refactor: use Promise.allSettled

3869c96

fix: console.error test

c14473c

Layouwen requested a review from zombieJ November 10, 2024 14:05

afc163 reviewed Nov 11, 2024

View reviewed changes

refactor: use Promise.all

07a2cdd