Further generalise the fetch API #888
Conversation
js/miso.js
Outdated
| @@ -27,7 +27,7 @@ function fetchJSON(url, method, body, headers, successful, errorful) { | |||
| if (!response.ok) { | |||
| throw new Error(response.statusText); | |||
| } | |||
| return response.json(); | |||
| return response.text(); | |||
There was a problem hiding this comment.
I think this means that we sometimes UTF-8 encode then decode unnecessarily. We should potentially use .bytes() instead.
There's no instance for FromJSVal ByteString unfortunately. There is FromJSVal [Word8] which is isomorphic, but obviously not the most performant data structure... Could do with some benchmarking. It'd be interesting to get some realistic benchmarks on strings anyway - I'm curious about how much MisoString helps when IME one is mostly interacting with libraries that use Text.
There was a problem hiding this comment.
We could try JSON.parse(response.json()), in the case of the MIME being JSON. Then we'd rely on the FromJSVal Value instance, since that seems to be faster than parsing, at least Haskell side it should be more performant.
For OctetStream we might want to consider trying ArrayBuffer (.arrayBuffer(), .blob() methods). There's also .formData() since I think servant supports that as well
There was a problem hiding this comment.
It is a lot simpler if we can treat everything uniformly on the JS side. Otherwise we'd need some new typeclass for delegating content types to response methods.
I think it's valid to use something like .bytes() or .arrayBuffer() as a lowest common denominator, and then marshal that to a lazy bytestring for input to mimeUnrender. Using .text() as we do currently is only a valid common approach for UTF-8 data, and less efficient.
There was a problem hiding this comment.
Then we'd rely on the
FromJSVal Valueinstance, since that seems to be faster than parsing, at least Haskell side it should be more performant.
I guess browsers probably have faster JSON parsers than Aeson, so yeah we could in effect do the ByteString -> Aeson.Value step there by using .json(). Whether it's worth the cost of doing FFI on a more complex object, I don't know. That part of JSaddle is opaque to me at this point. And anyway, it would have the downside mentioned in the previous comment, i.e. increased complexity.
There was a problem hiding this comment.
I'd say we find some way to operate on the raw bytes, and save special handling for JSON etc. as a potential future optimisation.
src/Miso/Fetch.hs
Outdated
| Comp x :* xs -> case partitionEithers x of | ||
| (err', []) -> bimap (map (\(t, s) -> show t <> ": " <> s) err' <>) S $ tryParsers xs | ||
| (_, (res : _)) -> Right . inject . I $ res | ||
| class UnrenderResponse (cts :: [Type]) (a :: Type) where |
There was a problem hiding this comment.
Should we use AllMimeUnrender w/ MimeUnrender from
https://hackage.haskell.org/package/servant-0.20.2/docs/Servant-API-ContentTypes.html#t:MimeUnrender
There was a problem hiding this comment.
Yes, good point, actually we can! Just pushed as 59b85a1.
I was copying servant-client a bit too closely here. I guess they're interesting enough to need their own type class for this, but we're not.
This is based on `servant-client` as well as our existing instance for `QueryParam' mods`.
Again heavily based on `servant-client`.
The exact name isn't particularly important since the function isn't exposed. But "JSON" had become a misnomer now that it handles other content types.
georgefst
force-pushed
the
fetch-generalise
branch
2 times, most recently
from
e0252d9 to
536ca3f
Compare
Errors on the left feels more natural. It also makes it easier to wrap calls in `ContT` to avoid nested callback hell.
This avoids various dependencies, including `template-haskell`. Note that `sop-core` is already a recursive dependency via `servant`.
In practice, this is for GHCJS.
georgefst
force-pushed
the
fetch-generalise
branch
from
536ca3f to
4fc2eb0
Compare
This has enabled me to work with some fairly big and complex APIs. I'm happy to change some details. Importantly,
fetchJSONshould still work exactly as before, and the internal version, which I've calledfetchFFI, isn't even exposed.The only big thing still missing is proper handling of HTTP status codes, but I haven't had a use case for that. Maybe I'll revisit it when the next release of Servant arrives with its multi-verb stuff.