-
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/xhr-upload: pass files to onBeforeRequest #5447
Conversation
Diff output filesdiff --git a/packages/@uppy/xhr-upload/lib/index.js b/packages/@uppy/xhr-upload/lib/index.js
index a5737d1..0f8848c 100644
--- a/packages/@uppy/xhr-upload/lib/index.js
+++ b/packages/@uppy/xhr-upload/lib/index.js
@@ -133,10 +133,15 @@ export default class XHRUpload extends BasePlugin {
_classPrivateFieldLooseBase(this, _getFetcher)[_getFetcher] = files => {
return async (url, options) => {
try {
- var _this$opts$getRespons, _this$opts, _body2;
+ var _this$opts$getRespons, _this$opts2, _body2;
const res = await fetcher(url, {
...options,
- onBeforeRequest: this.opts.onBeforeRequest,
+ onBeforeRequest: (xhr, retryCount) => {
+ var _this$opts$onBeforeRe, _this$opts;
+ return (_this$opts$onBeforeRe = (_this$opts = this.opts).onBeforeRequest) == null
+ ? void 0
+ : _this$opts$onBeforeRe.call(_this$opts, xhr, retryCount, files);
+ },
shouldRetry: this.opts.shouldRetry,
onAfterResponse: this.opts.onAfterResponse,
onTimeout: timeout => {
@@ -161,9 +166,9 @@ export default class XHRUpload extends BasePlugin {
}
},
});
- let body = await ((_this$opts$getRespons = (_this$opts = this.opts).getResponseData) == null
+ let body = await ((_this$opts$getRespons = (_this$opts2 = this.opts).getResponseData) == null
? void 0
- : _this$opts$getRespons.call(_this$opts, res));
+ : _this$opts$getRespons.call(_this$opts2, res));
try {
var _body;
(_body = body) != null ? _body : body = JSON.parse(res.responseText); |
@@ -194,13 +199,16 @@ export default class XHRUpload< | |||
*/ | |||
this.#getFetcher = (files: UppyFile<M, B>[]) => { | |||
return async ( | |||
url: Parameters<typeof fetcher>[0], | |||
options: NonNullable<Parameters<typeof fetcher>[1]>, | |||
url: string, |
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.
Why this change?
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.
Because in this plugin we expect url
to be a string, regardless of what fetcher supports so it's better to match it to the plugin code. fetcher
also expects a string
and should it ever change in fetcher
we still get a type error.
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.
Where? AFAICT url
is only passed to fetcher
, so it only matters what fetcher
supports IMO
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.
No, it only matters what this type supports:
endpoint: string |
So it's either string
or Opts<M, B>['endpoint']
. But it's such I minor thing I wouldn't overthink it.
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.
I guess it only matters if fetcher
has a change in its signature where it no longer supports string
, but we'd get a type error anyway.
| Package | Version | Package | Version | | ----------------- | ------- | ----------------- | ------- | | @uppy/companion | 5.1.1 | @uppy/tus | 4.1.1 | | @uppy/svelte | 4.0.2 | @uppy/xhr-upload | 4.2.0 | | @uppy/transloadit | 4.1.1 | uppy | 4.4.0 | - @uppy/tus: fix retry check for status code 400 (Merlijn Vos / #5461) - meta: Merge branch 'main' of https://github.com/transloadit/uppy (Murderlon) - meta: fix AwsS3 endpoint option in private/dev (Murderlon) - examples: build(deps): bump body-parser from 1.20.2 to 1.20.3 (dependabot[bot] / #5462) - examples: build(deps-dev): bump vite from 5.3.1 to 5.3.6 (dependabot[bot] / #5459) - @uppy/tus: set response from tus-js-client (Merlijn Vos / #5456) - docs: fix assemblyOptions example for React (Merlijn Vos / #5450) - docs: rename Edgly to Smart CDN (Merlijn Vos / #5449) - @uppy/tus: correctly type tus on UppyFile (Merlijn Vos / #5454) - docs: remove old legacy CDN reference (Murderlon) - @uppy/xhr-upload: pass files to onBeforeRequest (Merlijn Vos / #5447) - @uppy/svelte: fix generated module to not bundle Svelte (Antoine du Hamel / #5446) - examples,@uppy/svelte: Bump svelte from 4.2.18 to 4.2.19 (dependabot[bot] / #5440) - meta: bump Yarn to 4.4.1 (Antoine du Hamel / #5445) - docs: fix broken links in locale docs (Serghei Cebotari / #5441)
* main: (319 commits) build(deps): bump docker/build-push-action from 6.8.0 to 6.9.0 (#5483) Release: [email protected] (#5479) @uppy/transloadit: fix multiple upload batches & run again (#5478) build(deps): bump docker/build-push-action from 6.7.0 to 6.8.0 (#5477) build(deps): bump vite from 5.2.11 to 5.4.8 (#5471) build(deps-dev): bump rollup from 4.18.0 to 4.22.4 (#5470) build(deps): bump vite from 5.2.11 to 5.4.6 (#5466) Release: [email protected] (#5467) @uppy/tus: fix retry check for status code 400 (#5461) meta: fix AwsS3 endpoint option in private/dev build(deps): bump body-parser from 1.20.2 to 1.20.3 (#5462) build(deps-dev): bump vite from 5.3.1 to 5.3.6 (#5459) @uppy/tus: set response from tus-js-client (#5456) docs: fix assemblyOptions example for React (#5450) docs: rename Edgly to Smart CDN (#5449) @uppy/tus: correctly type tus on UppyFile (#5454) docs: remove old legacy CDN reference @uppy/xhr-upload: pass files to onBeforeRequest (#5447) @uppy/svelte: fix generated module to not bundle Svelte (#5446) Bump svelte from 4.2.18 to 4.2.19 (#5440) ...
Ref: https://community.transloadit.com/t/how-to-access-the-file-or-file-name-in-xhr-plugin-onbeforerequest/
We also pass the file
onBeforeRequest
for@uppy/tus
.