Skip to content

Hotfix/async upload ux#225

Merged
smarcet merged 10 commits intov4.xfrom
hotfix/async-upload-ux
Apr 23, 2026
Merged

Hotfix/async upload ux#225
smarcet merged 10 commits intov4.xfrom
hotfix/async-upload-ux

Conversation

@smarcet
Copy link
Copy Markdown
Collaborator

@smarcet smarcet commented Apr 22, 2026

ref: https://app.clickup.com/t/86b9gt7w3

Summary by CodeRabbit

  • New Features

    • Added support for asynchronous upload processing with status polling.
    • Implemented concurrent chunk upload limiting to improve performance.
    • Enhanced upload progress tracking with better accuracy and reduced oscillation.
    • Added configurable maximum concurrent uploads option (default: 6).
  • Tests

    • Added comprehensive test coverage for upload functionality.
  • Chores

    • Updated to version 4.2.28-beta.5.

smarcet added 2 commits April 22, 2026 17:48
Signed-off-by: smarcet <smarcet@gmail.com>
@smarcet smarcet requested a review from santipalenque April 22, 2026 20:54
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 22, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7e2c9317-2bec-473a-88e2-feb482cb0b90

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • ✅ Review completed - (🔄 Check again to review again)
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hotfix/async-upload-ux

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@smarcet smarcet removed the request for review from santipalenque April 22, 2026 21:44
smarcet added 2 commits April 22, 2026 18:50
…king

Move _asyncProcessing flag before dropzoneOnLoad so chunksUploaded
callback sees it (regression from Iteration 2 refactor). Replace
_maxProgress guard with _completedBytes floor for accurate progress
that never oscillates. Add tests for progress monotonicity.
@smarcet smarcet requested a review from santipalenque April 22, 2026 22:34
Copy link
Copy Markdown
Contributor

@santipalenque santipalenque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/components/inputs/dropzone/index.js (3)

355-368: ⚠️ Potential issue | 🟡 Minor

Guard against file.size === 0 to avoid NaN width.

effectiveBytes / file.size * 100 produces NaN when file.size is 0 (or undefined), which then becomes "NaN%" on the progress element. Add a zero-check:

🛡️ Proposed fix
-            const effectiveBytes = Math.max(bytesSent, file._completedBytes || 0);
-            progress = Math.min(effectiveBytes / file.size * 100, 100);
+            const effectiveBytes = Math.max(bytesSent, file._completedBytes || 0);
+            progress = file.size > 0
+                ? Math.min(effectiveBytes / file.size * 100, 100)
+                : 0;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/inputs/dropzone/index.js` around lines 355 - 368, The upload
progress handler (this.dropzone.on('uploadprogress')) can produce NaN when
file.size is 0 or undefined; guard file.size by computing progress only when
file.size > 0 and otherwise set progress to 0 (or 100 if you consider
completedBytes equals size) before using it to set the preview width. Update the
logic around effectiveBytes / file.size * 100 to check file.size and use a
fallback progress value, and keep references to file._completedBytes,
effectiveBytes, progress and file.previewElement so the element width assignment
still uses a valid percentage string.

78-114: ⚠️ Potential issue | 🟠 Major

Async setInterval callback can overlap itself, producing overlapping fetch calls.

The callback is async and performs await getAccessToken()await fetch()await response.json(). If any of those take longer than the 2 s tick (slow network, token refresh, backend stall), the next tick fires while the previous is still in flight, producing duplicate status requests and racing clearInterval calls on the same id. Either switch to a self-rescheduling setTimeout pattern (schedule the next tick only after the previous awaited work resolves), or guard with an inFlight flag on the file.

🛡️ Recommended pattern
const tick = async () => {
    attempts++;
    if (attempts > maxAttempts) { /* ... */ return; }
    try {
        // ... await work ...
        if (!done) setTimeout(tick, 2000);
    } catch (e) { /* ... */ }
};
setTimeout(tick, 2000);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/inputs/dropzone/index.js` around lines 78 - 114, The async
setInterval callback can overlap itself causing duplicate fetches and racing
clears; replace the setInterval-based polling around this._pollInterval with a
self-rescheduling async tick using setTimeout (or add a per-file inFlight guard)
so each tick awaits getAccessToken(), fetch(statusUrl) and response.json() to
completion before scheduling the next call; preserve existing logic for attempts
> maxAttempts, clearing file._pollingActive, calling file._chunksUploadedDone(),
and invoking this.onUploadComplete / this.onError, and ensure this._pollInterval
is set/cleared consistently (or store the timeout id if you keep using a timer).

