grammyjs / types

Type declarations of the Telegram Bot API.
MIT License
39 stars 5 forks source link

add languageCode type #45

Closed carafelix closed 4 months ago

carafelix commented 5 months ago

Would be helpful for getting autocompletion in the commands plugin while localizing commands, could potentially improve readability in other places of the codebase.

userCommands.command("start", "example", () => {})
            .localize('<AUTOCOMPLETES>')

reference: https://www.loc.gov/standards/iso639-2/php/code_list.php https://en.wikipedia.org/wiki/List_of_ISO_639_language_codes https://core.telegram.org/bots/api#setmycommands <-- spec in telegram

carafelix commented 5 months ago

Should I declared this just locally for the commands plugin or is it better intended here?

roziscoding commented 5 months ago

I do not like that you used a TS enum, they are basically not much more than a footgun and since we do not expose any of its functionality, we might as well directly go with

export type LanguageCode =
  | "ab" // Abkhazian
  | "aa" // Afar
  // ...

I like the idea of not going with an enum, but I think having these at runtime would be useful so users don't need to look for the correct code for the language they're using, so I'd suggest we go with

const LanguageCodes = {
  "Abkhazian": "ab",
  "Afar": "aa",
  // ...
} as const;

export type LanguageCode = typeof LanguageCodes[keyof typeof LanguageCodes];
carafelix commented 5 months ago

I think having these at runtime would be useful so users don't need to look for the correct code for the language they're using.

Yeah I was thinking exactly that. I did the same thing with an object in a gist. Could any of you explain why is not proper to use an enum for it?

KnorpelSenf commented 5 months ago

I think having these at runtime would be useful

So far, it's been a strict rule that this package exposes types only, and 0 bytes of executable code. We only really use .ts files because .d.ts was hard to import on Deno back then.

It's mostly for historic reasons that I'm not comfortable with this, but historic reasons are bad reasons, so please give me a few days to change my mind or to come up with a better solution :D

KnorpelSenf commented 5 months ago

Could any of you explain why is not proper to use an enum for it?

There are many articles on the web that explain this better than I could in a GitHub comment. One of them is https://dev.to/ivanzm123/dont-use-enums-in-typescript-they-are-very-dangerous-57bh.

carafelix commented 5 months ago

Changed to using an object. The re-request was not intended tho, do not pressure yourself

carafelix commented 5 months ago

Examples of the readability benefiting from adding the Language Code runtime object. It's more clear for languages that their respective code does not resemble their name in English or do not have the same initials :

const i18n = new I18n<MyContext>({
  defaultLocale: LanguageCodes.German, // Instead of 'de'
  directory: "locales",
});
i18n.loadLocaleSync(LanguageCodes.Greek, { // Instead of 'el'
  source: `greeting = Γειά σου { $name }!
language-set = Λορεμ ιπσθμ δολορ σιτ αμετ`,
});
cmds.command("duke", "_", () => {})
                .localize(LanguageCodes.Croatian, "vojvoda", "_") // instead of 'hr'
bot.hears('something', (ctx) => {
  ctx.api.setMyShortDescription('快乐机器人', LanguageCodes.Chinese) // instead of 'zh'
  ctx.api.setMyName('robot sásta', LanguageCodes.Irish) // instead of 'ga'
})