search

tulios / kafkajs

A modern Apache Kafka client for node.js
https://kafka.js.org
MIT License
3.75k stars 527 forks source link

`type` vs `interface` in the TypeScript types #952

Open ankon opened 4 years ago

ankon commented 4 years ago

Is your feature request related to a problem? Please describe.

Right now types/index.d.ts looks somewhat inconsistent: Some types are defined with type X = { ... }, while others are defined as interface Y { ... }.

Chosing one over the other isn't just a stylistic choice though, there is at least one important difference: Interfaces can merge, while types cannot. See https://www.typescriptlang.org/docs/handbook/declaration-merging.html for details.

Looking at index.d.ts then it doesn't seem like the choice was made deliberately, but rather more adhoc.

Essentially this boils down to the following question: "Is merging of the types exposed by KafkaJS with other types configured an acceptable behavior?"

Argument for that (i.e. using interface instead of type): Merging would allow users to cure at least some type errors/deficiences in the KafkaJS by providing a suitable .d.ts file. This won't work for everything though, for example one could add missing properties, but not change the type of existing ones.

Argument against that (i.e. preferrring type): The types exposed by KafkaJS are really what is supposed to be exposed, and if there are problems with these then they need to be fixed in KafkaJS itself. Users could always use explicit type casts to adjust types if needed, which would allow pretty much anything.

Describe the solution you'd like

Personally I find interface easier to read and reason about, and the ability to augment types seems like "nice to have". On the other hand though there shouldn't be many cases where the user actually needs to declare a new method on a KafkaJS type, as nowadays this file is updated fairly consistently when APIs change, and existing type issues do get fixed quite quickly.

WDYT?

To be clear: "let's not open this can of worms" is a good answer too. Minimally this issue could serve as a reminder to reviewers of changes to index.d.ts that there is a difference between these two things. :)

tulios commented 4 years ago

I am with you. I also prefer interfaces, but the bulk of the types came from contributions, and we chose not to make it unnecessarily harder for the folks' contributing, but we could start to reshape the types now.