[GitHub] [pulsar-client-node] alanhoff commented on pull request #127: feature: allow messages and producers to handle int64 sequences

[GitBox <gi...@apache.org>]

alanhoff commented on pull request #127:
URL: https://github.com/apache/pulsar-client-node/pull/127#issuecomment-751349897


   @hrsakai @sijie sorry for the delay here, let's start by correcting some of my misconceptions: 
   
   JS do support ints higher than 32 bits when representing them as a [`Number`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER), but it *may* start losing precision for ints above 2<sup>53</sup> - 1 (or lower than the negative equivalent). Example:
   
   ```javascript
   const x = Number.MAX_SAFE_INTEGER + 1;
   const y = Number.MAX_SAFE_INTEGER + 2;
   
   console.log(x === y); // will output true
   ```
   
   That said, we're "kinda" safe at the moment since it's the native addon that's [incrementing the sequence](https://github.com/apache/pulsar/blob/1cf29f244a783f62aa6e4b6860135758e0f8034c/pulsar-client-cpp/lib/ProducerImpl.cc#L65) using a proper `int64_t`, however IMHO I don't think the JS client should be using the `Number` primitive to represent an `int64_t` since that's unsafe.
   
   The correct thing to do would be changing all sequences so they accept a [BigInt](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt) instead of a Number, but it would be a breaking change and it wouldn't support Node.js < `10.4.0`. So in the meanwhile this PR allows devs to optionally pass sequences as strings which are being casted into `int64_t` by the addon without losing precision. 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org