-
Notifications
You must be signed in to change notification settings - Fork 2k
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: [email protected] (#5194) Release: [email protected] (#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: [email protected] (#5189) examples: add SvelteKit example (#5181) @uppy/companion: fix missing `await` Release: [email protected] (#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) ...
| Package | Version | Package | Version | | ---------------------- | ------------- | ---------------------- | ------------- | | @uppy/angular | 0.7.0-beta.5 | @uppy/instagram | 4.0.0-beta.6 | | @uppy/audio | 2.0.0-beta.6 | @uppy/locales | 4.0.0-beta.2 | | @uppy/aws-s3 | 4.0.0-beta.5 | @uppy/onedrive | 4.0.0-beta.6 | | @uppy/aws-s3-multipart | 4.0.0-beta.6 | @uppy/provider-views | 4.0.0-beta.7 | | @uppy/box | 3.0.0-beta.6 | @uppy/status-bar | 4.0.0-beta.9 | | @uppy/companion | 5.0.0-beta.9 | @uppy/transloadit | 4.0.0-beta.7 | | @uppy/companion-client | 4.0.0-beta.7 | @uppy/tus | 4.0.0-beta.6 | | @uppy/core | 4.0.0-beta.9 | @uppy/unsplash | 4.0.0-beta.7 | | @uppy/dashboard | 4.0.0-beta.9 | @uppy/url | 4.0.0-beta.7 | | @uppy/drop-target | 3.0.0-beta.5 | @uppy/utils | 6.0.0-beta.8 | | @uppy/dropbox | 4.0.0-beta.7 | @uppy/webcam | 4.0.0-beta.8 | | @uppy/facebook | 4.0.0-beta.6 | @uppy/xhr-upload | 4.0.0-beta.6 | | @uppy/form | 4.0.0-beta.4 | @uppy/zoom | 3.0.0-beta.6 | | @uppy/golden-retriever | 4.0.0-beta.5 | uppy | 4.0.0-beta.10 | | @uppy/google-drive | 4.0.0-beta.6 | | | - @uppy/audio: remove unused component props (Antoine du Hamel / #5209) - @uppy/angular: fix invalid char in `package.json` (Antoine du Hamel / #5224) - meta: use default argument value instead of `defaultProps` (Antoine du Hamel / #5222) - @uppy/angular: upgrade to Angular 18 (Antoine du Hamel / #5215) - @uppy/utils: remove unused `settle` (Antoine du Hamel / #5210) - @uppy/form: move internal property to private field (Antoine du Hamel / #5214) - @uppy/dashboard: remove unused component props (Antoine du Hamel / #5213) - @uppy/status-bar: remove unused component props (Antoine du Hamel / #5211) - @uppy/audio: move internal property to private field (Antoine du Hamel / #5207) - @uppy/aws-s3: remove todo (Mikael Finstad / #5200) - @uppy/core: remove unnecessary todo (Mikael Finstad / #5200) - @uppy/aws-s3: do not expose internal `assertHost` method (Mikael Finstad / #5200) - @uppy/aws-s3: make passing `signal` consistent (Mikael Finstad / #5200) - @uppy/core: remove `'upload-started'` event (Mikael Finstad / #5200) - @uppy/aws-s3: remove `chunkState` getter (Mikael Finstad / #5200) - @uppy/drop-target: remove `title` property (Mikael Finstad / #5200) - @uppy/golden-retriever: remove unused `ready` setters (Mikael Finstad / #5200) - @uppy/dashboard: remove deprecated `autoOpenFileEditor` option (Mikael Finstad / #5200) - @uppy/aws-s3: remove `uploaderSockets` (Mikael Finstad / #5200) - @uppy/locales: remove hacks for legacy bundle (Mikael Finstad / #5200) - @uppy/status-bar: rename `StatusBar` to `StatusBarUI` (Mikael Finstad / #5200) - @uppy/url: remove unused error handler (Mikael Finstad / #5200) - @uppy/aws-s3,@uppy/tus,@uppy/utils,@uppy/xhr-upload: remove `uploader` from `upload-progress` event (Mikael Finstad / #5200) - @uppy/webcam: remove `facingMode` option (Mikael Finstad / #5200) - @uppy/companion: invert some internal boolean options (Mikael Finstad / #5198) - @uppy/companion: rename `authProvider` to `oauthProvider` (Mikael Finstad / #5198) - @uppy/companion: remove unused headers (Mikael Finstad / #5198) - @uppy/companion: remove sanitizing of metadata (Mikael Finstad / #5198) - @uppy/companion-client: do not allow boolean `RequestOptions` (Mikael Finstad / #5198) - @uppy/companion-client: remove deprecated options (Mikael Finstad / #5198) - @uppy/companion: remove `error.extraData` (Mikael Finstad / #5198) - @uppy/companion-client: make `supportsRefreshToken` default (Mikael Finstad / #5198) - @uppy/companion-client: remove optional chaining (Mikael Finstad / #5198) - @uppy/companion: capitalize POST (Mikael Finstad / #5198) - @uppy/companion: simplify code by using modern Node.js APIs (Mikael Finstad / #5198) - @uppy/companion-client: remove `Socket` (Mikael Finstad / #5198) - @uppy/companion: rename `getExtraConfig` to `getExtraGrantConfig` (Mikael Finstad / #5198) - @uppy/companion: change `COMPANION_ENABLE_URL_ENDPOINT` default (Mikael Finstad / #5198) - @uppy/companion: change default value for Redis session prefix (Mikael Finstad / #5198) - examples: make React example up-to-date (Merlijn Vos / #5205) - @uppy/core: add type tests (Merlijn Vos / #5153) - @uppy/provider-views: PartialTree - get rid of `.onFirstRender()` (Evgenia Karunus / #5187) - @uppy/core: pass file to events consistently (Merlijn Vos / #5136) - docs: assume tree-shaking bundler is the most common case (Antoine du Hamel / #5160) - @uppy/core: remove `reason` (Antoine du Hamel / #5159) - @uppy/core: remove `resetProgress` and `reset-progress` (Mikael Finstad / #5221)
Since reason is now removed in 4.0, how do we delineate between the user removing a file and a cancel-all or .destroy triggering file-removed? We are only emitting delete events to the server when the file-removed reason was 'removed-by-user' which is causing us issues with the 4.0 upgrade. |
You should never need to call |
@Murderlon Angular. We were calling .close() in ngOnDestroy which moved to .destroy() in Uppy 4.0.
Is the recommendation to remove this? |
We should also update the React example who is still using it.