Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Deno? #850

Closed
viztastic opened this issue Aug 27, 2020 · 2 comments
Closed

Support for Deno? #850

viztastic opened this issue Aug 27, 2020 · 2 comments

Comments

@viztastic
Copy link

Is your feature request related to a problem? Please describe.
Trying to run KafkaJS with Deno.

Describe the solution you'd like
Given KafkaJS is a pure javascript solution, it would be amazing if we could make a few tweaks to have it compatable with Deno. Tried to load it via pika but getting the error below:

image

@ankon
Copy link
Contributor

ankon commented Aug 27, 2020

Given KafkaJS is a pure javascript solution, it would be amazing if we could make a few tweaks to have it compatable with Deno. Tried to load it via pika but getting the error below:

Looking at deno's list of differences/behaviors at https://deno.land/manual/introduction this doesn't look like "a few tweaks".

For example KafkaJS uses require, and switching that out to using import may require then to introduce a "build" step to keep things NodeJS compatible. Not impossible, but definitely something that needs a good reason to introduce, so far KafkaJS doesn't need any building.

Secondly KafkaJS does use NodeJS functionalities for handling the network communication with Kafka, and it seems that deno does have a completely different standard library for providing that. This then would mean minimally some conditional coding, or some code switching depending on whether you run with deno or with NodeJS. Not impossible, but definitely requires a strong use case.

So, what exactly is your use case here? :)

@Nevon
Copy link
Collaborator

Nevon commented Aug 28, 2020

I think it's an interesting idea, and I'm not necessarily 100% opposed to it in principle. What would motivate it would be the extremely large amount of unnecessary work that writing a Kafka client from scratch for Deno would be, when in reality 99% of the code is portable.

Let's investigate the cost:

  • Convert KafkaJS to use ES Modules (import/.mjs) and have a build step to convert it to CommonJS (require/.js)
  • Update our package definition so that people using ES Modules get the ES Module and CJS users get the CJS files. I think this is just a matter of adding some paths to our package.json.
  • Get rid of non-demo runtime dependencies (only long at the moment. Remove long.js in favor of BigInt #663 is in progress)
  • Deno's compatability layer currently does not support tls or net (std/node: add tls module denoland/deno#5391 & https://github.com/denoland/deno/issues/5385), so we would have to write a compatibility shim that would be swapped out at build-time (technically we would need conditional compilation regardless, but at least we wouldn't have to write the compatibility layer if they already supported that in Deno).
  • Update our pipeline to run with Deno as well as Node. At least a smoke test would be needed.
  • Update our pipeline to notify deno.land/x or whatever other CDN Deno users rely on.

Some issues that I'm not sure how to deal with:

  • Deno expects to just be able to download modules directly from a repo, which means that unless KafkaJS is Deno-native (which it will not be), we need to take the original sources, build them into something that's Deno compatible, and then store the output of that build in the repo itself. That is extremely ugly in my opinion. Maybe I have misunderstood something, but it really feels like building cross-compatible packages isn't intended by the Deno authors.
  • Not sure how we would achieve swapping implementations depending on the intended runtime. Granted, we could build a custom compiler just for this, something Akin to https://github.com/garronej/denoify, but that seems extremely out of scope.
  • We would never be able take on any new dependencies unless they are already Deno-compatible. Granted, we try to avoid runtime dependencies, but it's still a harsh limitation.

In addition to the above, there are also the additional running costs that we would incur:

  • We would have to support any Deno user that has an issue, despite us not actually using Deno ourselves.
  • The cost of our testing would increase.
  • Ongoing development cost would increase as we would now have to make sure that any new change is Deno compatible.

Looking at it in total, I think the chance that we can afford to do this is effectively 0%. Converting KafkaJS from a CommonJS module to an ES Module will probably have to happen at some point, as it seems ES Modules are kind of becoming the standard, but just doing that won't give us Deno compatibility.

However, KafkaJS is open-source, so what seems more realistic to me is to create a fork like @kafkajs/kafkajs-deno which maintains a small set of patches on top of KafkaJS to just swap out the Node specific parts. Realistically I think this is only doable after KafkaJS is converted to use ES Modules, as otherwise you'd need to patch every single file to get rid of require - otherwise you're back to a build-step and may as well just do it upstream.

I'm going to close this issue for now, as this is not something we are going to do at this moment. However, note that I am not saying we would never do it. The cost of doing it just has to come down.

This then would mean minimally some conditional coding, or some code switching depending on whether you run with deno or with NodeJS. Not impossible, but definitely requires a strong use case.

I don't think this is actually possible. ES Module imports are static, and I don't believe you can import anywhere other than at the top-level. If we want to import different modules, I believe we have to make a change at build-time. Something similar to how you in React-Native can have platform-specific files (foo.web.js, foo.native.js etc.).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants