Remove Lint Exceptions: Prop Type Exceptions#10521
Conversation
charleyf
changed the title
charleyf
changed the title
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| const defaultRegisterField = (string) => undefined; | ||
| const DEFAULT_PROPS = { | ||
| toPreviousStep() {}, | ||
| onChange() {}, | ||
| registerField() {}, | ||
| registerField: defaultRegisterField, |
There was a problem hiding this comment.
Could we instead update registerField to support void as a supported return type?
➡️
) => undefined | void | RefCallback<HTMLElement>;There was a problem hiding this comment.
I removed the inline exceptions, but adding void didn't seem to work.
Unfortunately adding void as a return type causes more type issues than it solves. An example:
// `app/javascript/packages/address-search/components/address-input.tsx`
<TextInput
required
ref={registerField('address')} // This is either undefined or `ForwardedRef<HTMLInputElement>` adding 'void' here seems like a bad idea.There was a problem hiding this comment.
Oof. Okay, in that case, maybe we could at least inline the default?
registerField: () => undefined,There was a problem hiding this comment.
Much better. For some reason I couldn't get the inline function to work before, but now it works. 👍
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
Couple minor comments, but LGTM 👍 Glad to be starting to chip away away at some of these exceptions!
| import UploadContext from '../context/upload'; | ||
|
|
||
| function InPersonLocationPostOfficeSearchStep({ onChange, toPreviousStep, registerField }) { | ||
| interface InPersonLocationPostOfficeSearchStepProps { |
There was a problem hiding this comment.
Elsewhere we extend an interface exposed by FormSteps which helps avoid repeating the type signatures, and also surfaces additional props that are available. Maybe we should do the same here and in InPersonPrepareStep and InPersonLocationFullAddressEntryPostOfficeSearchStep?
| toPreviousStep() {}, | ||
| onChange() {}, | ||
| registerField() {}, | ||
| registerField: (_string) => undefined, |
There was a problem hiding this comment.
If we're going to list the unused argument, I'd think we should name it the same as it's named in the original type signature (field). There's also technically an options parameter as well if we wanted to be exhaustive.
Personally, I don't think it matters much to list the arguments, which is why I left it out in my previous recommendations.
identity-idp/app/javascript/packages/form-steps/form-steps.tsx
Lines 33 to 34 in c386364
2 participants
No description provided.