Node-rdkafka: index.d.ts ConsumerStreamMessage key: string type is wrong

[sidiliu]

in "index.d.ts" file
declare interface ConsumerStreamMessage {
value: Buffer,
size: number,
topic: string,
offset: number,
partition: number,
key?: string,
timestamp?: number
}
"key?: string" should be "key?: Buffer"?

[codeburke]

Seems like it should be an optional string or Buffer, right?

[sidiliu]

I only get Buffer.
I think it should be Buffer, just like "value: Buffer". Since we do not know the encoding of "key" either.

[pwillcode]

I am getting the same behavior.
I'm providing the key as a string when I produce a message, but I think Avro turns it into a Buffer.
I think string keys are possible if you're not using Avro (Buffer support had to be added at one point.)

This is my current workaround for handling ConsumerStreamMessages in TypeScript:

consumer.on("data", (message: ConsumerStreamMessage) => {
          const key: Buffer | string | undefined = message.key;
          const keyString: string | undefined = (key && (key as any) instanceof Buffer) ? key.toString() : key;
          ...
});

[codeburke]

Thanks @pwillcode . According to https://github.com/Blizzard/node-rdkafka/blob/master/src/producer.cc#L397-L424 the key can be either a Buffer | string | null | undefined. Since nothing else in the this library assumes Avro encoding/buffers then we should make sure the TS types represent all the options for the API.

Instead of copy/pasting this long optional list, what if we create an alias type like the following?

type KeyType = Buffer | string | null | undefined;
declare interface ConsumerStreamMessage {
    value: Buffer,
    size: number,
    topic: string,
    offset: number,
    partition: number,
    key?: KeyType,
    timestamp?: number
}

I'm not TS guru, so I'm not sure if it should include the Type suffix or not. Feedback welcome.

[pwillcode]

@codeburke That seems good to me. I'm not a TS guru either, but I would personally leave off the Type suffix.

[codeburke]

I've opened a PR with the discussed changes for review. Feel free to comment.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[sam-github]

Didn't https://github.com/Blizzard/node-rdkafka/pull/732 fix this?