67-115: ⚠️ Potential issue | 🔴 Critical

Critical: this._pollInterval is shared across files — concurrent 202 uploads will leak intervals and clear the wrong one.

_pollInterval is a single instance field. If two files return HTTP 202 (e.g., maxFiles > 1 or multiple dropzones in one instance), the second pollUploadStatus call overwrites this._pollInterval before the first has finished, and every subsequent clearInterval(this._pollInterval) (on timeout/error/complete/unmount) only affects whichever interval is currently stored — the other one keeps polling forever (or gets clobbered mid-flight). The file._pollingActive guard only prevents re-entering polling for the same file; it does nothing for different files.

Store the interval id on the file itself (it already carries _pollingActive / _chunksUploadedDone) and track active intervals in a Map/Set so componentWillUnmount can clear them all.

🔒 Suggested direction
-    pollUploadStatus(fileId, baseUrl, file) {
+    pollUploadStatus(fileId, baseUrl, file) {
         // Guard against multiple polling intervals for the same file
         if (file._pollingActive) {
             return;
         }
         file._pollingActive = true;

         const statusUrl = `${baseUrl}/status/${fileId}`;
         const maxAttempts = 300; // 10 minutes at 2s intervals
         let attempts = 0;

-        this._pollInterval = setInterval(async () => {
+        const intervalId = setInterval(async () => {
             attempts++;
             if (attempts > maxAttempts) {
-                clearInterval(this._pollInterval);
-                this._pollInterval = null;
+                clearInterval(intervalId);
+                this._pollIntervals?.delete(intervalId);
                 file._pollingActive = false;
                 this.onError({ message: 'Upload timed out' });
                 return;
             }
             // ... use intervalId in every clearInterval site ...
         }, 2000);
+        (this._pollIntervals ||= new Set()).add(intervalId);
     }

And in componentWillUnmount, iterate this._pollIntervals instead of the single _pollInterval.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/inputs/dropzone/index.js` around lines 67 - 115, The bug is
that pollUploadStatus uses a single this._pollInterval for all files, causing
intervals to be overwritten and leaked; change pollUploadStatus to attach the
interval id to the specific file (e.g., file._pollIntervalId) and maintain a
container on the component (e.g., this._pollIntervals = new Set() or Map()) to
track active intervals; when creating the interval assign its id to
file._pollIntervalId and add it to this._pollIntervals, replace every
clearInterval(this._pollInterval) with clearInterval(file._pollIntervalId) plus
removal from this._pollIntervals and nulling
file._pollIntervalId/file._pollingActive, and update componentWillUnmount to
iterate this._pollIntervals and clear them all so no per-file polling can leak.
🧹 Nitpick comments (6)
src/components/inputs/dropzone/__tests__/dropzone.test.js (3)

148-149: Stale TDD comment.

The comment "THIS TEST SHOULD FAIL INITIALLY (demonstrating the bug)" is a leftover from the red-phase of TDD. Now that the fix is in place, this test is expected to pass and the note is misleading for future readers.

📝 Proposed edit
-   *
-   * THIS TEST SHOULD FAIL INITIALLY (demonstrating the bug)
    */
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/inputs/dropzone/__tests__/dropzone.test.js` around lines 148 -
149, Remove or update the stale TDD comment "THIS TEST SHOULD FAIL INITIALLY
(demonstrating the bug)" in the dropzone.test.js file; find the comment near the
test block (in the Dropzone tests) and either delete it or replace it with an
accurate note stating the test now passes, ensuring comments around the relevant
test(s) reflect current behavior (e.g., in the describe/it block for the
Dropzone component).

283-294: DropzoneMock.mockImplementation leaks across tests within this describe block.

jest.clearAllMocks() in beforeEach clears call history but does not reset mockImplementation. The override set here (and again at line 371) persists for subsequent tests and — since both describe blocks share the same module-level jest.mock('dropzone', …) factory — can also bleed into re-runs if this file is re-entered. Either move the mockImplementation into beforeEach, or call DropzoneMock.mockReset()/mockRestore() in afterEach to avoid hidden coupling between tests.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/inputs/dropzone/__tests__/dropzone.test.js` around lines 283 -
294, The test-suite-level DropzoneMock.mockImplementation in the describe block
leaks across tests because jest.clearAllMocks() doesn't reset implementations;
move the mock implementation into the beforeEach (so
DropzoneMock.mockImplementation(...) is called per-test) or add
DropzoneMock.mockReset() or DropzoneMock.mockRestore() in afterEach to clear the
implementation; update references to DropzoneMock.mockImplementation,
beforeEach, and afterEach in the test file so each test gets a fresh mock
implementation and no state bleeds between tests.

191-240: Test 4 is timing-fragile and may be flaky in CI.

pollUploadStatus uses setInterval(..., 2000). The test waits exactly 2500 ms, leaving only a 500 ms window for the async callback (token fetch + fetch + response.json() + assertions) to complete after the interval fires. On a loaded CI runner this can occasionally slip, producing a false failure. Consider either using Jest fake timers (jest.useFakeTimers() + jest.advanceTimersByTimeAsync(2000) then await flushPromises()), or refactoring pollUploadStatus to accept a configurable interval so tests can pass a much smaller value (e.g. 10 ms).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/inputs/dropzone/__tests__/dropzone.test.js` around lines 191 -
240, The test is flaky because pollUploadStatus uses a hard-coded 2000ms
setInterval and the test only waits 2500ms; update the test to use Jest fake
timers and advance them instead or make pollUploadStatus accept a configurable
interval to speed up polling in tests. Fix by either (A) in the test call
jest.useFakeTimers(), call instance.pollUploadStatus(...) then await
Promise.resolve/flushPromises and run jest.advanceTimersByTimeAsync(2000) (or
advanceTimersByTime and then await flushPromises) before asserting, or (B)
refactor the DropzoneJS.pollUploadStatus signature to accept an optional
intervalMs (default 2000) and in the test call instance.pollUploadStatus(...,
10) so you can wait a small timeout; reference pollUploadStatus and the test
name test_dropzone_202_polling_complete_fires_success when making the change.
src/components/inputs/dropzone/index.js (3)

464-466: Add maxConcurrentChunks to propTypes.

The new public prop consumed in processChunkQueue is not declared in DropzoneJS.propTypes, which is the idiomatic way to document the component's API for this codebase (v2 and v3 both forward it). A PropTypes.number entry (and optionally a defaultProps value) would make the contract explicit and replace the this.props.maxConcurrentChunks || 6 fallback scattered in code.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/inputs/dropzone/index.js` around lines 464 - 466, DropzoneJS
currently declares only id in DropzoneJS.propTypes but uses the new public prop
maxConcurrentChunks inside processChunkQueue; add maxConcurrentChunks:
PropTypes.number to DropzoneJS.propTypes and optionally add
DropzoneJS.defaultProps = { maxConcurrentChunks: 6 } (or adjust the default
there) so the component API is explicit and you can remove scattered
this.props.maxConcurrentChunks || 6 fallbacks in processChunkQueue.

203-235: Unmount does not clear an in-progress poll interval when the file is still polling.

componentWillUnmount clears this._pollInterval and aborts active XHRs, but once the critical issue above is addressed (multiple concurrent polls), unmount must iterate and clear all active polling intervals. Also note files.forEach at line 212 does not use the file key and the two branches of the files.length > 0 conditional both call this.destroy(this.dropzone) identically — the if/else can be collapsed.

♻️ Simplification
-            if (files.length > 0) {
-                // Cancel active uploads before destroying
-                files.forEach(file => {
-                    this.dropzone.cancelUpload(file);
-                });
-
-                this.dropzone = this.destroy(this.dropzone);
-            } else {
-                this.dropzone = this.destroy(this.dropzone)
-            }
+            files.forEach(file => this.dropzone.cancelUpload(file));
+            this.dropzone = this.destroy(this.dropzone);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/inputs/dropzone/index.js` around lines 203 - 235,
componentWillUnmount must clear every active polling timer and simplify the
dropzone destroy branch: after aborting XHRs, iterate and clear all polling
intervals (not just this._pollInterval) — e.g. if you maintain a collection like
this._pollIntervals, this._pollIntervalMap, this.activePolls or per-file timers
(e.g. file._pollInterval or file.pollInterval), call clearInterval for each and
null them; also collapse the duplicate branch by always calling this.dropzone =
this.destroy(this.dropzone) after canceling uploads (use
this.dropzone.getActiveFiles() and files.forEach(file =>
this.dropzone.cancelUpload(file))). Ensure you still null/clear
this._pollInterval and any per-file timer references so no timers remain after
unmount while keeping existing XHR abort and activeXHRs.clear() logic.

46-65: Both observations are valid; consider adding a comment documenting the private API dependency.

The code correctly handles the throttle logic, but two maintainability concerns remain:

  1. Redundant onChunkComplete() calls for non-chunked uploads: The xhr.onload and xhr.onerror hooks (lines ~401, ~437) call onChunkComplete() unconditionally. Since only chunked uploads increment chunksInFlight (in processChunkQueue), non-chunked uploads uselessly decrement from 0 and scan the queue. To fix: only register these throttle hooks when queuing a chunk, or add a flag to onChunkComplete() to skip processing for non-queued uploads.

  2. Undocumented Dropzone private API: _uploadData is a private Dropzone internal (leading underscore). The code is pinned to dropzone@5.7.2, but any minor/patch upgrade could break this silently. Add a comment explaining the dependency on this internal method and why (e.g., // Uses Dropzone's private _uploadData method (since v5.7.2) to intercept uploads for concurrency throttling).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/inputs/dropzone/index.js` around lines 46 - 65, The file
intercepts Dropzone's private _uploadData in setupChunkThrottle() and
unconditionally calls onChunkComplete() for all uploads; fix by (1) only
attaching the throttle-related xhr.onload/xhr.onerror handlers or marking the
upload as "throttled" when you push to chunkQueue in setupChunkThrottle() so
processChunkQueue() and onChunkComplete() run only for chunked uploads (e.g.,
set a per-upload flag and check it in onChunkComplete() before decrementing
chunksInFlight), and (2) add a short comment in setupChunkThrottle() documenting
the dependency on Dropzone's private _uploadData (include version note like
"Uses Dropzone's private _uploadData method (since v5.7.2) to intercept uploads
for concurrency throttling") so future maintainers know this is a private API
dependency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.gitignore:
- Around line 6-9: Remove the erroneous .gitignore entry "package.json.lock" (a
typo) and keep the correct "package-lock.json" entry instead; edit the
.gitignore to delete the "package.json.lock" line so only the valid npm lock
filename "package-lock.json" remains listed.

In `@src/components/inputs/dropzone/index.js`:
- Around line 400-407: The _completedBytes update in xhr.onload incorrectly
assumes every completed request equals dropzone.options.chunkSize (fallback
2000000), inflating progress for final chunk and for non-chunked uploads; change
the accumulation in xhr.onload to compute the actual bytes delivered for that
XHR (prefer using the current chunk's byte range: dataBlocks[i].dataBlock.end -
dataBlocks[i].dataBlock.start) or only add when the request is a chunked upload
(detect by checking formData.has('dzchunkindex') or the chunked path used by
setupChunkThrottle), and keep calling onChunkComplete() unchanged; update
references in xhr.onload to use the real size instead of chunkSize when
adjusting file._completedBytes and leave Math.min(file._completedBytes +
actualBytes, file.size) for clamping.

---

Outside diff comments:
In `@src/components/inputs/dropzone/index.js`:
- Around line 355-368: The upload progress handler
(this.dropzone.on('uploadprogress')) can produce NaN when file.size is 0 or
undefined; guard file.size by computing progress only when file.size > 0 and
otherwise set progress to 0 (or 100 if you consider completedBytes equals size)
before using it to set the preview width. Update the logic around effectiveBytes
/ file.size * 100 to check file.size and use a fallback progress value, and keep
references to file._completedBytes, effectiveBytes, progress and
file.previewElement so the element width assignment still uses a valid
percentage string.
- Around line 78-114: The async setInterval callback can overlap itself causing
duplicate fetches and racing clears; replace the setInterval-based polling
around this._pollInterval with a self-rescheduling async tick using setTimeout
(or add a per-file inFlight guard) so each tick awaits getAccessToken(),
fetch(statusUrl) and response.json() to completion before scheduling the next
call; preserve existing logic for attempts > maxAttempts, clearing
file._pollingActive, calling file._chunksUploadedDone(), and invoking
this.onUploadComplete / this.onError, and ensure this._pollInterval is
set/cleared consistently (or store the timeout id if you keep using a timer).
- Around line 67-115: The bug is that pollUploadStatus uses a single
this._pollInterval for all files, causing intervals to be overwritten and
leaked; change pollUploadStatus to attach the interval id to the specific file
(e.g., file._pollIntervalId) and maintain a container on the component (e.g.,
this._pollIntervals = new Set() or Map()) to track active intervals; when
creating the interval assign its id to file._pollIntervalId and add it to
this._pollIntervals, replace every clearInterval(this._pollInterval) with
clearInterval(file._pollIntervalId) plus removal from this._pollIntervals and
nulling file._pollIntervalId/file._pollingActive, and update
componentWillUnmount to iterate this._pollIntervals and clear them all so no
per-file polling can leak.

---

Nitpick comments:
In `@src/components/inputs/dropzone/__tests__/dropzone.test.js`:
- Around line 148-149: Remove or update the stale TDD comment "THIS TEST SHOULD
FAIL INITIALLY (demonstrating the bug)" in the dropzone.test.js file; find the
comment near the test block (in the Dropzone tests) and either delete it or
replace it with an accurate note stating the test now passes, ensuring comments
around the relevant test(s) reflect current behavior (e.g., in the describe/it
block for the Dropzone component).
- Around line 283-294: The test-suite-level DropzoneMock.mockImplementation in
the describe block leaks across tests because jest.clearAllMocks() doesn't reset
implementations; move the mock implementation into the beforeEach (so
DropzoneMock.mockImplementation(...) is called per-test) or add
DropzoneMock.mockReset() or DropzoneMock.mockRestore() in afterEach to clear the
implementation; update references to DropzoneMock.mockImplementation,
beforeEach, and afterEach in the test file so each test gets a fresh mock
implementation and no state bleeds between tests.
- Around line 191-240: The test is flaky because pollUploadStatus uses a
hard-coded 2000ms setInterval and the test only waits 2500ms; update the test to
use Jest fake timers and advance them instead or make pollUploadStatus accept a
configurable interval to speed up polling in tests. Fix by either (A) in the
test call jest.useFakeTimers(), call instance.pollUploadStatus(...) then await
Promise.resolve/flushPromises and run jest.advanceTimersByTimeAsync(2000) (or
advanceTimersByTime and then await flushPromises) before asserting, or (B)
refactor the DropzoneJS.pollUploadStatus signature to accept an optional
intervalMs (default 2000) and in the test call instance.pollUploadStatus(...,
10) so you can wait a small timeout; reference pollUploadStatus and the test
name test_dropzone_202_polling_complete_fires_success when making the change.

In `@src/components/inputs/dropzone/index.js`:
- Around line 464-466: DropzoneJS currently declares only id in
DropzoneJS.propTypes but uses the new public prop maxConcurrentChunks inside
processChunkQueue; add maxConcurrentChunks: PropTypes.number to
DropzoneJS.propTypes and optionally add DropzoneJS.defaultProps = {
maxConcurrentChunks: 6 } (or adjust the default there) so the component API is
explicit and you can remove scattered this.props.maxConcurrentChunks || 6
fallbacks in processChunkQueue.
- Around line 203-235: componentWillUnmount must clear every active polling
timer and simplify the dropzone destroy branch: after aborting XHRs, iterate and
clear all polling intervals (not just this._pollInterval) — e.g. if you maintain
a collection like this._pollIntervals, this._pollIntervalMap, this.activePolls
or per-file timers (e.g. file._pollInterval or file.pollInterval), call
clearInterval for each and null them; also collapse the duplicate branch by
always calling this.dropzone = this.destroy(this.dropzone) after canceling
uploads (use this.dropzone.getActiveFiles() and files.forEach(file =>
this.dropzone.cancelUpload(file))). Ensure you still null/clear
this._pollInterval and any per-file timer references so no timers remain after
unmount while keeping existing XHR abort and activeXHRs.clear() logic.
- Around line 46-65: The file intercepts Dropzone's private _uploadData in
setupChunkThrottle() and unconditionally calls onChunkComplete() for all
uploads; fix by (1) only attaching the throttle-related xhr.onload/xhr.onerror
handlers or marking the upload as "throttled" when you push to chunkQueue in
setupChunkThrottle() so processChunkQueue() and onChunkComplete() run only for
chunked uploads (e.g., set a per-upload flag and check it in onChunkComplete()
before decrementing chunksInFlight), and (2) add a short comment in
setupChunkThrottle() documenting the dependency on Dropzone's private
_uploadData (include version note like "Uses Dropzone's private _uploadData
method (since v5.7.2) to intercept uploads for concurrency throttling") so
future maintainers know this is a private API dependency.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 59cb56c6-5727-402a-b401-37c4121e4bd9

📥 Commits

Reviewing files that changed from the base of the PR and between 9717f19 and d133773.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (6)
  • .gitignore
  • package.json
  • src/components/inputs/dropzone/__tests__/dropzone.test.js
  • src/components/inputs/dropzone/index.js
  • src/components/inputs/upload-input-v2/index.js
  • src/components/inputs/upload-input-v3/index.js

Comment thread .gitignore
Comment on lines 6 to +9
package.json.lock
.codegraph
docs
package-lock.json
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove the redundant and incorrectly named entry.

Line 6 contains package.json.lock, which is not the correct name for npm's lock file. The actual file is package-lock.json (added on line 9). Line 6 appears to be a typo and should be removed to avoid confusion.

🧹 Proposed fix to remove the redundant entry
 .idea/
-package.json.lock
 .codegraph
 docs
 package-lock.json
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
package.json.lock
.codegraph
docs
package-lock.json
.codegraph
docs
package-lock.json
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.gitignore around lines 6 - 9, Remove the erroneous .gitignore entry
"package.json.lock" (a typo) and keep the correct "package-lock.json" entry
instead; edit the .gitignore to delete the "package.json.lock" line so only the
valid npm lock filename "package-lock.json" remains listed.

Comment on lines +400 to +407
// Release a slot in the chunk queue for the next chunk
_this.onChunkComplete();

// Track completed bytes for accurate progress (prevents oscillation)
const chunkSize = _this.dropzone?.options?.chunkSize || 2000000;
file._completedBytes = Math.min(
(file._completedBytes || 0) + chunkSize, file.size
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

_completedBytes accounting assumes every chunk is exactly chunkSize.

In xhr.onload you unconditionally add chunkSize (hardcoded fallback 2000000) to _completedBytes for every completed XHR. This is wrong in two ways:

  1. The last chunk is almost always smaller than chunkSize; the floor will jump past the real bytes delivered until clamped by Math.min(..., file.size). The clamp hides this for the final display value but the intermediate floor used in uploadprogress is inflated.
  2. xhr.onload runs for every XHR in this component, including the non-chunked single-POST upload path (setupChunkThrottle bypasses the queue for those) and potentially the 202 status poll path if it went through this handler. For non-chunked uploads _completedBytes gets +2 MB added for a single success, again hidden only by the clamp.

Prefer deriving the advance from the actual request body / dataBlocks[i].dataBlock.end - .start, or gate this accumulation to chunk uploads only (e.g., only when the sending formData carries dzchunkindex).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/inputs/dropzone/index.js` around lines 400 - 407, The
_completedBytes update in xhr.onload incorrectly assumes every completed request
equals dropzone.options.chunkSize (fallback 2000000), inflating progress for
final chunk and for non-chunked uploads; change the accumulation in xhr.onload
to compute the actual bytes delivered for that XHR (prefer using the current
chunk's byte range: dataBlocks[i].dataBlock.end - dataBlocks[i].dataBlock.start)
or only add when the request is a chunked upload (detect by checking
formData.has('dzchunkindex') or the chunked path used by setupChunkThrottle),
and keep calling onChunkComplete() unchanged; update references in xhr.onload to
use the real size instead of chunkSize when adjusting file._completedBytes and
leave Math.min(file._completedBytes + actualBytes, file.size) for clamping.

@smarcet smarcet merged commit 9ddc2f1 into v4.x Apr 23, 2026
5 checks passed
smarcet added a commit that referenced this pull request Apr 23, 2026
* fix(dropzone): fix upload async ux

Signed-off-by: smarcet <smarcet@gmail.com>

* v4.2.28-beta.1

* fix(dropzone): fix request flooding

Signed-off-by: smarcet <smarcet@gmail.com>

* v4.2.28-beta.2

* feat(dropzone): add batch chunking

* v4.2.28-beta.3

* fix(dropzone): prevent progress bar from going backwards during chunked upload

* v4.2.28-beta.4

* fix(dropzone): restore async polling UX fix and improve progress tracking

Move _asyncProcessing flag before dropzoneOnLoad so chunksUploaded
callback sees it (regression from Iteration 2 refactor). Replace
_maxProgress guard with _completedBytes floor for accurate progress
that never oscillates. Add tests for progress monotonicity.

* v4.2.28-beta.5

---------

Signed-off-by: smarcet <smarcet@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants