lostisland / faraday

Simple, but flexible HTTP client library, with support for multiple backends.
https://lostisland.github.io/faraday
MIT License
5.73k stars 976 forks source link

docs: update `body` param type for `run_request` #1545

Closed G-Rath closed 8 months ago

G-Rath commented 9 months ago

Description

It looks like this was already picked up in the past (via #601) but the docs have since been overhauled and this was lost in the transition.

I've used Object as my understanding is really body doesn't care what it's passed so long as it can be turned into a string or JSON, and then beyond supporting calling methods like to_json, that's up the object being passed in to handle.

Todos

List any remaining work that needs to be done, i.e:

olleolleolle commented 9 months ago

Another way to communicate the parameter type is to offer which method it ought to support. That's an alternative. If we do use Object, we may extend the description string in the docblock, saying what the Object may be, what is required of it.

G-Rath commented 9 months ago

I'm not sure how useful or possible that would be since it's really up to the middleware right? When I originally opened this I realised shortly after this that this method isn't quite what I was wanting to use and that the JSON handling is actually done by a middleware rather than the core which I think means this change is less important (though not incorrect) because it means it's not really something the core can document 🤷

iMacTia commented 8 months ago

which I think means this change is less important (though not incorrect) because it means it's not really something the core can document

That's a fair point, I can totally see the confusion here. The current String is clearly too limited, at the same time though I agree with @olleolleolle that Object is too broad and hardly adds any value to the documentation.

So here is my proposal: let's use [String, Hash, Array, nil] and improve the doc comment a bit to point out that this is based on the default configuration, and that using different middleware will potentially allow different types to be supported.

This is because by default Faraday uses the url_encoded middleware, that supports all those types:

# The following all run a request with body: "a=1"
Faraday.post('http://google.com', 'a=1')
Faraday.post('http://google.com', {a: 1})
Faraday.post('http://google.com', [[:a,1]])