Make BaseTarget parameters getters stricter #1534
Conversation
| @@ -39,7 +51,7 @@ export default abstract class BaseTarget implements Target { | |||
| isNotebookCell = false; | |||
| isWord = false; | |||
|
|
|||
| constructor(parameters: CommonTargetParameters) { | |||
| constructor(parameters: TParameters & CommonTargetParameters) { | |||
There was a problem hiding this comment.
This ensures that whatever type is declared actually ends up existing in the subclass constructor somehow.
There was a problem hiding this comment.
oh really? so the type signature of the base class constructor influences the type signature of derived class constructors? That's surprising to me. And I'm not sure how that works in the case where the derived class uses minimum parameters instead of common parameters
| return { | ||
| ...this.state, | ||
| leadingDelimiterRange: this.leadingDelimiterRange_, | ||
| trailingDelimiterRange: this.trailingDelimiterRange_, | ||
| insertionDelimiter: this.insertionDelimiter, |
There was a problem hiding this comment.
All this ceremony wasn't for nothing, it seems! I added the return type here for autocomplete.
There was a problem hiding this comment.
So you don't get autocomplete for free from the generic at the top / base class function? Not that I'm opposed to extra return type annotations 🤷♂️
There was a problem hiding this comment.
this is great. left a couple questions inline mostly for my own education 😄
I'm not totally in love with needing to specify that generic parameter, when you'd think TS would infer it from some function arg or something, but it doesn't, so I don't think we have a choice
btw elsewhere, I've made it so that it extracts what it needs from the type itself, and you just pass the type in as the only generic. we may consider going down that road if we have more things we need to infer. See eg
| * @template TParameters The constructor parameters. | ||
| */ | ||
| export default abstract class BaseTarget< | ||
| in out TParameters extends MinimumTargetParameters, |
There was a problem hiding this comment.
Can you explain why you used in out here?
Context
#1520 (comment)
What
This adds a generic to
BaseTarget, which is expected to receive the type of the subclass' parameters. This tries its best to enforce that is the case, and ensures implementations ofgetCloneParametersreturn that declared type.Checklist