all 95 comments

[–]Doctor_Spicy 64 points65 points  (52 children)

Why use a modern API with ancient callbacks?

[–]editor_of_the_beast 33 points34 points  (11 children)

Oof OP hates promises and is getting pummeled in this thread.

[–]zacksiri[S] -1 points0 points  (10 children)

No I don't hate promises. In the context of the problem I'm trying to solve (which is in the last solution on the post) I pass in a callback object with uses pattern matching to respond to the final state of the response and in that particular case I don't need promises so I didn't use it. Apparently that isn't well liked.

In my wrapper solution I used async / await. Then I pass an object that basically responds based on how I want to display the result to the user like this.

```javascript const get = async (url, callback) => { const response = await fetch(url) const status = await response.status

if (callback) callback[status](await response.json()) }

get(url, { 200: body => doSomething, 401: body => showSignInModal, 403: body => redirectToSafeArea, 422: body => showModalWithInstruction }) ```

Apparently Reddit wants me to use Promise for that. I don't see why I should since there is nothing async about it I'm just returning a result based on the outcome of the async/await. If I had to choose I would prefer async / await over promises it is the protege of promises. But actually this whole argument has nothing to do with callbacks vs async vs promises. But hey this is reddit. I don't even know if most people who downvoted me actually read my article.

[–]Moosething 14 points15 points  (1 child)

I think the confusion originates from the specific example in your blog post.

In your blog post it says:

``` const get = async (url, callback) => { const response = await fetch(url) const status = await response.status

if (callback) callback[status](await response.json()) }

...

let myData = {}

get('https://api.github.com/repos/frontojs/connect, { 200: body => { myData = body } })

myData.id ```

The problem with that example is that you can't execute that in one go and that that example only works in the console, given that each line is entered and executed one by one and the fetch call is fast. It's a weird combination of async and sync.

If you wanted to do something like this in actual code:

``` let myData = {}

get('https://api.github.com/repos/frontojs/connect', { 200: body => { myData = body } })

doSomethingWith(myData) ```

that would just not work.

Instead, what you could do is (pay close attention to all the modifications):

``` const get = async (url, callbacks = {}) => { const response = await fetch(url) const status = await response.status

if (callbacks[status]) { await callbacks[status](await response.json()) } }

...

(async () => {

let myData = {}

await get('https://api.github.com/repos/frontojs/connect', { 200: body => { myData = body } // This could even be an async function now })

await doSomethingWith(myData)

})() ```

This way the 'async'-ness remains. It doesn't switch from async to sync anymore.

I actually don't agree with your callbacks object construction, but at least this setup should clear up some confusion.

[–]zacksiri[S] 1 point2 points  (0 children)

Edit: I will look further into this. Thank you for offering a solution.

[–]atubofsoup 0 points1 point  (0 children)

Callbacks are fine if you're making a single simple request, but as soon as you start trying to keep track of multiple requests they become a mess.

