search

jarohen / chord

A library designed to bridge the gap between the triad of CLJ/CLJS, web-sockets and core.async.
439 stars 40 forks source link

Set WebSocket.binaryType to "arraybuffer". #17

Closed rosejn closed 10 years ago

rosejn commented 10 years ago

By default if binary data is received on a WebSocket it is handed to the client script as a DOM Blob object, which must be read asynchronously. If the binaryType option is set on the WebSocket then binary data is handed to the script behind an ArrayBuffer interface instead, which can more easily be read and decoded on the fly.

With this change in place, I've successfully added Fressian support to chord. I figured you probably don't want to have the base chord package rely on additional dependencies for Fressian in both Clojure and Clojurescript, so I put it in a separate package, now available here:

https://github.com/thinktopic/chord-fressian

Thanks for a nice library that just works.

rosejn commented 10 years ago

P.S. I'll need to change the chord dependency to a newer version if and when you do accept this, so for now you'll have to test by doing a lein install of chord locally with the change made.

jarohen commented 10 years ago

Hi Jeff - thanks for the PR! I've been looking at how to get Chord to support binary data for a while now - should have a chance to look over the PR at the weekend.

Cheers,

James

jarohen commented 10 years ago

Hi Jeff - I've merged the PR as requested and deployed 0.4.1. It's a really nice, simple change and chord-fressian seems to be a good extension - thank you! I've linked to it in the Chord README as well.

As for your question of whether to include Fressian in Chord - if I'm honest I've never been entirely sure about the balance of not depending on too many other libraries in any one library versus making it easier for users by only having one library to include. I'm happy to include this in Chord if you think it's better there, but also just as happy for you to keep it as a separate library if you'd prefer - what do you think?

Cheers,

James

rosejn commented 10 years ago

Cool!  Why don't we keep it separate, and then if we find many people are using the combo you can pull it into the base release.  Generally I think a la carte libs are much nicer than jumbo packages with everything included.

On Sat, May 24, 2014 at 11:00 PM, James Henderson notifications@github.com wrote:

Hi Jeff - I've merged the PR as requested and deployed 0.4.1. It's a really nice, simple change and chord-fressian seems to be a good extension - thank you! I've linked to it in the Chord README as well. As for your question of whether to include Fressian in Chord - if I'm honest I've never been entirely sure about the balance of not depending on too many other libraries in any one library versus making it easier for users by only having one library to include. I'm happy to include this in Chord if you think it's better there, but also just as happy for you to keep it as a separate library if you'd prefer - what do you think? Cheers,

James

Reply to this email directly or view it on GitHub: https://github.com/james-henderson/chord/pull/17#issuecomment-44089796

jarohen commented 10 years ago

Sounds like a good idea :)

Thanks again for the PR!

rosejn commented 10 years ago

Just updated the deps, bumped the version and deployed.

On Sun, May 25, 2014 at 7:24 PM, James Henderson notifications@github.comwrote:

Sounds like a good idea :)

Thanks again for the PR!

— Reply to this email directly or view it on GitHubhttps://github.com/james-henderson/chord/pull/17#issuecomment-44128703 .