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

File Upload fails due to added Content-Type header #213

Closed
Dogfalo opened this issue Mar 20, 2020 · 10 comments
Closed

File Upload fails due to added Content-Type header #213

Dogfalo opened this issue Mar 20, 2020 · 10 comments

Comments

@Dogfalo
Copy link

Dogfalo commented Mar 20, 2020

Describe the bug
File Upload fails due to Content-Type header being automatically added when multipart/form-data is expected

Fix: Remove the default content-type for POST and PUT that is added here https://github.com/alex-cory/use-http/blob/master/src/doFetchArgs.ts#L60

see here for more information: JakeChampion/fetch#505 (comment)

@iamthesiz
Copy link
Collaborator

will take a look.

@iamthesiz
Copy link
Collaborator

iamthesiz commented Mar 21, 2020

As a workaround in the meantime, you can remove that header like

const { ... } = useFetch(opts => {
  delete opts.headers['Content-Type']
  return opts
})

@iamthesiz
Copy link
Collaborator

I'm trying to decide what is more useful to more use cases. Adding the Content-Type: application/json header or not. I see axios has 'Content-Type': 'application/x-www-form-urlencoded' as the default.

I'm down to remove this, just want to make sure it's the best decision.

@iamthesiz
Copy link
Collaborator

This should now work as of v0.4.4. If the issue persists, let me know and I'll reopen the issue.

@sbichenko
Copy link
Contributor

This doesn't seem to work, at least in React Native. It seems that the isObject test evaluates to true when passed FormData. This is because React Native relies on an implementation of FormData that doesn't have toStringTag set (just like this one). As a result, the isBodyObject check in doFetchArgs evaluates to true, and FormData is stringified, and then incorrect Content-Type is added.

@iamthesiz iamthesiz reopened this Apr 18, 2020
@iamthesiz
Copy link
Collaborator

iamthesiz commented Apr 18, 2020

@sbichenko Are you able to work around this for now? I can fix this unless you want to submit a PR.

@sbichenko
Copy link
Contributor

I don't think I can work around this. The workaround from your reply at the top doesn't work for me (haven't figured out why yet).

I have a branch ready with a simple fix that I've tested manually, but

  1. When trying to push to the repo I get "fatal: Could not read from remote repository." -- do I need some additional permissions or is this something on my side?
  2. I did not add a test for this case because I'm not sure how to go about adding a React Native test to the test suite.

@iamthesiz
Copy link
Collaborator

iamthesiz commented Apr 19, 2020

If you need advice on contributing to useFetch, here's some commands to help.
You need to:

  1. Fork for the repo (click the fork button)
    image

  2. Environment Setup clone your fork and set up your environment for development

In terminal window 1

git clone git@github.com:YOUR_USERNAME/use-http.git
cd ./use-http
yarn
yarn link

In terminal window 2 (your react app to test use-http in)

create-react-app use-http-sandbox
cd ./use-http-sandbox
yarn
yarn link use-http

In terminal window 1 (inside your forked use-http directory)

npm link ../use-http-sandbox/node_modules/react
npm link ../use-http-sandbox/node_modules/react-dom
yarn build:watch

In terminal window 2 (your react app to test use-http in)

yarn start
  1. Develop Now just go into your use-http-sandbox/src/App.js and import use-http and now you can develop. When you make changes in use-http it should cause use-http-sandbox to refresh localhost:3000.

  2. Test once you're done making your changes be sure to make some + run all the tests. What I do is open up 3 different panes in the same iTerm2 window by pressing ⌘ + D on mac 2 times. In the far left I do yarn build:watch, in the middle I do yarn test:browser:watch (where you'll probably be writing your tests) and in the 3rd window I do yarn test:server:watch. It looks like this.
    image

  3. Push push your changes to your forked repo.

  4. Make PR Once you do this, you will see a link in your terminal that looks like this.

remote: Create a pull request for 'master' on GitHub by visiting:
remote:      https://github.com/YOUR_USERNAME/use-http/pull/new/master

go to that url. From there you should be able to compare your forked master branch to ava/use-http:master.

For your second issue, that's okay. Just add it to the todos in the readme and I'll get to it. I need to set that up anyway.

@sbichenko
Copy link
Contributor

Thanks, that's great! I think it would be worth adding the instructions to the contributor guidelines. Here's the PR: #249

@iamthesiz
Copy link
Collaborator

@sbichenko added a contributions.md

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

No branches or pull requests

3 participants