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

query parameters #118

Open
jmls opened this issue Aug 22, 2020 · 13 comments
Open

query parameters #118

jmls opened this issue Aug 22, 2020 · 13 comments

Comments

@jmls
Copy link

jmls commented Aug 22, 2020

is there an option to pass a json object in to use as query parameters for a get operation ?

@davehorton
Copy link

I'm also interested to know this

@mikeal
Copy link
Owner

mikeal commented Oct 6, 2020

You just pass them in as part of the URL path.

@Kostanos
Copy link

Would be great to have a query string option.

@hst-m
Copy link

hst-m commented Jan 29, 2021

you can use this:

const objToQuery=obj=>Object.keys(obj).map(key=>`${key}=${obj[key]}`).join('&')

@Kostanos
Copy link

you forgot about escaping :)
for now, I'm using querystring module...
Just mentioned here, because multiple other "request" like modules having it integrated already, and would be great to have bent with it too.

@hst-m
Copy link

hst-m commented Jan 29, 2021

if you need escaping then add it in. the point is it is very easy to do, no point adding it to this library which is minimalist on purpose

@mikeal
Copy link
Owner

mikeal commented Jan 29, 2021

I’ve gone back and forth on this one.

One one hand, i hate having to do write this over and over again, but I also can’t think of a great way to add this without complicating things because we already use the object type to represent headers.

When I first wrote this library URL and URLSearchParams was not available in Node.js and now it’s available in all supported versions, so there’s probably something that we can do now that we couldn’t in the past.

@mikeal
Copy link
Owner

mikeal commented Jan 29, 2021

What do people think about this:

  1. We allow you to pass an instance of URLSearchParams that will be appended to the URL (after concatenation if you’re using that feature of bent).
  2. We add bent.qs({ name: value }) that does proper string escaping and returns an instace of URLSearchParams.
  3. Now you can do:
const get = bent(url)
await get(/path’, bent.qs({ name: value }))

We can even allow them to be used combinatorially, so you can set default values.

const get = bent(url, bent.qs({ name: default })
await get(/path’, bent.qs({ name: value }))

Or if you really want to make it difficult for us:

const get = bent(url, bent.qs({ name: default })
await get(/path?name=valueThatWillBeOverwritten’, bent.qs({ name: value }))

We can even force a key ordering when we produce the final URL so that we add some determinism to the queryParam generation that isn’t there by default.

@hst-m
Copy link

hst-m commented Jan 29, 2021

I was not aware of this URLSearchParams before, so looks like you can just do:

new URLSearchParams(obj).toString()

I think keep it simple and let the user do that themselves, is easy enough. A higher priority would be allowing setting the http method in the bent function #127

@hst-m
Copy link

hst-m commented Jan 29, 2021

I think the bent.qs(queryObj) stuff could be a helpful shortcut to clean up the code. The bent async function parameters are in order so not sure how you add that in like your example, need to update the param parsing

await get('/path', bent.qs({ name: value }))

@mikeal
Copy link
Owner

mikeal commented Jan 30, 2021

It’s a little late to go changing the function signature.

I’m actually quite fond of using destructuring this way, i do it a lot in other modules, but it’s a little verbose for bent because the simple usage is already so few characters.

@hst-m
Copy link

hst-m commented Jan 30, 2021

I think the best thing to do is use bent.body() for the body parameter and bent.query() for query, everything else is auto-detected (url, method, response type, status codes, headers), this allows both functions to have the same signature with all options available and no parameter order requirements

const baseRequest = bent('https://example.com', 'json', {header: value})

await baseRequest('GET', '/path', bent.query(query), {header: value})
await baseRequest('POST', '/path', bent.body(body), {header: value})

@bugs181
Copy link

bugs181 commented Mar 13, 2021

What do people think about this:

  1. We allow you to pass an instance of URLSearchParams that will be appended to the URL (after concatenation if you’re using that feature of bent).

I vote for this one. Currently, I'm using bent like this:

const getJSON = bent('http://XXX/notes?token=' + authToken, 'GET', 'json', 200)

// Later
const queryParams = new URLSearchParams({
  page: pageNum,
}).toString()

const response = await getJSON('&' + queryParams)

Assuming I'm thinking about this correctly, I could dramatically simplify the above code without all of the boiler plate.

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

6 participants