API for fetch-effect

[zaceno]

In my humble opinion, there needs to be an official fetch effect (or http, or ajax or whatever) for the final release of Hyperapp 2. Otherwise, it will be the first thing people ask for, and telling them to make their own would be inconsistent with the message that "you should not need to write your own effects most of the time".

The question is only what the API should be. I opened this issue to have that discussion, as it isn't as easy as one might think at first.

My first suggestion would be something like this:

Like:

Fetch({
  url: ... 
    //  the URL in a native fetch(url) call, mandatory
  
  options: {...}, 
  //the options in a fetch(url, options) call. 
  // Optional, default: undefined or {},
  
  onResponse: ... ,
  // fetch(...).then(resp => dispatch(props.onResponse, resp)
  // Optional, default: no dispatch

  onFailure: ...,
  //fetch(...).catch(err => dispatch(props.onFailure, err)
  // Optional, default: no dispatch

  bodyType: ...
  //only relevant if using onBody callback
  // can be 'text', 'json', 'blob', 'formData', 'arrayBuffer'
  // Optional, default 'text',

  onBody: ...
  // fetch(...)
  //    .then(response => response[props.bodyType]())
  //    .then(body => dispatch(props.onBody, body)
  // Optional, default: no dispatch

  onErrorParsingBody: ...
  // fetch(...)
  //    .then(response => response[props.bodyType]())
  //    .catch(error => dispatch(props.onErrorParsingBody, error)
  // Optional. Default: no dispatch
})

The thing that concerns me with this, is that fetch will resolve for any response from the remote. Including 404's et c. That means you'll get onBody callbacks even when there is no body, or the body is not of the bodyType you specified (in which case onErrorParsingBody will be dispatched).

So I was thinking, we could limit the body-parsing routine to be done only for 200 OK responses, and only if the request was using the GET method. But perhaps that's to restrictive?

If we limit body parsing to GET requests with 200 OK responses, then we'll need another option: onNotOK (maybe?) for the case when we got a response but decided not to parse the body. Or is that getting too complicated?

[zaceno]

Clearly the complicating issue here is that what we most often want is the body (most often parsed JSON). And that requires a two-step promise-chain, with some logic in between.

A more "close-to-native" approach would be to have simply:

Http.fetch({
  url: ...,
  options: {...},
  onResolve: GotResponse,
  onReject: NetworkIssue
})

But then we'd need a secondary effect returned from GotResponse. Perhaps like:

const GotResponse = (state, response) => {
   if (response.statusCode !== 200) return state
   return [state, Http.BodyAsJSON({
      body: response.body,
      onResolve: GotData
   })]
}

I think people will dislike having to make intermediate actions like this. But perhaps it's actually the only sane approach.

[johnkazer]

Statechart option? https://statecharts.github.io/xstate-viz/ (general case) https://xstate.js.org/docs/packages/xstate-react/#configuring-machines (a React example, could be modified slightly for Hyperapp) https://blog.usejournal.com/handling-data-fetching-with-state-machines-4e25b6366d9 is a first-principles state-machine approach using React

[zaceno]

@johnkazer

I'm not completely sure what you're suggesting. If it is that we make an effect for fetch which has a dependency on xstate, that's not an option. Hyperapp is dependency free today and I don't think anyone is interested in changing that.

Your state chart does illustrate something very clearly: that fetch is a multi-step process where users will want to configure it in many different ways.

Hence I completely reject my first proposal and suggest this:

/*
Equivalent to:
fetch(url, options)
  .then(response => dispatch(onResolve, response)
  .catch(error => dispatch(onReject, error)
*/
Http.Fetch({url, options, onResolve, onReject})

/*
Equivalent to:
body[type]()
  .then(data => dispatch(onResolve, data))
  .catch(error => dispatch(onReject, error))
*/
Http.ParseBody({body, type, onResolve, onReject})

There's also the possibility of integrating AbortController signals, and progress tracking, but I think the above will suffice for most use cases.

[zaceno]

@okwolf Your hyperapp-fx lib has a fetch/http effect right? What are your thoughts?

[johnkazer]

The latter option for a state-machine approach doesn't have a dependency - just implements a simple comparable approach.

Either way, I think the point you raise about clear steps is a good one.

[zaceno]

@johnkazer Ah, I think I see what you're saying: by splitting up the flow as several effects rather than one huge one with various optional callbacks, we allow users more freedom in designing their desired flow, using state-charts for example. That makes sense, and is basically what I took away from your links, why I now believe more in having an effect for doing the fetch request, and then a collection of effects for managing the result of it (body-parsing, abort-signalling, progress-tracking)

[SkaterDad]

Any interest in basing this off XMLHttpRequest, rather than fetch? Since you're abstracting over it anyway, the awkward API isn't a big deal.

Pros: Browser support w/ easy cancellation (without polyfills), progress tracking of uploads, maybe more?

On an API-design note, I like how wretch handles errors. The library sorts out the various types of errors, and you can catch them as you like. https://github.com/elbywan/wretch#catchers

[okwolf]

Seems pretty similar to { Http } from "hyperapp-fx" except for different action and body type props. I'm noticing a trend toward dropping support for browsers without fetch, but if you really need it, there are polyfills.

[sergey-shpak]

@SkaterDad no worries about browser support, - there is no easy way to cancel hyperapp#2 'fetch' effect in general, you have to treat 'fetch' as a subscription to make it cancelable )

[zaceno]

@okwolf I checked the internals of your Http effect now (which I hadn't earlier) and see you're doing what I suggested about only parsing the body if the status code is 200. But you have a different take on errors. You use the same error callback when:

• the url is malformed (no response), or network issues hang up
• the status-code is anything but 200
• the body could not be parsed according to given response type

Is that correct?

I'm sure in the long run, people will want more logic branches, and ability to use signals, tracking (for downloads and uploads ). But for now, just to give people something, perhap's @okwolf's design is good enough? What can't you do with it?

If the the things you can't do with it are uncommon enough, we can still tell people to make their own fx/subs. We just shouldn't have to tell them that for things that almost everyone needs.

So now I'm leaning more towards something even simpler than what I first suggested: follow @okwolf's approach (https://github.com/okwolf/hyperapp-fx/blob/498416ad98d713b3d1a5a5d2a74a6335165ad7bb/src/fx/Http.js#L3).

But only for now. For later it will need to be more elaborate I'm quite sure.

[okwolf]

@zaceno your summary sounds accurate. My HTTP effect is built for users who for the most part care if a request succeeded or failed overall. The downside to this is if you need to react to different errors in different ways, your error action will need conditional logic based on inspecting the payload.

[zaceno]

Another question (which @SkaterDad wisely brought up) is: fetch or XmlHttpRequest ? Yes fetch can be polyfilled, but Hyperapp itself is compatible with IE >= 10 (I think), so should we make sure all fx and subs are as well? (without polyfilling) - If so: xhr it is.

[okwolf]

@zaceno for 3rd party I still feel fine with using fetch, but you do bring up a good point about what to use for the official @hyperapp/http library.

Ultimately it's an individual judgement call how many of your users are in the remaining small percentage without fetch support.

[zaceno]

Yeah, I suppose so. Personally I'd be fine dropping IE support all together in v2. But even if core remains IE-compatible, it could be argued that IE is a special use case so they have to write their own effects. I guess it's a question for @jorgebucaran

[okwolf]

I would rather not base the HTTP effect API on the awkward XmlHttpRequest implementation, and if the options are instead based on those from fetch, why not use it?

Just to be clear, what I'm advocating for isn't that users requiring IE support write their own FX, but that they have to bring their own polyfill for this feature. create-react-app ships with these polyfills as a reference.