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

Add PATCH to valid XHR types #5279

Closed

Conversation

pedantic-git
Copy link
Contributor

Thanks for fixing #5165 a couple of months back! I'm in the process of migrating my codebase to TS and this has hit me again because PATCH isn't in the list of valid methods.

PATCH is commonly used in Rails backends for issuing updates to existing records, so I think this would benefit more than just me. I know it works well because I've been using it for a long time in untyped JS.

Let me know if you need any more info!

@@ -15,7 +15,7 @@ export interface XHRUploadOptions extends PluginOptions {
timeout?: number
responseUrlFieldName?: string
endpoint: string
method?: 'GET' | 'POST' | 'PUT' | 'HEAD' | 'get' | 'post' | 'put' | 'head'
method?: 'GET' | 'POST' | 'PUT' | 'HEAD' | 'PATCH' | 'get' | 'post' | 'put' | 'head' | 'patch'
Copy link
Contributor

@aduh95 aduh95 Jun 27, 2024

Choose a reason for hiding this comment

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

IMO we should defined all the methods listed in https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods, plus all the lowercase version of the standard ones, minus the ones that would send a SecurityError. As discussed in the previous issue, lowercase patch is not valid, and only supported on 3.x for backward-compatibility.

Suggested change
method?: 'GET' | 'POST' | 'PUT' | 'HEAD' | 'PATCH' | 'get' | 'post' | 'put' | 'head' | 'patch'
method?:
| 'GET'
| 'HEAD'
| 'POST'
| 'PUT'
| 'DELETE'
| 'OPTIONS'
| 'PATCH'
| 'delete'
| 'get'
| 'head'
| 'options'
| 'post'
| 'put'
| string

Could you also update the type in the source file please? Currently it is lacking quite a bunch of valid types:

method?: 'post' | 'put'

@aduh95
Copy link
Contributor

aduh95 commented Jun 27, 2024

Landed in 46fd4d7

@aduh95 aduh95 closed this Jun 27, 2024
@pedantic-git
Copy link
Contributor Author

Thank you for doing this on my behalf!

@github-actions github-actions bot mentioned this pull request Jun 27, 2024
github-actions bot added a commit that referenced this pull request Jun 27, 2024
| Package          | Version | Package          | Version |
| ---------------- | ------- | ---------------- | ------- |
| @uppy/dashboard  |   3.9.1 | uppy             |  3.27.1 |
| @uppy/xhr-upload |   3.6.8 |                  |         |

- @uppy/xhr-upload: add `'PATCH'` as valid method (Quinn Daley / #5279)
- @uppy/dashboard: fix handling of `null` for `doneButtonHandler` (Antoine du Hamel / #5283)
- meta: Bump docker/build-push-action from 5.4.0 to 6.1.0 (dependabot[bot] / #5272)
- docs: rewrite Instagram dev setup section (Evgenia Karunus / #5274)
- meta: remove the Zoom section from `CONTRIBUTING.md` (Evgenia Karunus / #5273)
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