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: replace ev.target.status with xhr.status #3782

Merged
merged 3 commits into from
Jun 7, 2022
Merged

@uppy/xhr-upload: replace ev.target.status with xhr.status #3782

merged 3 commits into from
Jun 7, 2022

Conversation

weston-sankey-mark43
Copy link
Contributor

This resolves an issue when mocking XHR uploads using MSW.

@Murderlon
Copy link
Member

Hi, any reason why this is still WIP?

@Murderlon Murderlon requested a review from arturi May 25, 2022 09:12
@weston-sankey-mark43 weston-sankey-mark43 marked this pull request as ready for review May 25, 2022 11:38
@aduh95 aduh95 changed the title Replace instances of ev.target.status with xhr.status @uppy/xhr-upload: replace ev.target.status with xhr.status May 25, 2022
@aduh95
Copy link
Contributor

aduh95 commented May 25, 2022

Is it correct that this PR would fix #3717?

@weston-sankey-mark43
Copy link
Contributor Author

Is it correct that this PR would fix #3717?

From the Uppy side, yes. There's another required change in MSW for which I've submitted a PR.

Copy link
Contributor

@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.

We probably also need to update this line:

status: ev.target.status,

@Murderlon Murderlon linked an issue May 25, 2022 that may be closed by this pull request
@aduh95 aduh95 added the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label May 26, 2022
@github-actions github-actions bot removed pending end-to-end tests safe to test Add this label on trustworthy PRs to spawn the e2e test suite labels May 26, 2022
@Murderlon
Copy link
Member

Seems we still have an eslint error

@aduh95 aduh95 added the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label May 31, 2022
@github-actions github-actions bot removed the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label May 31, 2022
@aduh95 aduh95 added the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label May 31, 2022
@github-actions github-actions bot removed pending end-to-end tests safe to test Add this label on trustworthy PRs to spawn the e2e test suite labels May 31, 2022
@Murderlon
Copy link
Member

Murderlon commented May 31, 2022

Would be nice if we had an e2e test for xhr-upload so we can be more confident about changes like this. Not sure if blocking for this PR, @aduh95?

@aduh95
Copy link
Contributor

aduh95 commented Jun 7, 2022

Would be nice if we had an e2e test for xhr-upload so we can be more confident about changes like this. Not sure if blocking for this PR, @aduh95?

I agree increasing e2e coverage would be 💯, but it should not block this PR from landing imho.

@aduh95 aduh95 merged commit aa1ecd4 into transloadit:main Jun 7, 2022
@aduh95
Copy link
Contributor

aduh95 commented Jun 7, 2022

Thanks a lot for the contribution @weston-sankey-mark43, I'll make sure this is included in the next release.

github-actions bot added a commit that referenced this pull request Jun 7, 2022
| Package                | Version | Package                | Version |
| ---------------------- | ------- | ---------------------- | ------- |
| @uppy/aws-s3           |   2.2.1 | @uppy/tus              |   2.4.1 |
| @uppy/aws-s3-multipart |   2.4.1 | @uppy/url              |   2.2.0 |
| @uppy/companion-client |   2.2.1 | @uppy/xhr-upload       |   2.1.2 |
| @uppy/core             |   2.3.1 | @uppy/robodog          |   2.8.0 |
| @uppy/react            |   2.2.2 | uppy                   |  2.12.0 |
| @uppy/remote-sources   |   0.1.0 |                        |         |

- @uppy/remote-sources: Add @uppy/remote-sources preset/plugin (Artur Paikin / #3676)
- @uppy/react: Reset uppy instance when React component is unmounted (Tomasz Pęksa / #3814)
- @uppy/aws-s3-multipart,@uppy/aws-s3,@uppy/tus: queue socket token requests for remote files (Merlijn Vos / #3797)
- @uppy/xhr-upload: replace `ev.target.status` with `xhr.status` (Wes Sankey / #3782)
- @uppy/core: fix `TypeError` when file was deleted (Antoine du Hamel / #3811)
- @uppy/robodog: fix linter warnings (Antoine du Hamel / #3808)
- meta: fix GHA workflow for prereleases (Antoine du Hamel)
- @uppy/aws-s3-multipart: allow `companionHeaders` to be modified with `setOptions` (Paulo Lemos Neto / #3770)
- @uppy/url: enable passing optional meta data to `addFile` (Brad Edelman / #3788)
- @uppy/url: fix `getFileNameFromUrl` (Brad Edelman / #3804)
- @uppy/tus: make onShouldRetry type optional (Merlijn Vos / #3800)
- doc: fix React examples (Antoine du Hamel / #3799)
- meta: add GHA workflow for prereleases (Antoine du Hamel)
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.

How to use @uppy/xhr with MSW mock server?
3 participants