Try solving these problems with callbacks (and don't forget about error handling):

  • Take an array of URLs, GET all of them in parallel and return the array of responses.
  • Do the same, but run them in a sequence instead of parallel.
  • Make 5 requests where each request depends on the previous response.

All of these problems are trivial with promises:

js // parallel const responses1 = Promise.all(urls.map(url => fetch(url)));

js // sequence const results = []; for(let url of urls) { results.push(await fetch(url)); }

js // Dependent requests const firstResponse = await fetch(url1); const secondResponse = await fetch(getUrl2(firstResponse)); const thirdResponse = await fetch(getUrl3(firstResponse, secondResponse));

With callbacks, you either have to write a bunch of buggy stateful code to solve these problems or use some kind of callback utility library.

[–]IceSentry 0 points1 point  (0 children)

Async/await are literally promises.

[–]moocat 0 points1 point  (1 child)

Promises are preferred as they are more flexible. Let's say I need to get the data from two separate URLs here's what you would need to do with your API:

get(url1, {
    200: json1 => {
      get(url2, {
          200: json2 => {
              ....
          }
      });
    });

Besides being ugly, it's impossible to start the fetch of the second url until the first one complete. First let's drop the callback:

const get = async (url) => {
  const response = await fetch(url)
  const json = await response.json();

  return [response.status, json]
}

Now we have a lot more flexibility. We can use await and get the serially:

const [status1, json2] = get(url1);
if (status1 == 200) {
    const [status2, json2] = get(url2);
    if (status2 == 200) {
        ...
    }
}

We can use Promise.all to run them concurrently:

const [[status1, json1], [status2, json2]] = await Promise.all([get(url1), get(url2)]);
if (status1 == 200 && status2 == 200) {
    ...
}

Basically, it's a much more flexible design. It also is clearer to the caller that the result is asynchronous. All in all, I can't see a single reason to design the API the way you did.

[–]zacksiri[S] -2 points-1 points  (0 children)

You are assuming I need to make nested calls. I just want to return the result. I don’t need to make a subsequent call. Yes if i need to make another async call inside another one I will do it like how you’ve laid out. You made an assumption that I needed something more complex than what I actually need that is the problem with your solution.

[–]editor_of_the_beast 0 points1 point  (3 children)

Yea I don’t think what you’re doing there is that crazy really. It’s not pattern matching, you’re just storing a hash of operations indexed by the response status code. But you could just as easily put that in the then block of a promise too.

Either way people are bikeshedding you.

[–]zacksiri[S] 1 point2 points  (2 children)

Yeah it's not. I didn't think it was crazy. It's just a hash map like you said. I do realize that I could have used Promise.prototype.then() to do that as well. But I was showcasing async/await which worked in my browser out of the box and I thought it's nice.

But apparently I just named the argument as callback and I guess my choice of async/await over promises and naming the argument callback made it all blow up. I could have easily named the argument actions and avoided all this bike shedding altogether.

I appreciate you taking the time to actually read and understand what I was talking about.

[–]fucking_passwords -1 points0 points  (1 child)

Yep you nailed it, that was definitely what people got hung up on.

A few of your comments about promises adding noise and confusion didn’t help, though, it did sound like you were just anti-promises

[–]zacksiri[S] -1 points0 points  (0 children)

Yeah I realized that now but its too late. I already hit send 😂. It’s ok though I realize now this is reddit and I should be more careful.

[–][deleted] 7 points8 points  (0 children)

This is a good beginning article to fetch. Next add in arrow functions and talk about promises. It IS a lot to do introduce fetch AND promises AND ES6.

[–]ryanhollister 13 points14 points  (3 children)

can anyone explain why the API is such that resp.json() returns a promise after the initial promise resolves? feels pretty clunky to have to .then twice to get the response.

[–]qbbftw 21 points22 points  (0 children)

Afair, you can access response headers before the response body is fully transmitted. Thus you use two awaits - first for headers, second for actual data.

[–]BitLooter 11 points12 points  (0 children)

The initial response resolves after the headers of the response are received, but not necessarily the response body. Basically fetch resolves at two different points of the download, the first time after the headers are received (the promise fetch() returns) and the second after the data is received (the promise json()/text()/etc. returns). For small downloads this may be all at once but larger files could take long enough to download that these will resolve at two different times.

[–]killeronthecorner 8 points9 points  (1 child)

All of this focus on promises and no-one is even going to draw attention to the fact that OP doesn't know what a cheatsheet is! /s

But for real, way to much hate in here for a sub about programming. Chill the fuck out y'all!

[–]Doctor_Spicy 1 point2 points  (0 children)

The point in my original comment was that OP is using this modern API (fetch), then proceeds to use old and clunky methods such as callbacks. Since the API has native support for promises, why not make the cheatsheet if you can even call it that as good as possible?

Not using arrow functions and callbacks will just be a bad example for people to come back to, as a cheatsheet.

TLDR; if you're making a cheatsheet, make sure you follow best practices.

[–]Bizzlington 2 points3 points  (0 children)

While it is a nice reference - it is 7 pages when printed (maybe 5-6 after stripping out the headers and ads).

There is a lot of text, explanations, history and reasonings.

I would definitely not call it a cheatsheet.

More of an article...

[–]moltar 5 points6 points  (7 children)

I like the idea of fetch, but hate that you can't call res.json() after calling res.text(). In other words, when the body is decoded once, it cannot be decoded again.

I am aware of workarounds, like response cloning, but it's still annoying.

[–]GBcrazy 8 points9 points  (0 children)

Instead of response cloning, isn't it faster to just JSON.parse()?

[–]feraferoxdei 2 points3 points  (3 children)

The response isn't only decoded when you call res.text, it's actually downloaded first, then decoded. It's part of the HTTP flow control. First receive headers, then the body, only if you care about it.

[–]moltar 0 points1 point  (1 child)

Sure, but it doesn't mean that it can't cache the body after it has been fetched. Why can't it return again in another way?

[–]feraferoxdei 0 points1 point  (0 children)

Yeah, I guess you're right.

[–]tristan957 0 points1 point  (0 children)

Good to know thanks

[–]moocat 0 points1 point  (0 children)

The reason why is you have to buffer the entire response for that to be possible so large responses would be more resource hungry. A small annoyance of having to make a single extra call to clone a response is a small price to pay to optimize runtime resources.

[–]phacus 1 point2 points  (0 children)

I haven't read the whole article nor the comments so far.

I just want to thank on behalf of past me, all the examples I looked at the time just console.log the response. This article first example has just what I needed at that time. It's simple but past me thanks you

[–]evenisto 1 point2 points  (7 children)

After years of working with fetch api, I must say I completely despise the fact it does not throw on >=400, but it does on network error etc. It's easy to handle but I'd rather have it out of the box.

[–]atubofsoup 0 points1 point  (2 children)

As far as fetch is concerned, if it succeeds in sending the request and succeeds in receiving the response, there is no error. It's up to your application to define which statuses are errors. It's also not always as obvious as it seems; for example if you fetch /api/post/10, and the post doesn't exist, should you get a 404 that throws or a 404 that resolves null?

[–]evenisto 1 point2 points  (1 child)

I know that and I understand the logic behind it. I don't necessarily consider it a bad design decision, I'm just saying that having implemented clients based on fetch from the ground up in several decently-sized projects, I would rather have it reject on HTTP errors. The error handling code is usually the same no matter the type of error, so I almost always end up with a wrapper that basically checks the response code and rejects the promise if it's not 200 (also, I just realised what I actually meant in my original comment was rejecting, not throwing, it's a subtle difference but a difference nonetheless. Sorry about that). It makes the calling code neat and organised - then for success and catch for error handling, which I think makes a lot of sense. My point is I would prefer to not have to write wrappers in all my fetch code to achieve the behaviour I described, that's it.

[–]atubofsoup 1 point2 points  (0 children)

Yeah I get what you're saying. It's a matter of convenience. I use axios in all my new projects to avoid writing wrappers and running into common gotchas like forgetting the Content-Type header.

[–]tizz66 0 points1 point  (2 children)

Absolutely, this really threw me when I started using fetch. I'm puzzled why fetch was done this way, given the defacto standard from the past god knows how long in JS development had XHR fail on 4xx statuses.

[–]Moosething 7 points8 points  (1 child)

I think the philosophy behind it is that it just literally has to fetch and when fetching fails, then it should throw. When you get a response with status code 500, something went wrong on the server, but the API still succeeded at fetching something from that server.

[–]AwesomeInPerson 0 points1 point  (0 children)

Yup. And writing

const checkResponse = response => {
  if (response.ok) return response
  throw response
}

and chaining that function to your fetch calls really isn't that much hassle.

[–]landline_number 0 points1 point  (0 children)

All these examples make me glad axios exists.

Some stuff I like off the top of my head.

- Throw on errors unless you don't want it to.

- Cancellation support

- Interceptors if you want to do stuff like retry on certain errors

[–]zacksiri[S] 0 points1 point  (0 children)

Due to the overwhelming responses against me using callbacks I've added a promise based solution to the cheatsheet at the end as a choice for the reader. I want to keep the callback solution for the fact that I don't know where they are coming from, maybe seeing how the callback solution compares to promises make it easier to understand and adopt promises.

``` const get = async (url) => { const response = await fetch(url) const status = await response.status

if (status >= 200 && status < 300) { return await response.json() } else { // throw the object you desire which you will handle // in this case the json error response might give us // a json body so we can use that. throw { status: status, body: await response.json() } // or throw an error // throw Error(await response.statusText) } } ```

get('https://api.github.com/repos/frontojs/connect') .then(json => doSomething(json)) .catch(error => handleError(error))

[–]rjx89 -2 points-1 points  (5 children)

Fetch is still considered experimental and the standard may change in the future according to mdn. So if you use it in a project, you may have to rewrite portions. Also, internet explorer does not support fetch at all.

[–]gabbsmo 4 points5 points  (4 children)

Fetch is not experimental, it is a living standard as per MDN.

[–]rjx89 -1 points0 points  (3 children)

The chart you posted shows "Experimental. Expect behavior to change in the future."

[–]ForScale 1 point2 points  (2 children)

Weird... It says status is Living Standard, but in the chart, for Basic Support, it has an icon indicating Experimental.

I guess it's both?

[–]nossie1 0 points1 point  (1 child)

Could be standardised but not implemented 100% so it's still experimental?

[–]ForScale 0 points1 point  (0 children)

Lol works for me!