Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@uppy/transloadit: do not cancel assembly when removing all files #5191

Merged
merged 1 commit into from
May 22, 2024

Conversation

Murderlon
Copy link
Member

A common use case is clear the state of Uppy (with setTimeout) after success. But the @uppy/transloadit plugin cancels all assemblies, even if in progress, when that happens. This is causing issues for two customers.

@Murderlon Murderlon self-assigned this May 22, 2024
Copy link
Contributor

Diff output files
diff --git a/packages/@uppy/transloadit/lib/index.js b/packages/@uppy/transloadit/lib/index.js
index 31e4d6c..65b38d9 100644
--- a/packages/@uppy/transloadit/lib/index.js
+++ b/packages/@uppy/transloadit/lib/index.js
@@ -644,13 +644,7 @@ function _createAssembly2(fileIDs, uploadID, assemblyOptions) {
       } else if (fileRemoved.id in updatedFiles) {
         delete updatedFiles[fileRemoved.id];
         const nbOfRemainingFiles = Object.keys(updatedFiles).length;
-        if (nbOfRemainingFiles === 0) {
-          assembly.close();
-          _classPrivateFieldLooseBase(this, _cancelAssembly)[_cancelAssembly](newAssembly).catch(() => {});
-          this.uppy.off("file-removed", fileRemovedHandler);
-        } else {
-          this.client.updateNumberOfFilesInAssembly(newAssembly, nbOfRemainingFiles).catch(() => {});
-        }
+        this.client.updateNumberOfFilesInAssembly(newAssembly, nbOfRemainingFiles).catch(() => {});
       }
     };
     this.uppy.on("file-removed", fileRemovedHandler);

@Murderlon Murderlon requested review from mifi and aduh95 May 22, 2024 08:55
Copy link
Member

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

Could you add at least a TODO in the test so we don't forget it's a scenario we want to validate?

@Murderlon
Copy link
Member Author

Could you add at least a TODO in the test so we don't forget it's a scenario we want to validate?

Already working on this in a different branch so not needed I think :)

@aduh95 aduh95 merged commit 2597bb9 into main May 22, 2024
17 checks passed
@aduh95 aduh95 deleted the tl-file-removed branch May 22, 2024 11:40
@github-actions github-actions bot mentioned this pull request May 23, 2024
github-actions bot added a commit that referenced this pull request May 23, 2024
| Package           | Version | Package           | Version |
| ----------------- | ------- | ----------------- | ------- |
| @uppy/transloadit |   3.6.2 | uppy              |  3.25.5 |
| @uppy/xhr-upload  |   3.6.7 |                   |         |

- @uppy/transloadit: do not cancel assembly when removing all files (Merlijn Vos / #5191)
- @uppy/xhr-upload: fix regression for lowercase HTTP methods (Antoine du Hamel / #5179)
- meta: improve changelog generator (Antoine du Hamel / #5190)
github-actions bot added a commit that referenced this pull request May 23, 2024
| Package           |      Version | Package           |      Version |
| ----------------- | ------------ | ----------------- | ------------ |
| @uppy/companion   | 5.0.0-beta.8 | @uppy/xhr-upload  | 4.0.0-beta.5 |
| @uppy/transloadit | 4.0.0-beta.6 | uppy              | 4.0.0-beta.9 |

- @uppy/companion: remove `chalk` from dependencies (Antoine du Hamel / #5178)
- @uppy/transloadit: do not cancel assembly when removing all files (Merlijn Vos / #5191)
- @uppy/xhr-upload: fix regression for lowercase HTTP methods (Antoine du Hamel / #5179)
- meta: improve changelog generator (Antoine du Hamel / #5190)
Murderlon added a commit that referenced this pull request May 28, 2024
* 4.x: (38 commits)
  docs: assume tree-shaking bundler is the most common case (#5160)
  @uppy/core: remove `reason` (#5159)
  Release: uppy@4.0.0-beta.9 (#5194)
  Release: uppy@3.25.5 (#5193)
  @uppy/companion: remove `chalk` from dependencies (#5178)
  @uppy/transloadit: do not cancel assembly when removing all files (#5191)
  @uppy/xhr-upload: fix regression for lowercase HTTP methods (#5179)
  meta: improve changelog generator (#5190)
  Release: uppy@4.0.0-beta.8 (#5189)
  examples: add SvelteKit example (#5181)
  @uppy/companion: fix missing `await`
  Release: uppy@3.25.4 (#5188)
  @uppy/svelte: do not attempt removing plugin before it's created (#5186)
  Update facebook.mdx
  fixup! @uppy/tus: fix no headers passed to companion if argument is a function (#5182)
  @uppy/core: resolve some (breaking) TODOs (#4824)
  @uppy/tus: fix no headers passed to companion if argument is a function (#5182)
  @uppy/companion: fix google drive gsuite export large size (#5144)
  @uppy/companion: encode `uploadId` (#5168)
  @uppy/companion: bump `express-session` (#5177)
  ...
Murderlon added a commit that referenced this pull request Jun 10, 2024
* main: (262 commits)
  Release: uppy@3.26.0 (#5223)
  meta: remove Companion's `prepublishOnly` (#5220)
  docs: document clearUploadedFiles (#5204)
  @uppy/webcam: add missing types for `recordedVideo` (#5208)
  @uppy/core: check capabilities in clearUploadedFiles (#5201)
  PartialTree - change the `maxTotalFileSize` error (#5203)
  @uppy/transloadit: remove `updateNumberOfFilesInAssembly` (#5202)
  @uppy/aws-s3: resolve all headers on response (#5195)
  Improve provider docs: OneDrive (#5196)
  Release: uppy@3.25.5 (#5193)
  @uppy/transloadit: do not cancel assembly when removing all files (#5191)
  @uppy/xhr-upload: fix regression for lowercase HTTP methods (#5179)
  meta: improve changelog generator (#5190)
  Release: uppy@3.25.4 (#5188)
  @uppy/svelte: do not attempt removing plugin before it's created (#5186)
  Update facebook.mdx
  fixup! @uppy/tus: fix no headers passed to companion if argument is a function (#5182)
  @uppy/tus: fix no headers passed to companion if argument is a function (#5182)
  @uppy/companion: fix google drive gsuite export large size (#5144)
  Improve provider docs: Box & Zoom (#5166)
  ...
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.

None yet

2 participants