elbywan / wretch

A tiny wrapper built around fetch with an intuitive syntax. :candy:
MIT License
4.79k stars 96 forks source link

Roadmap to 1.0 #5

Closed chimon2000 closed 6 years ago

chimon2000 commented 6 years ago

This is my new favorite HTTP client.

Wondering if there's any Roadmap of features to implement before wretch reaches 1.0

elbywan commented 6 years ago

This is my new favorite HTTP client.

Extremely glad to read this 😄 !

Wondering if there's any Roadmap of features to implement before wretch reaches 1.0

Well, not really, personally I'm satisfied with the current API and functionalities.

I'm mostly waiting for user feedback, ideas and bug reports before releasing 1.0. If the library is stable enough (no bug reports, no features asked), then I'll publish.

So if anyone wants anything, now is the time to ask !

The constraints are :

Pending commits are on the dev branch.

bb010g commented 6 years ago

Using qs for application/x-www-form-urlencoded support under a form(…) body type would be handy. I'll see if I can get a PR in soon.

I've also had some issues with the type definitions and how Jest looks for dependencies, but I'll make a separate issue for that.

elbywan commented 6 years ago

Thanks for the suggestions, I will check this out tomorrow, I need some sleep right now :)

elbywan commented 6 years ago

@bb010g About qs, are you sure it's really important to include it in wretch ?

If I read the docs right, we can use it easily with the current syntax :

import wretch from "wretch"
import * as qs from "qs"

const w = wretch().content("application/x-www-form-urlencoded")
w.url("myurl").body(qs.stringify({ a: { b: 'c' } }).post()

Plus I would like to keep wretch ultra small, meaning no dependencies (or as few as possible).

What I could do is add a formUrl method to ease up a little bit :

// sets body + content header
wretch("myurl").formUrl(qs.stringify({ a: { b: 'c' } }).post()
tchak commented 6 years ago

Now that cancelable fetch is a thing, it would be great to have support for it in wretch. (I know most polyfills do not support it yet :(). It also mean it will be possible to add .timeout() to the factory object.

elbywan commented 6 years ago

@tchak Yes it's definitely number 1 on my todolist 😉 .

But as you said, current support sucks.

abenhamdine commented 6 years ago

First, congrats for this tiny module ! 💃 I started using it yesterday, and I'm satisfied with it so far. Maybe it could sounds a bit chauvinistic, but I'm glad to see some french developers creating nice modules like that one.

For 1.0, I would suggest you to clarify some points in the docs. While the current docs are pretty clear, it's very important for newcomers to be able to avoid common pitfalls when using a new library.

I think about the following points (probably others) :

I didn't use much this librabry so probably there's some other points which could help newcomers.

elbywan commented 6 years ago

Maybe it could sounds a bit chauvinistic, but I'm glad to see some french developers creating nice modules like that one.

@abenhamdine Merci beaucoup !! 😄

difference between wretch().baseUrl("http://example.com") and let w = wretch("http://example.com") for subsequents calls ? (I suppose no difference)

There is, baseUrl prefixes all calls while wretch("...") and url both replace the full url.

const w = wretch().baseUrl("http://example.com/app")
w("/test").get()   // calls "http://example.com/app/test"
w("/test2").get()  // calls "http://example.com/app/test2"

const w2 = wretch("http://example.com/app")
w2.url("/test").get() //calls "/test", not "http://example.com/app/test"

I'm tempted to simply remove baseUrl and just add a true/false flag to .url to set or append.

While the current docs are pretty clear, it's very important for newcomers to be able to avoid common pitfalls when using a new library.

You're absolutely right, I will try to make things clearer in the docs soon by adding more use cases and code samples.

Thanks for your suggestions !

bb010g commented 6 years ago

What I could do is add a formUrl method to ease up a little bit :

LGTM. It just seems like a common thing that shouldn't need a whole separate content call to work.

Also, :+1: on the mutating/factory methods. I've had to check the source a few times to see how I should be storing a certain config set.

I'd be against removing baseUrl. I think that's more of a docs case, and it's how fetch operates. I think this ties into the mutation/factory problem. Some form of saying that wretch is a factory, calling it with a URL and options mutates a new Wretcher object, and that you're mutating until you call options and get a new factory.

elbywan commented 6 years ago

@bb010g

LGTM. It just seems like a common thing that shouldn't need a whole separate content call to work.

Allright then, let's get down to business :wink:

I'd be against removing baseUrl.

Could you check issue #8 (and the associated commit) ? I made some proposals (and implemented the one I liked more).

I really think that this syntax is not only better, but it also allows stuff (like chaining) that wasn't possible previously with baseUrl while preserving the concept.

bb010g commented 6 years ago

Those both look great. Thanks!

abenhamdine commented 6 years ago

I've just seen the recent improvements on the readme, and it's now much better ! Very useful. 👍

elbywan commented 6 years ago

1.0.0 is out !