Kafkajs: Include compression algorithms by default

[xamgore]

Now one has to write the following code to opt-in zstd compression, though it could be included in kafkajs by default via optional dependencies.

const { CompressionTypes, CompressionCodecs } = require('kafkajs')
const ZstdCodec = require('@kafkajs/zstd')
CompressionCodecs[CompressionTypes.ZSTD] = ZstdCodec()

P.S. Thank you for such a great package!

[ankon]

default via optional dependencies.

But this would still mean that npm will attempt to download the packages, and build them. Depending on how "complete" your build environment is this might lead to quite some additional time before it is determined that the build cannot go through. From a PoV of managing a CI environment I rather not have such dependencies.

Looking at the other side: The cost for setting up build environments, testing, and maintaining the dependencies right now is paid by users that require zstd (or snappy, or lz4, ...) compression, which IMHO is "fair".

The question therefore is a bit: Apart from Out-of-the-box experience improvements, what is the value of moving the dependencies (presumably lz4 and snappy, too)?

Edit: Thinking further about it: In case you're building a setup that _requires_ zstd support, you would still have to touch your dependencies to force the dependency on the codec library, because you now want your build to fail when something doesn't work anymore -- so in a way making these codecs optional dependencies actually creates a potential problem for you later at a time where you don't want to debug it :)

[Nevon]

I agree with Ankon. Making KafkaJS depend on the codecs is not gonna happen, as NPM will still install optional dependencies, meaning that anyone using KafkaJS would also get every single compression codec as a bonus (not to mention that we'd need to figure out a way to select which codec to use in case there are multiple options (such as native bindings vs pure-JS implementations).

However, it made me think about reversing the dependency. A codec could register itself if it has a peer dependency on kafkajs:

const MyGreatCodec = () => ({
  compress: async () => Buffer.alloc(0),
  decompress: async () => Buffer.alloc(0)
})

const register = () => {
  try {
    const { CompressionTypes, CompressionCodecs } = require('kafkajs')
    CompressionCodecs[CompressionTypes.ZSTD] = MyGreatCodec
  } catch (e) {
    throw new Error('Failed to register MyGreatCodec compression codec. Are you missing peer dependency "kafkajs"?')
  }
}

module.exports => {
  register,
  MyGreatCodec
}

Now, I'm not sure I think this is such a great idea. It would mean that every codec will have a slightly different registration method just by virtue of having different authors, and all we're really getting out of it is reducing a tiny, tiny amount of boilerplate.

[xamgore]

But this would still mean that npm will attempt to download the packages, and build them.

There is --no-optional flag to omit those. Does it solve your issue?

Apart from Out-of-the-box experience improvements, what is the value of moving the dependencies (presumably lz4 and snappy, too)?

• to reduce boilerplate code and keep it consistent with kafkajs
• to hide code that modifies the kafkajs internals, which is odd

[tulios]

You are not modifying internals; you are plugging a codec. I guess the interface to do that could be better, but realistically it is a one-line integration since the other two are basically imports. The --no-optional flag works, but I am certain that most users would forget about it and increase questions about codecs. I like the current setup because kafkajs is not concerned about the codecs, there is a simple but so far good enough interface to write the codecs, and on the codec side, it doesn't require any code from kafkajs.

We are not doing validating the codec interface before we call, we could override the setter and make sure the codec fulfills the required signature.