Updated index.d.ts with ResourceType to ResourceTypes

[wmtst8]

Hi, a little out of the blue, but a request to make a change to the bundled index.d.ts file we had requested be added a couple of months ago. Thank you!

[tulios]

This change is backward-incompatible, we will have to keep both names for a while :/

[wmtst8]

Gotcha. I'll revert to ResourceType and add a second enum named ResourceTypes for now. Thank you.

[sklose]

do we really need both? the .d.ts is bundled with the kafkajs version itself, so it only needs to support that particular version of kafkajs. And in the latest version kafkajs is using ResourceTypes it seems according to docs/Admin.md.

const { ResourceTypes } = require('kafkajs')

await admin.describeConfigs({
  includeSynonyms: false,
  resources: [
    {
      type: ResourceTypes.TOPIC,
      name: 'topic-name'
    }
  ]
})

[Nevon]

What happens at runtime doesn't really matter, since the two types are equivalent anyway. The issue is how to make this change without people updating KafkaJS and finding that their build no longer works.

Just adding a second Enum with the correct name doesn't do anything. What we would want is to be able to add a new Enum and update all the references to return that, and then keep the old Enum around in such a way that old code will still compile.

I'm far from a TS expert, but I would expect that we would need to update the references to now be a union of ResourceTypes and ResourceType:

export enum ResourceType {
  UNKNOWN = 0,
  ANY = 1,
  TOPIC = 2,
  GROUP = 3,
  CLUSTER = 4,
  TRANSACTIONAL_ID = 5,
  DELEGATION_TOKEN = 6,
}

// wouldn't it be great if TS allowed you to extend string-based enums...
export enum ResourceTypes {
  UNKNOWN = 0,
  ANY = 1,
  TOPIC = 2,
  GROUP = 3,
  CLUSTER = 4,
  TRANSACTIONAL_ID = 5,
  DELEGATION_TOKEN = 6,
}

export interface ResourceConfigQuery {
  type: ResourceType | ResourceTypes
  name: string
  configNames: string[]
}

export interface DescribeConfigResponse {
  resources: {
    configEntries: ConfigEntries[]
    errorCode: number
    errorMessage: string
    resourceName: string
    resourceType: ResourceTypes | ResourceType
  }[]
  throttleTime: number
}

[sklose]

So the current enum is just broken with TypeScript because the .d.ts file is just for type definitions. Everything in there still needs some backing implementation that is available at runtime. If you do

import { ResourceType } from "kafkajs";
console.log(ResourceType.DELEGATION_TOKEN);

you will get

console.log(kafkajs_1.ResourceType.DELEGATION_TOKEN);
                                   ^
TypeError: Cannot read property 'DELEGATION_TOKEN' of undefined
    at Object.<anonymous> (/Users/s0k00sg/Work/sandbox/dist/kafkajs.js:26:36)
    at Module._compile (module.js:643:30)
    at Object.Module._extensions..js (module.js:654:10)
    at Module.load (module.js:556:32)
    at tryModuleLoad (module.js:499:12)
    at Function.Module._load (module.js:491:3)
    at Function.Module.runMain (module.js:684:10)
    at startup (bootstrap_node.js:187:16)
    at bootstrap_node.js:608:3

because this will transpile into the following JavaScript:

const kafkajs_1 = require("kafkajs");
console.log(kafkajs_1.ResourceType.DELEGATION_TOKEN);

[tulios]

Yeah, @sklose is right. The code in the .d.ts has to match what's in the JS parts. I will fix the file and merge the PR, thanks.

[tulios]

Thanks @wmtst8 @sklose.
We should have merged this much quicker, sorry for the confusion

[tulios]

Pre-release 1.11.0-beta.7 was published with the change.
Thanks again!

[wmtst8]

Thank you.