Replies: 29 comments 7 replies
-
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:
But then we'd need a secondary effect returned from GotResponse. Perhaps like:
I think people will dislike having to make intermediate actions like this. But perhaps it's actually the only sane approach. |
Beta Was this translation helpful? Give feedback.
-
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
|
Beta Was this translation helpful? Give feedback.
-
I'm not completely sure what you're suggesting. If it is that we make an effect for 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:
There's also the possibility of integrating |
Beta Was this translation helpful? Give feedback.
-
@okwolf Your hyperapp-fx lib has a fetch/http effect right? What are your thoughts? |
Beta Was this translation helpful? Give feedback.
-
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. |
Beta Was this translation helpful? Give feedback.
-
@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) |
Beta Was this translation helpful? Give feedback.
-
Any interest in basing this off Pros: Browser support w/ easy cancellation (without polyfills), progress tracking of uploads, maybe more? On an API-design note, I like how |
Beta Was this translation helpful? Give feedback.
-
Seems pretty similar to |
Beta Was this translation helpful? Give feedback.
-
@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 ) |
Beta Was this translation helpful? Give feedback.
-
@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:
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. |
Beta Was this translation helpful? Give feedback.
-
@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. |
Beta Was this translation helpful? Give feedback.
-
Another question (which @SkaterDad wisely brought up) is: |
Beta Was this translation helpful? Give feedback.
-
@zaceno for 3rd party I still feel fine with using Ultimately it's an individual judgement call how many of your users are in the remaining small percentage without |
Beta Was this translation helpful? Give feedback.
-
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 |
Beta Was this translation helpful? Give feedback.
-
I would rather not base the HTTP effect API on the awkward 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. |
Beta Was this translation helpful? Give feedback.
-
Is there something in there that doesn't work with IE10? I didn't notice anything, but I only skimmed through quickly. |
Beta Was this translation helpful? Give feedback.
-
Only my copy paste error of |
Beta Was this translation helpful? Give feedback.
-
Thanks everyone who chimed in! My conclusion is this: I propose that Hyperapp adopts both the api and
Yes, but Elm is a different language, and must implement its own solutions for everything. Hyperapp really doesn't need to support every feature of every browser api, since any user with an "advanced" use case can simply make custom fx/subs. It is important that we come out with something along with the release of Hyperapp 2, but it doesn't need to do everything. @okwolf's API seems good enough for most use case, and is easy to implement since it is close to the We can always come out with a more advanced version of the effect (or an entire suite of HTTP-related effects & subs) later.
IE's XHR doesn't support json As for the things XHR supports that I would PR it, except that would involve making a package.json that I can't verify. Also the question of how to package & export the effect. I leave it up to @jorgebucaran to consider this suggestion 🙏 |
Beta Was this translation helpful? Give feedback.
-
@zaceno @okwolf One argument in favor of XHR is the native support for the timeout. Is this something to consider? An alternative can be sending a signal using the const controller = new AbortController();
const signal = controller.signal;
setTimeout(() => controller.abort(), 5000);
fetch(url, { signal }).then(response => {
return response.text();
}).then(text => {
console.log(text);
}).catch(err => {
if (err.name === 'AbortError') {
console.log('Fetch aborted');
} else {
console.error('Uh oh, an error!', err);
}
}); Reference: https://developers.google.com/web/updates/2017/09/abortable-fetch |
Beta Was this translation helpful? Give feedback.
-
@hlibco Definitely. 👍 Here's an example I put together to show how we could use import { app, text } from "hyperapp"
import { main } from "@hyperapp/html"
import { request, track } from "@hyperapp/http" We also need a way to refer to multiple http requests. const TrackerId = 1 This is just a function to create a specific http effect based on our desired parameters. const getHyperapp = (action) =>
request({
tracker: TrackerId,
method: "GET",
url: "https://unpkg.com/hyperapp/hyperapp.js",
headers: [],
action,
}) This is a helper function to set the status of the request, which can be: loading, success, or failure. Maybe it would be nicer if JavaScript had enums, but const setStatus = ({ loading, success, failure } = {}) => ({
loading,
success,
failure,
}) This action is only called when we have everything requested or if there was an error. const GotSource = (state, { data, error }) => ({
...state,
...setStatus(error ? { failure: error } : { success: data }),
}) The
const Progress = (state, progress) => ({
...state,
progress,
}) Here's the rest of the app. app({
init: [
setStatus({
loading: {
tracker: TrackerId,
progress: { received: 0, size: null },
},
}),
getHyperapp(GotSource),
],
view: (state) =>
state.failure
? text("I was unable to load your book: " + state.failure.error)
: state.loading
? text("Loading..." + state.loading.progress.received)
: main({}, text(state.success.data)),
subscriptions: (state) => [
state.loading && track(state.loading.tracker, Progress),
],
node: document.getElementById("main"),
}) Pattern matching would make this app way cooler to write, but not today. |
Beta Was this translation helpful? Give feedback.
-
Well, there are some APIs that may give json payloads on 4xx errors, including 404s, so you may still want to parse those, as they may give feedback to the application and user. I think keeping things close to the metal would be good; effects.Fetch({
url: '...',
options: {}, // whatever fetch accepts, including method, headers, etc.
onResponse: someActionHere,
onError: someActionHere,
}) The more API we build on top, the more we have to maintain, and the less control we give to the user. I could adventure with a little sugar on top to make things easier: effects.Fetch({
url: '...',
options: {}, // whatever fetch accepts, including method, headers, etc.
onOk: someActionHere,
onError: someActionHere,
okStatusCodes: [200, 201, 202, 203, 204, 205, 206, 207, 226],
bodyType: 'text|json|blob|formData|arrayBuffer',
}) Behind the scenes, this would be something like: const parsers = {
text: response => response.text(),
json: response => response.json(),
blob: response => response.blob(),
formData: response => response.formData(),
arrayBuffer: response => response.arrayBuffer(),
};
const FetchFX = (dispatch, props) => {
const parser = parsers[props.bodyType] || parsers.text;
fetch(props.url, props.options)
.then((response) => {
if (!props.okStatusCodes.includes(response.statusCode)) {
return parser(response)
.then(body => dispatch(props.onError, { statusCode: response.statusCode, body })
}
return parser(response)
.then(body => dispatch(props.onOk, { statusCode: response.statusCode, body });
})
} But I'd have a lot of caution adding too much on top of the default fetch implementation. |
Beta Was this translation helpful? Give feedback.
-
We're on the same page. Userland belongs to userland.
We should provide the bare minimum that works for Hyperapp. Elm's |
Beta Was this translation helpful? Give feedback.
-
@jorgebucaran I'm curious to understand more about the API for the In your example:
Will it be possible to return to the |
Beta Was this translation helpful? Give feedback.
-
Is there an updated version that works with the actual way request works? In your example code your action function is:
but request's implementation today is:
That is, How are you supposed to read the json error message of a 4xx response in Hyperapp 2.0? |
Beta Was this translation helpful? Give feedback.
-
@Ran4 I agree that the current http effect is not the ultimate for all use cases. I don’t think the response body is very commonly needed, for example (but sometimes: yes absolutely) The good news is that making your own custom effects that do exactly what you want is easy. But for Jorge making an official http effect that satisfies all the use cases will mean making an api that is pretty complicated and involves (how complicated? - look how elm does it and tremble) From my perspective this api strikes a good balance. It is convenient to get started and once you need more than it offers, you just switch over to a custom effect that suits you. There’s never a reason to complicate your logic with an overworked generic api. |
Beta Was this translation helpful? Give feedback.
-
Eventually, we do want a more elaborate HTTP effect, very much like Elm's. We don't have one right now because designing one is challenging, but we'll get there. For now, using your own custom effect as a workaround is fine, though. |
Beta Was this translation helpful? Give feedback.
-
I actually just hit this issue and it was pretty painful to figure out, but it now works reliably (I'm not sure about "elegantly", as there's like 4 layers of callbacks, and the code to handle "200 + json" vs "403 + json" are completely different... but it works) (Using My API returns data like {
"status": "ok",
"code": 200,
"messages": ["Successfully did the thing"],
"data": {
"cats": ["Alice", "Bob", "Cirilla"]
}
} As long as the server doesn't crash completely, then it'll return data in that format, whether it's 200 or 403 or 302 or whatever. If it does crash, then we just see export function DisplayResponseMessage(dispatch, response) {
function parseResponse(data) {
// if we got 2XX or 3XX, show a temporary "ok" notification
// which will disappear after 2 seconds
if(data.code >= 200 && data.code < 400) {
dispatch(state => [
{ ...state, notification: {text: data.messages[0], style: "ok"} },
Delay({
wait: 2000,
action: (state) => ({...state, notification: null})
})
]);
}
// if we got something else (4XX, 5XX), show an "error" notification
// which doesn't go away until clicked
else {
dispatch(state => ({ ...state, notification: {text: data.messages[0], style: "error"} }));
}
}
// When Http() is successful, "response" is the json from the server
if(response.code) {
parseResponse(response);
}
// When Http() fails, "response" is the raw blob, which we need
// to decode for ourselves. First attempt to decode JSON (like
// if the server sent us a 403). Fall back to "internal error" if
// we can't decode it (like if the server crashed)
else {
response
.json()
.then(parseResponse)
.catch(err => {
console.log(err);
dispatch(state => ({ ...state, notification: {text: "Internal Error", style: "error"} }));
});
}
} Which gets used like: <button
onclick={(state) => [
{ ...state, loading: true },
Http({
url: "https://mysite.com/cats.json",
action: (state, response) => ([
{
...state,
loading: false,
cats: response.data.list,
},
[DisplayResponseMessage, response],
),
error: (state, response) => [
{
...state,
loading: false,
},
[DisplayResponseMessage, response],
],
}),
]}
disabled={state.loading}
>Get Cats</button> |
Beta Was this translation helpful? Give feedback.
-
EDIT: Fix typo While writing custom effects is an easy way to escape Hyperapp's exercise in purity allowing you to do absolutely anything, it's also clear that we're missing a functional equivalent to promises to compose pipelines and extend our stay in pure land. 🦄 If you need to make more than one request to fetch some data, for example, when we need a piece of information from a response to make the subsequent request, the only decent way we have right now is to handle each response in a different action, issuing a subsequent effect and so on, but of course, this means ephemeral actions and in some cases storing intermediate results in the state. Let's introduce a new package to deal with stuff like this. We can call it
import * as Task from "@hyperapp/task" This package needs to export at least one effect: Task.run(task, GotResult) How do we create a task? Well, you know just how import { request } from "@hyperapp/http"
request({ url }, GotResult) Now that we have a task, let's run it. app({
view: (state) =>
button({
onclick: (state) => [
state,
Task.run(request({ url: "..." }), GotResult),
],
}),
}) For single requests, this might be overkill. import { get } from "@hyperapp/http"
app({
view: (state) =>
button({
onclick: (state) => [
state,
get({ url: "..." }, GotResult),
],
}),
}) The humble const GotAllComments = (state, comments) =>
comments
? {
...state,
allComments: state.allComments.concat(comments),
}
: noCommentsError(state)
const GotPosts = (state, posts) =>
posts
? [
state,
...posts.map((post) =>
get({ url: `${SERVER_URL}/comments?postId=${post.id}` }, GotAllComments)
),
]
: noPostsError(state)
const GotUsers = (state, [first]) =>
first
? [state, get({ url: `${SERVER_URL}/posts?userId=${first.id}` }, GotPosts)]
: noUsersError(state)
const GetAllComments = (state) => [
state,
get({ url: `${SERVER_URL}/users` }, GotUsers),
]
app({
view: (state) =>
section(
button(
{ onclick: GetAllComments },
text("Get all comments from first user")
)
),
}) We managed to do this without storing unnecessary data in the state, but we needed a bunch of actions, one for each step of the process. Can we improve this with tasks? I don't know. Let's see. This time the API is not as clear-cut as usual, so I've come up with a few pseudo-examples for you to look at. All examples use The Familiar Patternimport { request } from "@hyperapp/http"
import * as Task from "@hyperapp/task"
const GetAllComments = (state) => [
state,
Task.run(
request({ url: `${SERVER_URL}/users` })
.andThen(([first]) =>
first
? request({ url: `${SERVER_URL}/posts?userId=${first.id}` })
: Task.fail(Error("no users"))
)
.andThen((posts) =>
Task.all(
posts
? posts.map((post) =>
request({ url: `${SERVER_URL}/comments?postId=${post.id}` })
)
: Task.fail(Error("no posts"))
)
),
GotAllComments
),
] The Fantastic Pattern
import { request } from "@hyperapp/http"
import * as Task from "@hyperapp/task"
const GetAllComments = (state) => [
state,
Task.run(
request({ url: `${SERVER_URL}/users` })
|> Task.andThen(([first]) =>
first
? request({ url: `${SERVER_URL}/posts?userId=${first.id}` })
: Task.fail(Error("no users"))
)
|> Task.andThen((posts) =>
Task.all(
posts
? posts.map((post) =>
request({ url: `${SERVER_URL}/comments?postId=${post.id}` })
)
: Task.fail(Error("no posts"))
)
),
GotAllComments
),
] The Nested Patternimport { request } from "@hyperapp/http"
import * as Task from "@hyperapp/task"
const GetAllComments = (state) => [
state,
Task.run(
request(
{ url: `${SERVER_URL}/users` },
request(
([first]) =>
first
? request({ url: `${SERVER_URL}/posts?userId=${first.id}` })
: Task.fail(Error("no users")),
request((posts) =>
Task.all(
posts
? posts.map((post) =>
request({ url: `${SERVER_URL}/comments?postId=${post.id}` })
)
: Task.fail(Error("no posts"))
)
)
)
),
GotAllComments
),
] Finally, how do we deal with So, what do you all think? Even if the pipeline operator is still far away from materializing any time soon, we still have a couple of options that could work for us right now. 🎯 |
Beta Was this translation helpful? Give feedback.
-
I just published a package:
The repo with readme is here: https://github.com/zaceno/zx-hyperapp-extra I published it in order to make my recent experiments with tasks available for anyone to experiment with and suggest improvements (like a prototyping space for potential future @hyperapp/xyz packages). My http package provides But if you're not chaining multiple http-calls, then working with tasks probably feels like more complexity than necessary. That's why I also offer some functions that return regular effects:
I should probably also offer a |
Beta Was this translation helpful? Give feedback.
-
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:
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 getonBody
callbacks even when there is no body, or the body is not of thebodyType
you specified (in which caseonErrorParsingBody
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?Beta Was this translation helpful? Give feedback.
All reactions