disnake
disnake copied to clipboard
refactor: Improve webhook typing.
Summary
Fixes #626
This is a typing-breaking change.
Should Webhook.edit and Webhook.delete also be overloaded with a Never somewhere to forbid calling them (you can't delete or edit an interaction webhook, right?)?
Checklist
- [x] If code changes were made, then they have been tested
- [x] I have updated the documentation to reflect the changes
- [x] I have formatted the code properly by running
pdm lint - [x] I have type-checked the code by running
pdm pyright
- [x] This PR fixes an issue
- [ ] This PR adds something new (e.g. new method or parameters)
- [ ] This PR is a breaking change (e.g. methods or parameters removed/renamed)
- [ ] This PR is not a code change (e.g. documentation, README, ...)
It's not quite clear whether this is meant to be a typing or runtime change; followup is annotated as returning an InteractionFollowupWebhook, but returns a "plain" Webhook in practice. Types that are only used in annotations can be confusing to some people, so it might be better to actually use this type at runtime too?
Making it actually return an InteractionFollowupWebhook will make it a runtime breaking change. I intentionally avoided that "by default" but if it's okay in this case i'll willingly do this!
Making it actually return an
InteractionFollowupWebhookwill make it a runtime breaking change. I intentionally avoided that "by default" but if it's okay in this case i'll willingly do this!
ah, gotcha. :thinking:
In that case, what about moving InteractionFollowupWebhook.send into a type-checking block as well? Similar to Bot.__init__, such that the runtime implementation is inherited from the parent class, while being able to adjust the (typing-only) method signature freely.
Making it actually return an
InteractionFollowupWebhookwill make it a runtime breaking change. I intentionally avoided that "by default" but if it's okay in this case i'll willingly do this!ah, gotcha. 🤔 In that case, what about moving
InteractionFollowupWebhook.sendinto a type-checking block as well? Similar toBot.__init__, such that the runtime implementation is inherited from the parent class, while being able to adjust the (typing-only) method signature freely.
That's a nice idea, thanks!
Done in ec9a610259e3a4aa1c4eaa50ecdbcfe0ab81af72!
having to subclass something to change a return type annotation for specific situations is rather awkward
This is why those situations should be working on distinct types. Not a subclass of one handle-all type, as it later shows the interface allows for more than is actually supported.
having to subclass something to change a return type annotation for specific situations is rather awkward
This is why those situations should be working on distinct types. Not a subclass of one handle-all type, as it later shows the interface allows for more than is actually supported.
So you suggest something like making InteractionFollowupWebhook the base type and Webhook as sub-type?
So you suggest something like making
InteractionFollowupWebhookthe base type andWebhookas sub-type?
I suggest making InteractionFollowupWebhook one type and Webhook another type.
Only then consider creating some base/mixin class.
This is why those situations should be working on distinct types. Not a subclass of one handle-all type, as it later shows the interface allows for more than is actually supported.
Yeah. A bunch of the class/instance methods on Webhook don't work or make sense in the context of application webhooks. Splitting it into separate types would be great, but is simultaneously the most breaking change