reason-graphql-fullstack icon indicating copy to clipboard operation
reason-graphql-fullstack copied to clipboard

Suggestions

Open andreas opened this issue 7 years ago β€’ 5 comments

Thanks for sharing your code, @anmonteiro! πŸ™It's great to have some more examples out there.

Two quick suggestions/questions:

  • Could some of the usages of io_field be replaced with field to avoid Lwt_result.return, e.g. this one?
  • Could you use use HTTP server that ships with graphql-lwt, or does it lack some feature compared to the one you implemented? At a glance they seem quite similar.

andreas avatar Jun 07 '18 18:06 andreas

@andreas

Could you use use HTTP server that ships with graphql-lwt, or does it lack some feature compared to the one you implemented? At a glance they seem quite similar.

I'm guessing it's because of the static file serving logic. I had to do something similar to customize the included GraphiQL page.

Is there a way to include the module and override the assets/route handlers without vendoring graphql_lwt?

ozanmakes avatar Jul 10 '18 13:07 ozanmakes

@osener that's indeed the reason; @andreas and I talked on Discord about it and I never responded in this issue.

Is there a way to include the module and override the assets/route handlers without vendoring graphql_lwt?

I think that's what this issue is for, correct me if I'm wrong: https://github.com/andreas/ocaml-graphql-server/issues/93

anmonteiro avatar Jul 10 '18 17:07 anmonteiro

If I understand you correctly, what you're looking for is a way to compose the GraphQL HTTP handler with other routes. To that effect, Graphql_lwt.Server could expose a function with the following type

'ctx Graphql_lwt.Schema.schema ->
Cohttp.Request.t ->
Body.t ->
'ctx ->
(Cohttp.Response.t * Body.t) Lwt.t

Is it something like that you're looking for?

I don't consider andreas/ocaml-graphql-server#93 to be about such a function, that's simply moving the HTTP server to it's own OPAM package.

andreas avatar Jul 10 '18 18:07 andreas

That sounds like a good idea but I'd still put it in the separate package that https://github.com/andreas/ocaml-graphql-server/issues/93 is about.

If I wanna use ocaml-graphql-server with Httpaf, for example, I wouldn't want there to be Cohttp dependencies in the dependency I'm pulling.

anmonteiro avatar Jul 10 '18 18:07 anmonteiro

@anmonteiro that makes sense πŸ‘

andreas avatar Jul 10 '18 20:07 andreas