Provide HasForeign instance for servant-foreign

[felixSchl]

trafficstars

My use case is I am using servant-purescript to generate purescript code from my API, but since there is no HasForeign instance for Servant.Multipart.MultipartForm, compilation fails. I wish I could contribute a patch for this but at the time of this writing, the HasForeign typeclass and it's instances are beyond my grasp

[alpmestan]

@felixSchl If you're willing to throw some hours into this (and/or anyone else really), I (and maybe @phadej too) would definitely be around to help, explain things, etc. I'm sure servant-multipart will one day have support for -client, -foreign and what not, but I'm afraid that has to happen on an on-demand basis. I put together the package as it is now so that at least everyone could handle server-side multipart file uploads, but it is indeed currently missing support for all the other interpretations.

[phadej]

@felixSchl to clarify, you want to upload files from PureScript app? otherwise you could split your API into one having HasForeign and "everything else".

I mean, it would be great to be able to upload files from foreign languages, but there is no work done in that direction. I'd personally start with servant-client as it way easier to figure out.

[afcady]

I've just looked into this. It turns out that it is easy to create an instance, but it is impossible to create the correct instance -- without patching servant-js. The problem is that servant-js hard-codes xhr.send(JSON.stringify(body)) where we need just plain xhr.send(body) in order to construct a mime multipart request body.

Here is an instance along with the javascript that gets generated:

instance (HasForeignType lang ftype a, HasForeign lang ftype api)
      => HasForeign lang ftype (MultipartForm t a :> api) where
  type Foreign ftype (MultipartForm t a :> api) = Foreign ftype api

  foreignFor lang ftype Proxy req =
    foreignFor lang ftype (Proxy :: Proxy api) $
      req & reqBody .~ (Just $ typeFor lang ftype (Proxy :: Proxy a))

...and...

        var postMessageByUuidBlob = function(uuid, body, onSuccess, onError) {
          var xhr = new XMLHttpRequest();
          xhr.open('POST', '/message/' + encodeURIComponent(uuid) + '/blob', true);
          xhr.setRequestHeader('Accept', 'application/json');
-->       xhr.setRequestHeader('Content-Type', 'application/json');
          xhr.onreadystatechange = function () {
            var res = null;
            if (xhr.readyState === 4) {
              if (xhr.status === 204 || xhr.status === 205) {
                onSuccess();
              } else if (xhr.status >= 200 && xhr.status < 300) {
                try { res = JSON.parse(xhr.responseText); } catch (e) { onError(e); }
                if (res) onSuccess(res);
              } else {
                try { res = JSON.parse(xhr.responseText); } catch (e) { onError(e); }
                if (res) onError(res);
              }
            }
          };
-->       xhr.send(JSON.stringify(body));
        };

What we want to do here to make this work is get rid of JSON.stringify and also the Content-Type header. These behaviors aren't parameterized on the type instance though. They're just hard-coded right here:

https://github.com/haskell-servant/servant-js/blob/46ff335b330b20e7d77532011fbd51e710764605/src/Servant/JS/Vanilla.hs#L37

https://github.com/haskell-servant/servant-js/blob/46ff335b330b20e7d77532011fbd51e710764605/src/Servant/JS/Vanilla.hs#L52

https://github.com/haskell-servant/servant-js/blob/46ff335b330b20e7d77532011fbd51e710764605/src/Servant/JS/Vanilla.hs#L80-L83

Apparently the way to do this is to add a new member to data Req, to set the value of this member in the instance above, and then patch servant-js to alter its output based on that value:

https://github.com/haskell-servant/servant/blob/af7ba3d6b82e67e032a03992649cdbaa7ca51517/servant-foreign/src/Servant/Foreign/Internal.hs#L126-L134

E.g. it could be something like _reqRawBody :: Bool

But yeah basically this requires patching servant itself as well as servant-js.

[alpmestan]

That's not a problem, I think supporting more interpretations (client, foreign, etc) for -multipart would be great and if the patches make sense we'd be happy to take them, in the core servant libs.

@afcady Would you like to give it a shot?

[afcady]

OK, I did it! I bumped the version on servant-foreign and added dependencies on that new version to the other packages.

[afcady]

Sad :(

[alpmestan]

Because the PRs have been up for 2 weeks but did not get merged yet? We're doing our best to review things promptly, but it's not always possible. Sorry for the inconvenience.

[alpmestan]

I reviewed your PRs. Sorry for the delay.