Closed mihkeleidast closed 4 years ago
@risker Thanks for catching that
1) The registerOnChange
type does take a second optional param of 'intent'
. Looking at the code, that's the only valid argument there. It already defaults to 'input'
if undefined. unregisterOnChange
does not take a second param: https://github.com/ten1seven/what-input/blob/master/src/scripts/what-input.js#L463-L476 Is there something else wrong with those types?
2) Agree!
@greypants what i meant was the first param callback type. currently it's callback: () => void
but it should be callback: (type: InputType) => void
. That's the documented behavior that we're using: https://github.com/ten1seven/what-input#custom-callbacks.
BTW regarding the second param I think the types should allow input
as well since it's the default value, there's really no reason why specifying that as the second param would be incorrect.
@ten1seven Did I infer correctly from the assignment of this issue that you want to tackle this yourself? Or can I file the PR myself? It's quite a small fix so I'd like to see this fixed as soon as possible.
@mihkeleidast I removed myself from the assignment. Please go ahead and submit a PR. Thank you!
Updated today and tried to use the new TypeScript definitions. Ran into two issues:
registerOnChange
andunRegisterOnChange
callback parameter types are faulty - in reality they have atype
param as well.The input type string union could be exported as a separate type - then we could reuse the type in our own code. Additionally, it should include
'initial'
since that's the initial value before that what-input returns before initializing.Happy to file a PR in a day or so if noone beats be to it, filing so I can point a TODO in my own code at something.