Kafkajs: 'offset' parameter in seek and others is "string" but docs use a number

[ankon]

I'm trying to work out the typing of the offset part of the seek call parameter: The documentation at https://kafka.js.org/docs/consuming#a-name-seek-a-seek shows this to be a number, which intuitively makes sense.

But, looking at the code https://github.com/tulios/kafkajs/blob/cef2676e32e773bbb2fbc0ebca51321dd03af70c/src/consumer/consumerGroup.js#L250-L261 it is annotated as string, which seems to go through the code base.

1. Which is "correct?"
2. And, of course: If this is a string, should be a string-that-can-be-converted-to-a-number, or is it completely opaque?

[tulios]

Good question, the offset is an int64, so technically it is a number. When using seek or other methods that receive the offset you can always use the number, but integers in javascript only go up to 2^53 - 1 which is less than the regular 2^63-1, a real int64. KafkaJS will always parse int64 fields from the protocol into strings to avoid any precision loss, it will also convert javascript numbers into "int64", and this converts the input into a buffer, that's why you can provide numbers or strings.

So, if you know that your offsets will never go above the javascript limit, you can use numbers, but I would recommend you always to use the string.

[ankon]

Great, thank you!

[JaapRood]

FYI: in case you might think you will run out of numbers, so need a string, but also what to do offset arithmetic, consider using long. It's what KafkaJS does to do so and gives all the basic operators you might need.

[ThisIsMissEm]

We could also use javascript's new BigInt type: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt

It's currently in stage 3 of ECMAScript specification, which means that it's being implemented in various javascript engines. It is available for Node.js >= 10

[JaapRood]

Cool! Node v8 LTS ends December 2019, so that's not too far away 🎉.

[tulios]

I tried BigInt some time ago, but it wasn't there yet, I have a branch somewhere here. I should look at it again; we would love to drop Long and have no dependencies.