mcollina/msgpack5

`instanceof bl` does not work across versions of `bufferlist`

everett1992 opened this issue · 2 comments

msgpack5 has a dependency on bl@^4.0.0
If a msgpack5 consumer passes a BufferList from a different bl module

https://github.com/mcollina/msgpack5/blob/master/lib/decoder.js#L45-L48

msgpack5 uses instanceof require('bl') to check if it's argument is a BufferList which will return false if the consumer resolves a different bl package. This happens when msgpack5 and the consumer depend on different versions of bufferlist.

node_modules/
  bl/ , require('bl') in consumer code
  msgpack5/
    node_modules/
      bl/ <- require('bl') in msgpack5 code

When I upgraded my applications bl to 5 tests started failing because the BufferList passed to decode was not consumed.

const { decode, encode } = require('msgpack5')()
const bl = new BufferList([encode(0), encode(1), encode(2)])
assert(bl.length === 3)
decode(bl)
// fails after upgrading to bl@5.
assert(bl.length === 2)

I have a few suggestions

  • move bl to a peer dependency
  • use bl@5's BufferList.isBufferList method which uses a symbol on the bl instance and should work across versions.

The latter would be better! Would you like to send a PR?

Sent #105 before I saw this response, I did both but I can back out the peerDep change. Why not use a peer dep?