oliyh / re-graph

A graphql client for clojurescript and clojure
461 stars 40 forks source link

Websocket "connection_init" message #19

Closed gabrielnau closed 6 years ago

gabrielnau commented 6 years ago

Hello,

Thanks for this library, it's really great to not have to interop with Apollo.

I am looking on how to add authorization, and following your discussion I landed on this PR in lacinia-pedestal which didn't seem to have much activity, so I submitted a new one.

In the original PR and in mine too, the connection params are extracted from the payload of the connection_init message, because it seems to be apollo's behaviour.

I'm able to override re-graph's event handler to add the connection_init message but it makes me duplicate the whole handler code: https://gist.github.com/gabrielnau/6817c938754a182a796366c572f7607d

I wonder if you would be interested in adding this in re-graph ? I didn't opened a PR because it changes a lot of details, and you certainly have a strong opinion on how to approach it.

Cheers

oliyh commented 6 years ago

Hi,

Thanks for raising this. It is something that needs support in re-graph.

If I am not mistaken you would know the payload when you initialise re-graph, and the payload should be sent whenever the websocket makes a connection (including after reconnect).

Because of this I think the best approach would be to add another option in re-graph's init event where the user could provide the message as data.

Does that fit your use case?

Thanks

gabrielnau commented 6 years ago

Hello,

Yes, that would be perfect.

I can submit a PR if you want, we would have to discuss the init option naming, and if the ::send-several-ws effect I came up with is the right way to approach the initialization.

Thanks

oliyh commented 6 years ago

Hi,

I think :connection-init-payload would be a good name.

There shouldn't be any need to change the way messages are sent, it should be sufficient to initialise the queue to contain this message as the first message (and to enqueue it again on disconnect).

gabrielnau commented 6 years ago

Hello,

I submitted a PR: https://github.com/oliyh/re-graph/pull/20, I will close this issue and we could follow up in the PR ?