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

misleading or missing documentation, must JSON.stringify body #23

Closed
davekaro opened this issue Jun 15, 2021 · 3 comments
Closed

misleading or missing documentation, must JSON.stringify body #23

davekaro opened this issue Jun 15, 2021 · 3 comments
Assignees

Comments

@davekaro
Copy link

I tried to follow the doc example to patch to my rails server:

const response = await patch('localhost:3000/my_endpoint', { body: { name: 'Request.JS' }})

but noticed that [object Object] was being sent as the body. Looking at samples on https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch, I updated my code to stringify the JSON payload first.

const response = await patch('localhost:3000/my_endpoint', { body: JSON.stringify({ name: 'Request.JS' })})

Is this expected or did I miss something? Should the library do this for us? Maybe when the detected content type is application/json (

return 'application/json'
) it should stringify the body?

@davekaro davekaro changed the title misleading or missing documentation misleading or missing documentation, must JSON.stringify body Jun 15, 2021
@marcelolx
Copy link
Collaborator

@davekaro Well, I think the docs are wrong. I didn't pay attention to this detail when I wrote the docs, will fix them soon as possible.

About the idea of request.js doing this, I'm not sure if we should or not do it, but maybe if we detect the content-type is application/json and the body is a js object we stringify the body, if it is already a string we do nothing. 🤔 Anyway, I'll fix the docs while I think in this possibility

@marcelolx marcelolx self-assigned this Jun 15, 2021
@davekaro
Copy link
Author

Thanks! Yeah I can see if this is a lightweight wrapper for fetch, it would be better to keep it that way and expect folks to rely on how fetch works.

marcelolx added a commit that referenced this issue Jun 15, 2021
As pointed out on #23 fetch requires to stringify the body of the request when the Content-Type is application/json, but actualy we don't do this automaticly and in the docs we were suggesting that we doo which is confusing.
@marcelolx
Copy link
Collaborator

marcelolx commented Jun 15, 2021

@davekaro I've fixed the docs. I agree with your point but I think that if we can provide the possibility to the user send the body of the request in both ways and document this, it can be be useful. At least I think that it is a bit anoying to need to stringify the body every time I'll do such kind of request.

I'll close the issue, since the docs match the behavior but I will work on supporting this functionality and then we can see what the other folks think about

marcelolx added a commit that referenced this issue Jun 18, 2021
References #23 

This allows us the make the request sending a javascript object and then RequestJS does the job of stringify the body.
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

2 participants