Add type field to Message and Update

[wojpawlik]

https://www.typescriptlang.org/docs/handbook/unions-and-intersections.html#discriminating-unions

[levlam]

This field is redundant in general. You can do that in Telegraf.JS on a framework level if you need this,

[wojpawlik]

Extra code running on every update, for straying away from the API? I'll pass.

[levlam]

Extra redundant data sent to everyone over network would be much worse, so if it is needed for TypeScript, it can be added there.

[wojpawlik]

error_code and ok already duplicate info available as HTTP status code.

[wojpawlik]

And to comply with the API, any extra field I'd add would have to be inaccessible to library users, limiting its usefulness.

[levlam]

error_code and ok already duplicate info available as HTTP status code.

Not really. HTTP is used as a transport and API should be possible to use with different trasports. Moreover, special error_code values for specific errors can be introduced in the future and there can be transport layer errors, which result in a valid HTTP response without payload.

[wojpawlik]

I feel like it's just exposing information you already have, in a more convenient format. I didn't expect push-back.

Let me try to make a proper case:

These extra fields would help users narrow down the update type (telegraf/telegraf#1319 (comment)).

They'd also help TypeScript: since "type" cannot equal "message" and "callback_query" at the same time, tt.Update.MessageUpdate & tt.Update.CallbackQueryUpdate becomes contradictory (never), so it's safe to simplify tt.Update & tt.Update.MessageUpdate into tt.Update.MessageUpdate.

Adding the discriminants on my end is not worth the hassle:

I cannot simply name it "type", because if you decide to add "type" as well, with different semantics, I'd be forced to make a breaking change. So I'd have to use a symbol property.

It's actually impossible to programmatically determine update type in a future-proof way. I'd have to use a allowlist or denylist of fields, which would get outdated with new API versions and break existing bots.

It's even worse for Message, since it has way more fields, and it's nested at one of 4 places (more in the future?) within Update.

[levlam]

These extra fields are redundant. For example, we wouldn't add separate fields Message.inline_keyboard and Message.keyboard additionally to Message.reply_markup, because that would be a lot of redundant data, even the fields can be useful for someone. But they can be easily added on a bot's side.

I cannot simply name it "type"

You can name it "message_type"/"update_type" or "_type". These names are safe.

It's actually impossible to programmatically determine update type in a future-proof way.

You don't need to do that. You wouldn't be able to match them to a specific type anyway. You can mark all messages without content or with content of unknown type as "unknown" or "other".

[wojpawlik]

You can name it "message_type"/"update_type" or "_type". These names are safe.

_type is good. Is there a general rule for what names are safe?

You wouldn't be able to match them to a specific type anyway.

I would, actually. Types can be updated without updating the lib. Exceptions: new API methods, and hardcoded UpdateTypes allowlist, which could be moved to typegram in a breaking release, or eliminated if this is merged.

[levlam]

_type is good. Is there a general rule for what names are safe?

No, but Bot API never had fields beginning with underscore.
It is also very unlikely that a type X will have a new field with prefix X in the future.
Also, all fields are snake_case, so any name not in snake_case is also safe.

[wojpawlik]

Adding a phantom discriminator didn't fix my issue actually... thank you for your time.