-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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/core: remove reason
#5159
Conversation
Diff output filesdiff --git a/packages/@uppy/aws-s3/lib/index.js b/packages/@uppy/aws-s3/lib/index.js
index 1b136e1..856e4de 100644
--- a/packages/@uppy/aws-s3/lib/index.js
+++ b/packages/@uppy/aws-s3/lib/index.js
@@ -613,13 +613,11 @@ function _uploadLocalFile2(file) {
});
resolve(`upload ${removed} was removed`);
});
- eventManager.onCancelAll(file.id, options => {
- if ((options == null ? void 0 : options.reason) === "user") {
- upload.abort();
- this.resetUploaderReferences(file.id, {
- abort: true,
- });
- }
+ eventManager.onCancelAll(file.id, () => {
+ upload.abort();
+ this.resetUploaderReferences(file.id, {
+ abort: true,
+ });
resolve(`upload ${file.id} was canceled`);
});
eventManager.onFilePause(file.id, isPaused => {
diff --git a/packages/@uppy/companion-client/lib/RequestClient.js b/packages/@uppy/companion-client/lib/RequestClient.js
index a7ef904..6d54db2 100644
--- a/packages/@uppy/companion-client/lib/RequestClient.js
+++ b/packages/@uppy/companion-client/lib/RequestClient.js
@@ -453,14 +453,9 @@ async function _awaitRemoteFileUpload2(_ref4) {
this.uppy.log(`upload ${file.id} was removed`, "info");
resolve();
};
- const onCancelAll = _ref5 => {
+ const onCancelAll = () => {
var _socketAbortControlle5;
- let {
- reason,
- } = _ref5;
- if (reason === "user") {
- socketSend("cancel");
- }
+ socketSend("cancel");
(_socketAbortControlle5 = socketAbortController) == null || _socketAbortControlle5.abort == null
|| _socketAbortControlle5.abort();
this.uppy.log(`upload ${file.id} was canceled`, "info");
diff --git a/packages/@uppy/core/lib/Uppy.js b/packages/@uppy/core/lib/Uppy.js
index 7feaf32..aaf0e13 100644
--- a/packages/@uppy/core/lib/Uppy.js
+++ b/packages/@uppy/core/lib/Uppy.js
@@ -566,7 +566,7 @@ export class Uppy {
_classPrivateFieldLooseBase(this, _startIfAutoProceed)[_startIfAutoProceed]();
}
}
- removeFiles(fileIDs, reason) {
+ removeFiles(fileIDs) {
const {
files,
currentUploads,
@@ -617,7 +617,7 @@ export class Uppy {
this.calculateTotalProgress();
const removedFileIDs = Object.keys(removedFiles);
removedFileIDs.forEach(fileID => {
- this.emit("file-removed", removedFiles[fileID], reason);
+ this.emit("file-removed", removedFiles[fileID]);
});
if (removedFileIDs.length > 5) {
this.log(`Removed ${removedFileIDs.length} files`);
@@ -625,8 +625,8 @@ export class Uppy {
this.log(`Removed files: ${removedFileIDs.join(", ")}`);
}
}
- removeFile(fileID, reason) {
- this.removeFiles([fileID], reason);
+ removeFile(fileID) {
+ this.removeFiles([fileID]);
}
pauseResume(fileID) {
if (!this.getState().capabilities.resumableUploads || this.getFile(fileID).progress.uploadComplete) {
@@ -710,23 +710,16 @@ export class Uppy {
});
return _classPrivateFieldLooseBase(this, _runUpload)[_runUpload](uploadID);
}
- cancelAll(_temp) {
- let {
- reason = "user",
- } = _temp === void 0 ? {} : _temp;
- this.emit("cancel-all", {
- reason,
- });
- if (reason === "user") {
- const {
- files,
- } = this.getState();
- const fileIDs = Object.keys(files);
- if (fileIDs.length) {
- this.removeFiles(fileIDs, "cancel-all");
- }
- this.setState(defaultUploadState);
+ cancelAll() {
+ this.emit("cancel-all");
+ const {
+ files,
+ } = this.getState();
+ const fileIDs = Object.keys(files);
+ if (fileIDs.length) {
+ this.removeFiles(fileIDs);
}
+ this.setState(defaultUploadState);
}
retryUpload(fileID) {
this.setFileState(fileID, {
@@ -880,14 +873,9 @@ export class Uppy {
};
this.setState(updatedState);
}
- destroy(_temp2) {
- let {
- reason,
- } = _temp2 === void 0 ? {} : _temp2;
+ destroy() {
this.log(`Closing Uppy instance ${this.opts.id}: removing all files and uninstalling plugins`);
- this.cancelAll({
- reason,
- });
+ this.cancelAll();
_classPrivateFieldLooseBase(this, _storeUnsubscribe)[_storeUnsubscribe]();
this.iteratePlugins(plugin => {
this.removePlugin(plugin);
diff --git a/packages/@uppy/transloadit/lib/index.js b/packages/@uppy/transloadit/lib/index.js
index 863f8d6..1f4cdd4 100644
--- a/packages/@uppy/transloadit/lib/index.js
+++ b/packages/@uppy/transloadit/lib/index.js
@@ -65,12 +65,10 @@ var _onError = _classPrivateFieldLooseKey("onError");
var _onTusError = _classPrivateFieldLooseKey("onTusError");
export default class Transloadit extends BasePlugin {
constructor(uppy, opts) {
- var _this;
super(uppy, {
...defaultOptions,
...opts,
});
- _this = this;
Object.defineProperty(this, _connectAssembly, {
value: _connectAssembly2,
});
@@ -131,22 +129,18 @@ export default class Transloadit extends BasePlugin {
});
Object.defineProperty(this, _onCancelAll, {
writable: true,
- value: async function(_temp) {
- let {
- reason,
- } = _temp === void 0 ? {} : _temp;
+ value: async () => {
try {
- if (reason !== "user") return;
const {
uploadsAssemblies,
- } = _this.getPluginState();
+ } = this.getPluginState();
const assemblyIDs = Object.values(uploadsAssemblies).flat(1);
- const assemblies = assemblyIDs.map(assemblyID => _this.getAssembly(assemblyID));
+ const assemblies = assemblyIDs.map(assemblyID => this.getAssembly(assemblyID));
await Promise.all(
- assemblies.map(assembly => _classPrivateFieldLooseBase(_this, _cancelAssembly)[_cancelAssembly](assembly)),
+ assemblies.map(assembly => _classPrivateFieldLooseBase(this, _cancelAssembly)[_cancelAssembly](assembly)),
);
} catch (err) {
- _this.uppy.log(err);
+ this.uppy.log(err);
}
},
});
@@ -614,16 +608,13 @@ function _createAssembly2(fileIDs, uploadID, assemblyOptions) {
...updatedFiles,
},
});
- const fileRemovedHandler = (fileRemoved, reason) => {
+ const fileRemovedHandler = fileRemoved => {
var _assembly$status;
if (((_assembly$status = assembly.status) == null ? void 0 : _assembly$status.ok) === "ASSEMBLY_COMPLETED") {
this.uppy.off("file-removed", fileRemovedHandler);
return;
}
- if (reason === "cancel-all") {
- assembly.close();
- this.uppy.off("file-removed", fileRemovedHandler);
- } else if (fileRemoved.id in updatedFiles) {
+ if (fileRemoved.id in updatedFiles) {
delete updatedFiles[fileRemoved.id];
const nbOfRemainingFiles = Object.keys(updatedFiles).length;
this.client.updateNumberOfFilesInAssembly(newAssembly, nbOfRemainingFiles).catch(() => {});
diff --git a/packages/@uppy/tus/lib/index.js b/packages/@uppy/tus/lib/index.js
index 22083e0..1d645f2 100644
--- a/packages/@uppy/tus/lib/index.js
+++ b/packages/@uppy/tus/lib/index.js
@@ -178,7 +178,6 @@ export default class Tus extends BasePlugin {
}
}
function _uploadLocalFile2(file) {
- var _this = this;
this.resetUploaderReferences(file.id);
return new Promise((resolve, reject) => {
let queuedRequest;
@@ -374,16 +373,11 @@ function _uploadLocalFile2(file) {
queuedRequest.abort();
upload.abort();
});
- eventManager.onCancelAll(file.id, function(_temp) {
- let {
- reason,
- } = _temp === void 0 ? {} : _temp;
- if (reason === "user") {
- queuedRequest.abort();
- _this.resetUploaderReferences(file.id, {
- abort: !!upload.url,
- });
- }
+ eventManager.onCancelAll(file.id, () => {
+ queuedRequest.abort();
+ this.resetUploaderReferences(file.id, {
+ abort: !!upload.url,
+ });
resolve(`upload ${file.id} was canceled`);
});
eventManager.onResumeAll(file.id, () => {
diff --git a/packages/@uppy/xhr-upload/lib/index.js b/packages/@uppy/xhr-upload/lib/index.js
index 8211d47..11050ce 100644
--- a/packages/@uppy/xhr-upload/lib/index.js
+++ b/packages/@uppy/xhr-upload/lib/index.js
@@ -290,13 +290,8 @@ async function _uploadLocalFile2(file) {
});
});
events.onFileRemove(file.id, () => controller.abort());
- events.onCancelAll(file.id, _ref => {
- let {
- reason,
- } = _ref;
- if (reason === "user") {
- controller.abort();
- }
+ events.onCancelAll(file.id, () => {
+ controller.abort();
});
try {
await uppyFetch().abortOn(controller.signal); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI is failing and there is also custom logic to be removed in companion-client, tus, xhr, and transloadit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about this?
I'm currently investigating this code because this is likely a problem in production for a customer, all that code should be moved to cancel-all
. But I suppose in this PR you can remove the if statement.
uppy/packages/@uppy/transloadit/src/index.ts
Lines 454 to 474 in 53cb32f
if (reason === 'cancel-all') { | |
assembly.close() | |
this.uppy.off('file-removed', fileRemovedHandler) | |
} else if (fileRemoved.id in updatedFiles) { | |
delete updatedFiles[fileRemoved.id] | |
const nbOfRemainingFiles = Object.keys(updatedFiles).length | |
if (nbOfRemainingFiles === 0) { | |
assembly.close() | |
this.#cancelAssembly(newAssembly).catch(() => { | |
/* ignore potential errors */ | |
}) | |
this.uppy.off('file-removed', fileRemovedHandler) | |
} else { | |
this.client | |
.updateNumberOfFilesInAssembly(newAssembly, nbOfRemainingFiles) | |
.catch(() => { | |
/* ignore potential errors */ | |
}) | |
} | |
} | |
} |
I'm not so sure about 087ad5b, I recommend careful review |
// If the assembly has successfully completed, we do not need these checks. | ||
// Otherwise we may cancel an assembly after it already succeeded | ||
if (assembly.status?.ok === 'ASSEMBLY_COMPLETED') { | ||
this.uppy.off('file-removed', fileRemovedHandler) | ||
return | ||
} | ||
if (reason === 'cancel-all') { | ||
assembly.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we remove this line here, then we need to listen for the cancel-all
event and cancel all assemblies. Which is the one and only only use case when an assembly should be cancelled, instead of continuing to run in the background.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is on the 4.x
branch, we can also leave it removed for now and fix this in #5158, where assembly handling is already changing and it would have to be done different there after the refactor.
* 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) ...
We should also update the React example who is still using it.