What is the signature of form fields

[NicholasBoll]

💬 Questions and Help

So far, our form field components have been extending <input> or <select> which have the following signature (not Field exactly, but have this shape):

interface Field<T = HTMLElement> {
  value: string;
  onChange: ChangeEventHandler<T>
}

ChangeEventHandler<T> roughly has a signature like: (event: Event) => void where the value is in event.target.value.

Many form libraries, including Formik, assume this is the signature of the Field you give it. A controlled field sets the value to the event.target.value.

There are components, like the Select (#294) and ColorPicker (#376) that don't wrap input elements, so the temptation is to have a signature like onChange: (value: string) => void. Some form libraries like Redux-form can handle signatures like this.

We have 2 options:

1. Always match the ChangeEventHandler<T> interface and create a utility function that is able to dispatch synthetic events with a string value.

interface Field<T = HTMLElement> {
  value: string;
  onChange: ChangeEventHandler<T>
}

Pros:

• All fields have the same interface. A Select component could wrap a <select> element or a <button> element - the interface ensures this is a decision that is up to the implementor. They are interchangeable.
• Easy integration with any form library.

Cons:

• Trickier to implement.
• event.preventDefault() doesn't have much meaning. Are these events even cancellable? Not quite the same as a real event.

2. Allow onChange to have a parameter that is either a value or an event

interface Field<T = HTMLElement, U = string> {
  value: U;
  onChange: ChangeEventHandler<T> || (value: U) => void
}

Pros:

• Easier to fire an onChange with any value shape.
• No "fake" event.

Cons:

• Harder to integrate with form fields or form libraries. They have back-doors, but those are often unintuitive.
• More complicated to consume. Changing a component from an <input> element to something else (for whatever reason) is a breaking change.

[NicholasBoll]

We already do some interesting things on ComboBox. The autocomplete combobox can be updated by the user typing into the input element, but also updated by the user clicking or selecting a menu option. The code creates a fake event in this case to normalize the onChange interface for outside consumers.

https://github.com/Workday/canvas-kit/blob/25e16680773c44760b325a4fea9373af178ae46e/modules/_labs/combobox/react/lib/Combobox.tsx#L158-L185

[donovangini]

Noticing something similar here when we are uptaking Checkbox. Chatted with @anicholls, we thought it would make sense to bring up here

https://github.com/Workday/canvas-kit/blob/1d4e3d4ffc27d7743c4bfb33b67e3d7fba473758/modules/checkbox/react/lib/Checkbox.tsx#L47

The typing of (e: React.SyntheticEvent) => void; is throwing typing errors even when we pass in a ChangeEventHandler, causing us to have to cast our method to use it in Checkbox. For example:

const onChange: ChangeEventHandler<HTMLInputElement> = event => {
    doSomething(event.target.checked);
  };

return (<FormCheckbox onchange={onChange as (e: SyntheticEvent) => void} />

Given that ChangeEvents are SyntheticEvents, this seems pretty weird... Also, I'm not sure how many other kinds of events we'd want to accept from Checkbox's perspective, so changing to ChangeEvent might make sense...

[ziel5122]

Would be nice to have Checkbox extend the onChange from InputHTMLAttributes onChange like TextInput does https://github.com/Workday/canvas-kit/blob/1d4e3d4ffc27d7743c4bfb33b67e3d7fba473758/modules/text-input/react/lib/TextInput.tsx#L6-L9

[NicholasBoll]

@donovangini Oh, I think that is a bug on our part. I think the signature of the checkbox should be:

onChange?: React.ChangeEventHandler<HTMLInputELement>

That's the signature of the HTMLInput that Checkbox extends from. I'm not sure why that was explicitly specified to be different...

[NicholasBoll]

@donovangini @ziel5122 I opened an issue: #496

If you have time, feel free to open a PR to fix this.

[ziel5122]

Awesome. One of us can take care of it.

[lychyi]

@donovangini Thanks for the workaround. What's the version of React and @types/react are you're using? Also, can you post the error if you don't do a type assertion?

[donovangini]

Types:

"react": "16.12.0",
"@types/react": "^16.9.15",

Note: our yarn.lock locks at 16.9.17

Error:

Type '(event: ChangeEvent<HTMLInputElement>) => void' is not assignable to type '(e: SyntheticEvent<Element, Event>) => void'.
  Types of parameters 'event' and 'e' are incompatible.
    Type 'SyntheticEvent<Element, Event>' is not assignable to type 'ChangeEvent<HTMLInputElement>'.
      Types of property 'target' are incompatible.
        Type 'EventTarget' is not assignable to type 'EventTarget & HTMLInputElement'.
          Type 'EventTarget' is missing the following properties from type 'HTMLInputElement': accept, align, alt, autocomplete, and 288 more.ts(2322)
Checkbox.d.ts(10, 5): The expected type comes from property 'onChange' which is declared here on type 'IntrinsicAttributes & Props & { children?: ReactNode; }'

[jaclynjessup]

Closing this issue - We've resolved several of the issues that have been linked & this has been inactive for 3 years.