search

taoensso / sente

Realtime web comms library for Clojure/Script
https://www.taoensso.com/sente
Eclipse Public License 1.0
1.74k stars 196 forks source link

fixing a bug in arraybuffer binary data unwrapped v2 #428

Closed rosejn closed 1 year ago

rosejn commented 1 year ago

Hey, Thanks for getting the binary support updates I made merged into mainline sente! I've finally gotten back around to moving our platform over to your latest release, and I found one minor bug that pops up when using arraybuffers (msgpack) for the binary packed format. The sente/parse-packed function is expected to always return a tuple of packed + format, but in the non string case it is only returning the packed value. With this commit everything works as expected, although I'm not 100% familiar with the v1/v2 wrapp/unwrap scheme you are using for this transition so this might not be quite write. I'm thinking maybe the (string? packed) should be the first clause in the cond, and then it should return [packed :v2/unwrapped], but maybe there are string versions that also need to do that?

ptaoussanis commented 1 year ago

@rosejn Hi Jeff, thanks for catching this - and for the fix! Your fix looks good to me 👍 Apologies for the trouble.

Just taking care of one more PR, then will update the v1.18 snapshot. I'll ping back here when it's done 👍

ptaoussanis commented 1 year ago

Merged manually, [com.taoensso/sente "1.18.0-SNAPSHOT"] is updated on Clojars 👍 Cheers :-)

rosejn commented 1 year ago

Brilliant, thanks Peter!