From 89d278282a979713534bf64d05a8efd6401b997f Mon Sep 17 00:00:00 2001 From: lh01217311 Date: Tue, 15 Sep 2026 12:18:36 +0800 Subject: [PATCH 1/4] feat: return whether upload started from retry/post to fix loading stuck ant-design's Upload retry (PR #59286) optimistically sets file status to `uploading` before calling rc-upload's `retry()`. When `beforeUpload` returns false, `retry` skips `post`, so no `onStart/onSuccess/onError` fires and the file gets stuck in loading forever. Return a signal so downstream consumers can decide loading state: - `post` is invoked -> `retry` resolves to `true` - rejected by `beforeUpload`, rejected `action`, or catch -> resolver returns from the relevant branch `Upload.retry` propagates the `Promise` to consumers. Co-Authored-By: Claude --- README.md | 8 ++++---- README.zh-CN.md | 8 ++++---- src/AjaxUploader.tsx | 7 ++++--- src/Upload.tsx | 4 ++-- tests/uploader.spec.tsx | 39 +++++++++++++++++---------------------- 5 files changed, 31 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index fd0e832..0cd3699 100644 --- a/README.md +++ b/README.md @@ -97,10 +97,10 @@ Then open `http://localhost:8000`. ### Methods -| Name | Type | Description | -| ------- | ------------------------ | ------------------------------------ | -| `abort` | `(file: RcFile) => void` | Abort an active upload. | -| `retry` | `(file: RcFile) => void` | Retry an upload for a specific file. | +| Name | Type | Description | +| ------- | ------------------------------------ | ------------------------------------ | +| `abort` | `(file: RcFile) => void` | Abort an active upload. | +| `retry` | `(file: RcFile) => Promise` | Retry an upload for a specific file. | ## Development diff --git a/README.zh-CN.md b/README.zh-CN.md index 8b35710..3344e48 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -97,10 +97,10 @@ npm start ### 方法 -| 名称 | 类型 | 说明 | -| ------- | ------------------------ | -------------------- | -| `abort` | `(file: RcFile) => void` | 中止进行中的上传。 | -| `retry` | `(file: RcFile) => void` | 重试特定文件的上传。 | +| 名称 | 类型 | 说明 | +| ------- | ------------------------------------ | -------------------- | +| `abort` | `(file: RcFile) => void` | 中止进行中的上传。 | +| `retry` | `(file: RcFile) => Promise` | 重试特定文件的上传。 | ## 本地开发 diff --git a/src/AjaxUploader.tsx b/src/AjaxUploader.tsx index 2d4de93..f5ddec5 100644 --- a/src/AjaxUploader.tsx +++ b/src/AjaxUploader.tsx @@ -301,18 +301,19 @@ class AjaxUploader extends Component { this.reqs[uid] = request(requestOption, { defaultRequest }); } - retry = (originFile: RcFile) => { + retry = async (originFile: RcFile): Promise => { const { uid } = originFile; - this.processFile(originFile, [originFile]) + return this.processFile(originFile, [originFile]) .then(fileInfo => { if (this.reqs[uid]) { return; } if (fileInfo.parsedFile) { this.post(fileInfo); + return true; } }) - .catch(() => {}); + .catch(() => false); }; reset() { diff --git a/src/Upload.tsx b/src/Upload.tsx index 46f4dac..b67987f 100644 --- a/src/Upload.tsx +++ b/src/Upload.tsx @@ -30,8 +30,8 @@ class Upload extends Component { this.uploader.abort(file); } - retry(file: RcFile) { - this.uploader.retry(file); + retry(file: RcFile): Promise { + return this.uploader.retry(file); } saveUploader = (node: AjaxUpload) => { diff --git a/tests/uploader.spec.tsx b/tests/uploader.spec.tsx index 41f42fe..8f53955 100644 --- a/tests/uploader.spec.tsx +++ b/tests/uploader.spec.tsx @@ -253,7 +253,7 @@ describe('uploader', () => { }, 100); }); - it('retry should make new request', done => { + it('retry should make new request', async () => { const uploadRef = React.createRef(); render(); @@ -268,15 +268,13 @@ describe('uploader', () => { const initialRequestCount = requests.length; - uploadRef.current.retry(file as any); + const result = await uploadRef.current.retry(file as any); - setTimeout(() => { - expect(requests.length).toBe(initialRequestCount + 1); - done(); - }, 100); + expect(result).toBe(true); + expect(requests.length).toBe(initialRequestCount + 1); }); - it('retry should not make request when action rejects', done => { + it('retry should not make request when action rejects', async () => { const uploadRef = React.createRef(); render( { const initialRequestCount = requests.length; - uploadRef.current.retry(file as any); + const result = await uploadRef.current.retry(file as any); - setTimeout(() => { - expect(requests.length).toBe(initialRequestCount); - done(); - }, 100); + expect(result).toBe(false); + expect(requests.length).toBe(initialRequestCount); }); - it('retry should not start overlapping request for the same file', done => { + it('retry should not start overlapping request for the same file', async () => { const uploadRef = React.createRef(); render(); @@ -318,18 +314,17 @@ describe('uploader', () => { const initialRequestCount = requests.length; - uploadRef.current.retry(file as any); - uploadRef.current.retry(file as any); + const firstResult = uploadRef.current.retry(file as any); + const secondResult = uploadRef.current.retry(file as any); + const [first] = await Promise.all([firstResult, secondResult]); - setTimeout(() => { - expect(requests.length).toBe(initialRequestCount + 1); + expect(first).toBe(true); + expect(requests.length).toBe(initialRequestCount + 1); - expect(requests[requests.length - 1].aborted).toBeFalsy(); + expect(requests[requests.length - 1].aborted).toBeFalsy(); - uploadRef.current.abort(file); - expect(requests[requests.length - 1].aborted).toBe(true); - done(); - }, 100); + uploadRef.current.abort(file); + expect(requests[requests.length - 1].aborted).toBe(true); }); it('drag to upload', done => { From 929d96fe0a59eaca3b688b867b7345b4216b2503 Mon Sep 17 00:00:00 2001 From: lh01217311 Date: Tue, 15 Sep 2026 13:16:18 +0800 Subject: [PATCH 2/4] fix: return false for skipped retry branches to honor Promise Address review feedback on #731: the `this.reqs[uid]` and empty `parsedFile` branches fell through to `undefined`, breaking the `Promise` contract even though `catch` already resolves `false`. Make every no-upload path resolve `false` so consumers can rely on a strict boolean, and assert the second overlapping retry resolves `false`. Co-Authored-By: Claude --- src/AjaxUploader.tsx | 3 ++- tests/uploader.spec.tsx | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/AjaxUploader.tsx b/src/AjaxUploader.tsx index f5ddec5..609d443 100644 --- a/src/AjaxUploader.tsx +++ b/src/AjaxUploader.tsx @@ -306,12 +306,13 @@ class AjaxUploader extends Component { return this.processFile(originFile, [originFile]) .then(fileInfo => { if (this.reqs[uid]) { - return; + return false; } if (fileInfo.parsedFile) { this.post(fileInfo); return true; } + return false; }) .catch(() => false); }; diff --git a/tests/uploader.spec.tsx b/tests/uploader.spec.tsx index 8f53955..0b3520f 100644 --- a/tests/uploader.spec.tsx +++ b/tests/uploader.spec.tsx @@ -316,9 +316,10 @@ describe('uploader', () => { const firstResult = uploadRef.current.retry(file as any); const secondResult = uploadRef.current.retry(file as any); - const [first] = await Promise.all([firstResult, secondResult]); + const [first, second] = await Promise.all([firstResult, secondResult]); expect(first).toBe(true); + expect(second).toBe(false); expect(requests.length).toBe(initialRequestCount + 1); expect(requests[requests.length - 1].aborted).toBeFalsy(); From 2336b884b25e11914f661f3ddcdccf8c5b428472 Mon Sep 17 00:00:00 2001 From: lh01217311 Date: Tue, 15 Sep 2026 13:33:26 +0800 Subject: [PATCH 3/4] test: cover beforeUpload-rejected retry path restoring coverage Add a case asserting `retry` resolves `false` and makes no request when `beforeUpload` returns false. This exercises the empty-`parsedFile` branch (`return false`) added in the previous commit, which the existing "action rejects" case does not reach (it hits `.catch` instead), and restores project coverage above the master baseline. Co-Authored-By: Claude --- tests/uploader.spec.tsx | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/uploader.spec.tsx b/tests/uploader.spec.tsx index 0b3520f..87838c7 100644 --- a/tests/uploader.spec.tsx +++ b/tests/uploader.spec.tsx @@ -300,6 +300,25 @@ describe('uploader', () => { expect(requests.length).toBe(initialRequestCount); }); + it('retry should resolve false when beforeUpload rejects', async () => { + const uploadRef = React.createRef(); + render( false} />); + + const file = { + name: 'before-reject.png', + toString() { + return this.name; + }, + }; + + const initialRequestCount = requests.length; + + const result = await uploadRef.current.retry(file as any); + + expect(result).toBe(false); + expect(requests.length).toBe(initialRequestCount); + }); + it('retry should not start overlapping request for the same file', async () => { const uploadRef = React.createRef(); render(); From 8a89e64b878a981609846bc20c90a1ce98e73118 Mon Sep 17 00:00:00 2001 From: lh01217311 Date: Wed, 16 Sep 2026 17:16:47 +0800 Subject: [PATCH 4/4] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20sync-?= =?UTF-8?q?fail=20retry,=20cache=20cleanup,=20contract=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per zombieJ's review on #731: - Sync callback no longer leaves a dead in-flight marker. customRequest may synchronously call onError/onSuccess (which delete reqs[uid]) and return void; the placeholder must be installed before request(), and the handle is written back only when the entry still exists — otherwise the post-delete assignment wrote back {} and blocked every later retry. Add a regression test (customRequest that synchronously fails). - Clear fileInfoCache on success. A successful file is never retried (callers expose retry only for failed files), so its cached origin/parsedFile Blob references can be released instead of held until unmount. - Document the retry contract in README (en/zh): true/false meaning and that retry reuses the first upload's fileInfo (no beforeUpload/action/data re-run), so only previously uploaded files can be retried. 62 passed, tsc clean. Co-Authored-By: Claude --- README.md | 8 +-- README.zh-CN.md | 8 +-- src/AjaxUploader.tsx | 43 ++++++++----- tests/uploader.spec.tsx | 136 +++++++++++++++++++++++++++++----------- 4 files changed, 135 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 0cd3699..e04143b 100644 --- a/README.md +++ b/README.md @@ -97,10 +97,10 @@ Then open `http://localhost:8000`. ### Methods -| Name | Type | Description | -| ------- | ------------------------------------ | ------------------------------------ | -| `abort` | `(file: RcFile) => void` | Abort an active upload. | -| `retry` | `(file: RcFile) => Promise` | Retry an upload for a specific file. | +| Name | Type | Description | +| --- | --- | --- | +| `abort` | `(file: RcFile) => void` | Abort an active upload. | +| `retry` | `(file: RcFile) => Promise` | Retry an upload for a specific file. Resolves `true` if a request was started, otherwise `false` (file never uploaded, an upload is in flight, or unmounted). Reuses the fileInfo from the first upload (does not re-run `beforeUpload` / `action` / `data`), so only files that have been uploaded before can be retried. | ## Development diff --git a/README.zh-CN.md b/README.zh-CN.md index 3344e48..83df828 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -97,10 +97,10 @@ npm start ### 方法 -| 名称 | 类型 | 说明 | -| ------- | ------------------------------------ | -------------------- | -| `abort` | `(file: RcFile) => void` | 中止进行中的上传。 | -| `retry` | `(file: RcFile) => Promise` | 重试特定文件的上传。 | +| 名称 | 类型 | 说明 | +| --- | --- | --- | +| `abort` | `(file: RcFile) => void` | 中止进行中的上传。 | +| `retry` | `(file: RcFile) => Promise` | 重试特定文件的上传。发起了请求返回 `true`，否则返回 `false`（文件从未上传过、已有请求在飞、或组件已卸载）。复用首次上传的 fileInfo（不再执行 `beforeUpload` / 重算 `action` / `data`），因此只能重试已上传过的文件。 | ## 本地开发 diff --git a/src/AjaxUploader.tsx b/src/AjaxUploader.tsx index 609d443..7d34cbd 100644 --- a/src/AjaxUploader.tsx +++ b/src/AjaxUploader.tsx @@ -27,6 +27,8 @@ class AjaxUploader extends Component { reqs: Record = {}; + private fileInfoCache: Map = new Map(); + private fileInput: HTMLInputElement; private _isMounted: boolean; @@ -260,15 +262,17 @@ class AjaxUploader extends Component { }; }; - post({ data, origin, action, parsedFile }: ParsedFileInfo) { + post({ data, origin, action, parsedFile }: ParsedFileInfo): boolean { if (!this._isMounted) { - return; + return false; } - const { onStart, customRequest, name, headers, withCredentials, method } = this.props; - const { uid } = origin; + this.fileInfoCache.set(uid, { data, origin, action, parsedFile }); + + const { onStart, customRequest, name, headers, withCredentials, method } = this.props; + const request = customRequest || defaultRequest; const requestOption = { @@ -287,6 +291,7 @@ class AjaxUploader extends Component { const { onSuccess } = this.props; onSuccess?.(ret, parsedFile, xhr); + this.fileInfoCache.delete(uid); delete this.reqs[uid]; }, onError: (err: UploadRequestError, ret: any) => { @@ -298,23 +303,27 @@ class AjaxUploader extends Component { }; onStart(origin); - this.reqs[uid] = request(requestOption, { defaultRequest }); + this.reqs[uid] = {}; + try { + const handle = request(requestOption, { defaultRequest }); + if (this.reqs[uid]) { + this.reqs[uid] = handle || {}; + } + } catch (e) { + delete this.reqs[uid]; + return false; + } + return true; } retry = async (originFile: RcFile): Promise => { const { uid } = originFile; - return this.processFile(originFile, [originFile]) - .then(fileInfo => { - if (this.reqs[uid]) { - return false; - } - if (fileInfo.parsedFile) { - this.post(fileInfo); - return true; - } - return false; - }) - .catch(() => false); + const cachedFileInfo = this.fileInfoCache.get(uid); + if (!cachedFileInfo || this.reqs[uid]) { + return false; + } + + return this.post(cachedFileInfo); }; reset() { diff --git a/tests/uploader.spec.tsx b/tests/uploader.spec.tsx index 87838c7..a1ccbf0 100644 --- a/tests/uploader.spec.tsx +++ b/tests/uploader.spec.tsx @@ -253,75 +253,58 @@ describe('uploader', () => { }, 100); }); - it('retry should make new request', async () => { + it('retry should return false when the file was never uploaded', async () => { const uploadRef = React.createRef(); render(); const file = { - name: 'retry.png', + name: 'never-uploaded.png', toString() { return this.name; }, }; - const files = [file]; - (files as any).item = (i: number) => files[i]; const initialRequestCount = requests.length; const result = await uploadRef.current.retry(file as any); - expect(result).toBe(true); - expect(requests.length).toBe(initialRequestCount + 1); + expect(result).toBe(false); + expect(requests.length).toBe(initialRequestCount); }); - it('retry should not make request when action rejects', async () => { + it('retry should make a new request for a previously uploaded file', async () => { const uploadRef = React.createRef(); - render( - { - throw new Error('action error'); - }} - />, + const retryUploader = render( + {}} />, ); const file = { - name: 'reject.png', + name: 'retry.png', toString() { return this.name; }, }; + const files = [file]; + (files as any).item = (i: number) => files[i]; - const initialRequestCount = requests.length; - - const result = await uploadRef.current.retry(file as any); - - expect(result).toBe(false); - expect(requests.length).toBe(initialRequestCount); - }); - - it('retry should resolve false when beforeUpload rejects', async () => { - const uploadRef = React.createRef(); - render( false} />); + const input = retryUploader.container.querySelector('input')!; + fireEvent.change(input, { target: { files } }); - const file = { - name: 'before-reject.png', - toString() { - return this.name; - }, - }; + await sleep(0); + requests[0].respond(400, {}, `error 400`); const initialRequestCount = requests.length; const result = await uploadRef.current.retry(file as any); - expect(result).toBe(false); - expect(requests.length).toBe(initialRequestCount); + expect(result).toBe(true); + expect(requests.length).toBe(initialRequestCount + 1); + retryUploader.unmount(); }); it('retry should not start overlapping request for the same file', async () => { const uploadRef = React.createRef(); - render(); + const retryUploader = render( {}} />); const file = { name: 'overlap.png', @@ -330,6 +313,14 @@ describe('uploader', () => { }, }; (file as any).uid = 'fixed-overlap-uid'; + const files = [file]; + (files as any).item = (i: number) => files[i]; + + const input = retryUploader.container.querySelector('input')!; + fireEvent.change(input, { target: { files } }); + + await sleep(0); + requests[0].respond(400, {}, `error 400`); const initialRequestCount = requests.length; @@ -345,6 +336,81 @@ describe('uploader', () => { uploadRef.current.abort(file); expect(requests[requests.length - 1].aborted).toBe(true); + + retryUploader.unmount(); + }); + + it('retry should not overlap when customRequest returns void', async () => { + const uploadRef = React.createRef(); + const retryUploader = render( + {}} customRequest={() => {}} />, + ); + + const file = { + name: 'void-retry.png', + toString() { + return this.name; + }, + }; + const files = [file]; + (files as any).item = (i: number) => files[i]; + + const input = retryUploader.container.querySelector('input')!; + // First upload caches the fileInfo; customRequest returns void so reqs[uid] + // is the `{}` placeholder, which never gets cleared by any callback. + fireEvent.change(input, { target: { files } }); + await sleep(0); + // Abort clears reqs[uid] (the `{}` has no .abort, so the call is skipped) + // while leaving fileInfoCache intact for retry to reuse. + uploadRef.current.abort(file); + + // Two concurrent retries race: the `|| {}` placeholder keeps reqs[uid] truthy + // after the first post(), so the second retry must be blocked. + const [first, second] = await Promise.all([ + uploadRef.current.retry(file as any), + uploadRef.current.retry(file as any), + ]); + + expect(first).toBe(true); + expect(second).toBe(false); + + retryUploader.unmount(); + }); + + it('retry should work after customRequest fails synchronously', async () => { + const uploadRef = React.createRef(); + const retryUploader = render( + {}} + customRequest={option => { + // Synchronously fail and return void. + option.onError(new Error('sync fail'), null); + }} + />, + ); + + const file = { + name: 'sync-fail.png', + toString() { + return this.name; + }, + }; + const files = [file]; + (files as any).item = (i: number) => files[i]; + + const input = retryUploader.container.querySelector('input')!; + fireEvent.change(input, { target: { files } }); + await sleep(0); + + // After a synchronous failure the in-flight marker must be cleared, + // otherwise retry would permanently see reqs[uid] and bail out. + const result = await uploadRef.current.retry(file as any); + + expect(result).toBe(true); + + retryUploader.unmount(); }); it('drag to upload', done => {