Issue #26 - Add support for passing a different state encoder to expo… #28
Conversation
…xported encoding and decoding methods, and add a binary encoding strategy.
| * @param {encoding.Encoder} encoder | ||
| * @param {any} update | ||
| */ | ||
| static encodeState (encoder, update) { |
There was a problem hiding this comment.
I've moved these to static.
A different pattern we could consider here is to instead instantiate a copy of this class and store it on an exported constant. I'm happy to change to that if you'd prefer.
|
|
||
| export const testAwarenessWithBinary = testAwarenessWithEncoding(awareness.BinaryAwarenessStateEncoder, awareness.BinaryAwarenessStateEncoder) | ||
| export const testAwarenessWithDefault = testAwarenessWithEncoding(awareness.DefaultAwarenessStateEncoder, awareness.DefaultAwarenessStateEncoder) | ||
| export const testAwarenessBackwardsCompatDecoder = testAwarenessWithEncoding(awareness.DefaultAwarenessStateEncoder, undefined) |
There was a problem hiding this comment.
I've swapped the original "default" test I had, and instead replaced it with two tests to verify that the default encoder is used if none is passed into the methods.
| @@ -189,9 +237,10 @@ export const removeAwarenessStates = (awareness, clients, origin) => { | |||
| /** | |||
| * @param {Awareness} awareness | |||
| * @param {Array<number>} clients | |||
| * @param {typeof DefaultAwarenessStateEncoder|typeof BinaryAwarenessStateEncoder} stateEncoder The encoder to use for encoding and decoding each state entry | |||
There was a problem hiding this comment.
I don't use jsdoc types a lot, but my understanding is that typeof Blah is the best way to describe a constructor.
Another way to type this would be to instead type the "interface" expected - but that would make changes to the requirements of the encoder a breaking change.
|
Hey @dmonad, this PR is ready for a review when you have some time |
As requested, I've reopened the PR with some adjustments - in particular moving the encoding methods to be static on the classes so that a new instance is not required.