darrachequesne / notepack

A fast Node.js implementation of the latest MessagePack spec
MIT License
75 stars 19 forks source link

Support toJSON as a fallback on non-object types (BigInt) #28

Closed papandreou closed 2 years ago

papandreou commented 2 years ago

Hi!

My application uses BigInt heavily, which results in Could not encode with this library. I tried to follow this piece of advice and specified a BigInt.prototype.toJSON. Unfortunately that doesn't work, because the typeof operator returns bigint, and this library only supports toJSON for values where typeof returns object.

I saw https://github.com/darrachequesne/notepack/pull/25 but thought it'd be worthwhile to consider a more minimalistic approach.

Drive-by: Don't test with node.js 18, as that seems to break Travis. (Should probably switch to a different CI provider)

darrachequesne commented 2 years ago

That sounds reasonable, thanks :+1:

Merged as https://github.com/darrachequesne/notepack/commit/9ec1fcde640dfb57b69ff5f8b45ffa5439a87700.

papandreou commented 2 years ago

@darrachequesne, thanks a lot! Mind doing a new release? 🤗

darrachequesne commented 2 years ago

Here we go: https://github.com/darrachequesne/notepack/releases/tag/3.0.0

papandreou commented 2 years ago

@darrachequesne, thanks a lot! I'm using this library via @socket.io/redis-emitter, which depends on notepack.io@~2.1.0: https://github.com/socketio/socket.io-redis-emitter/blob/d62af2c9ec45cf991cb32c286c511354f11ea0b1/package.json#L23

I see your name on that library as well, so just putting in a good word for a new release that bumps that dependency as well 🙏 😅

papandreou commented 2 years ago

@darrachequesne, thanks a lot! I'm using this library via @socket.io/redis-emitter, which depends on notepack.io@~2.1.0: https://github.com/socketio/socket.io-redis-emitter/blob/d62af2c9ec45cf991cb32c286c511354f11ea0b1/package.json#L23

I see your name on that library as well, so just putting in a good word for a new release that bumps that dependency as well 🙏 😅

Opened a PR here for that update: https://github.com/socketio/socket.io-redis-emitter/pull/113