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

Allow setting method in the client instance #127

Open
gustavnikolaj opened this issue Nov 27, 2020 · 6 comments
Open

Allow setting method in the client instance #127

gustavnikolaj opened this issue Nov 27, 2020 · 6 comments

Comments

@gustavnikolaj
Copy link

This http client looks really nice and approachable. :-)

I love the idea of partially configuring the client ahead of time, and having the ability to e.g. only add the path of the URL at the actual callsite. There's one thing I'd wish it would do differently - I am unable to set the method at the call-site:

const myHttpClient = bent('https://example.com', 'json', 200);

const myGetRequest = await myHttpClient('GET', '/api/entity/42');
const myPostRequest = await myHttpClient('POST', '/api/entity', { "name": "My new Entity" });

I can achieve something like the above, by hacking around it, but it feels less clean :-)

const clientOptions = ['https://example.com', 'json', 200];
const myHttpClient = {
  get: bent(...clientOptions, 'GET'),
  post: bent(...clientOptions, 'POST')
};

const myGetRequest = await myHttpClient.get('/api/entity/42');
const myPostRequest = await myHttpClient.post('/api/entity', { "name": "My new Entity" });

This is just a suggestion - please feel free to close the issue immediately if it isn't something you want to do. I'd love know your reasoning, if there's a good reason why you don't want to - but it's also alright if it's just a matter of taste :-)

@hst-m
Copy link

hst-m commented Nov 29, 2020

I was just looking for this, being allowed to set the Method in the async function, otherwise you need unique functions for each method

@hst-m
Copy link

hst-m commented Nov 30, 2020

I added PR for this, to keep it backward compatible just add the optional Method argument at the end, for example:

const baseRequest = bent('https://example.com', 'json')
baseRequest('/path', { body: test }, { header: 123 }, 'POST')

@aledpardo
Copy link

aledpardo commented Apr 13, 2021

In this case we can leverage bind function method:

const api = bent.bind(null, "url", "json");
const get = api("GET");
const post = api("POST");

This lib is wonderful being as simple as it is. Combined with other JS resources we may not need to add extensions like requestes here, IMHO.

@gustavnikolaj
Copy link
Author

@aledpardo That's just a variation of what I described in the workaround, and it fails to address the actual problem.

@aledpardo
Copy link

aledpardo commented Apr 13, 2021

With a bit more code we can achieve it:

const api = bent.bind(null, 200, "url", "json")
const apiClient = (method, path, body, headers) => api(method)(path, body, headers)

const getRequest = apiClient('GET', '/path?page=1')
const postRequest = apiClient('POST', '/path', { foo: 'bar' })

Very raw idea.

Your requirement is valid and can be achieved easily with some minimal JS features.

But, I am just an enthusiastic bent's user of 😉.

@gustavnikolaj
Copy link
Author

Thank you for the suggestion, and your good intentions. I appreciate it! But with all due respect, I still don't think your solution is addressing the issue at hand.

The current API is clearly not designed for constructing a new instance of the client per request. It's intended that you construct a partially configured instance of bent, which you can then go apply to different use cases. Otherwise, what is the point of having a function return a function...

I don't see adding this functionality as scope-creep and adding useless new features to the project - on the contrary - it's doubling down on the very idea that makes this project neat.

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 a pull request may close this issue.

4 participants
@gustavnikolaj @aledpardo @hst-m and others