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/xhr-upload: pass files to onBeforeRequest #5447

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

Murderlon
Copy link
Member

@Murderlon Murderlon commented Sep 3, 2024

@Murderlon Murderlon self-assigned this Sep 3, 2024
Copy link
Contributor

github-actions bot commented Sep 3, 2024

Diff output files
diff --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,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Member Author

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.

Copy link
Contributor

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

Copy link
Member Author

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:

So it's either string or Opts<M, B>['endpoint']. But it's such I minor thing I wouldn't overthink it.

Copy link
Contributor

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.

@Murderlon Murderlon merged commit f331622 into main Sep 5, 2024
16 of 17 checks passed
@Murderlon Murderlon deleted the xhr-onbeforerequest-files branch September 5, 2024 10:44
@github-actions github-actions bot mentioned this pull request Sep 20, 2024
github-actions bot added a commit that referenced this pull request Sep 20, 2024
| 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)
Murderlon added a commit that referenced this pull request Oct 7, 2024
* main: (319 commits)
  build(deps): bump docker/build-push-action from 6.8.0 to 6.9.0 (#5483)
  Release: uppy@4.4.1 (#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: uppy@4.4.0 (#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)
  ...
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