add clj client support

[Frozenlock]

I tried to separate sente's logic from the websocket implementation while changing the least amount of things possible. Ajax is still cljs only.

It's probably a little more low-level than the IServerChanAdapter. Here we provide a function to let sente create new websockets when needed.

Tested with both cljs and clj (aleph) and it seems to work.

[ptaoussanis]

Hi there, thanks a lot for this!

Since the PR touches so much code, any chance I could ask you to possibly summarise any significant changes that were made so that I know where to focus attention?

Will try schedule a look w/in the next week or two.

Again, much appreciated - cheers! :-)

[danielcompton]

This is easier to review with https://github.com/ptaoussanis/sente/pull/275/files?w=0 as it hides whitespace changes.

[ptaoussanis]

@danielcompton Ooh, wasn't familiar with that feature - thanks! That's a nice time saver.

@Frozenlock For clarity: still keen to get an overview of necessary changes, esp. if anything is non-obvious or involved tradeoffs that I should be aware of. The quicker I can evaluate a PR, the more likely that I can get it merged quickly. Thanks, cheers! :-)

[Frozenlock]

@danielcompton thanks for the link.

@ptaoussanis not much has changed; for some reason git doesn't like when we remove the reader conditionals ;-)

Most of the work (other than understanding how all that worked) was extracting the JS-only stuff from ChWebSocket and putting it in JSDefaultWebSocket.

I would invite you to check the addition into interfaces.clj to know what was my approach for this.

Users can provide :cws-creator like this:

(let [{:keys [chsk ch-recv send-fn state]}
      (sente/make-channel-socket-client! "/chsk"
       {:type :ws ; e/o #{:auto :ajax :ws}
        :protocol :http
        :host "localhost:3444"
        :cws-creator al/aleph-create-client-websocket!})]
  (def chsk       chsk)
  (def ch-chsk    ch-recv) ; ChannelSocket's receive channel
  (def chsk-send! send-fn) ; ChannelSocket's send API fn
  (def chsk-state state)   ; Watchable, read-only atom
  )

All of this is very basic - just enough to get it to work. The user has to manually provide the protocol and the host when using the clj client.

[ptaoussanis]

Great, thanks for the details. Will try take a look next week, soon as I can!

[Frozenlock]

I'll emphasize on how this was just the minimum required to get it to work. I'd appreciate if someone a little more familiar with sente could polish it to be more sente-like.

[ptaoussanis]

No problem :-) Would welcome feedback if anyone has in the meantime, but I also take a good look at everything myself before merging.

Just have my hands very full lately, so try schedule batches on each project.

[robert-spurrier]

Just chiming in to say that I ran into this limitation today, and will be eyeballing this PR. Thanks to you both!

[ptaoussanis]

Sorry for the delay on this, haven't forgotten - just prepping for a move in the next 2 weeks. Will try make this a priority soon as I'm settled again next month.

[ignorabilis]

Hi - I've got a question regarding this PR since I'm an interested side as well :) - any chance of this getting ajax support as well?

[Frozenlock]

Any updates on the support for clj client?

[nha]

it looks like errorf expects a format string as second argument?

[nha]

did you mean (when on-error (on-error e))?

[nha]

It looks like the meta argument of the packer has been removed since this PR.

(pack packer (meta resp-clj) resp-clj) ;; (meta resp-clj) should be removed

I cannot see the metadata effectively used by the new code, is that correct @Frozenlock ?

[nha]

It looks like this has been renamed from node-npm-websocket_? I can't find where this comes from now though.

[nha]

should be ws-ev?

[ptaoussanis]

This will be addressed in the next release which I'll be pushing w/in the next hour. Closing.