StrictTextAreaProps has a type that includes number and undefined for the "value" field

[skondrashov]

Bug Report

Steps

<Form.TextArea onChange={(_: unknown, data: TextAreaProps) => {
  const s: string =  data.value;
}}/>

Expected Result

No TypeScript errors.

Actual Result

Type 'string | number | undefined' is not assignable to type 'string'.

Version

2.1.5

Testcase

https://codesandbox.io/p/sandbox/semantic-ui-react-forked-5tf5yh

Notes

It doesn't appear to be possible to actually set the value of the textarea to a number or undefined, so it's not clear why I'd have to cast, and it's very inconvenient to cast undefined to a String since String(undefined) is "undefined" instead of "".

[welcome[bot]]

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you've completed all the fields in the issue template so we can best help.

We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

[layershifter]

@skondrashov good call, feel free to submit a PR 😉

[skondrashov]

I looked into this in detail, and I think I understand the problem now, but I don't know what kind of solution would be acceptable.

Semantic UI components take callbacks with a signature of:

(event, data) => void

Where event is some kind of event object, and data is generally just the props that were passed into the component reflected back through the callback. In the cases where the component has a value which updates, the data object has some fields added or overwritten, and the user is supposed to pull the value out of the data.

From my perspective, this is a design oversight - in the case of TextArea, for example, the concept of a "value" which is passed into the component is different from the concept of the "value" which the component returns to the user when the user types something into it. Here is the relevant runtime code for TextArea:

_.invoke(props, 'onChange', e, { ...props, value: newValue })

props always already contains a value, since value is a required prop, and this code overwrites that value (which can be undefined, or a number) with a string. But if the purpose of the data parameter is to make the props that were passed in available to the function, then in this case the value prop was overwritten, and essentially lost altogether, and the parameter cannot fulfill its purpose.

It seems that conceptually it is a mistake to call this parameter "data" and to add things into it during callback execution. Instead, it makes sense to call this parameter "props", and add a third parameter called "data" or "value". It is possible to sort of fix this by creating a type that extends the props passed into the component and specifies the data coming back out of the element, but it makes much more sense to me to keep those things conceptually separate - there is an event that the user triggers, there is a props object that the parent code passed to the component, and there is data that the component returns to the parent code. Meshing the latter 2 into one object seems like a flaw, and that flaw is brought into focus in this instance.

I'm happy to make/update my PR with whatever is best and to try to cover all of the components, but I know that my idea of creating a third parameter would be a breaking change, so I would like to know what makes sense.

code bits for context

The onChange callback for TextArea says that the data is just the props, and yet splices a value in that does not match the value of the props (what the OP is about):

onChange?: (event: React.ChangeEvent<HTMLTextAreaElement>, data: TextAreaProps) => void

The onChange callback for Checkbox also says that data is just props, and yet inserts two values into that object which are not explicitly defined in the types at all (they rely on a [key: string]: any definition instead to avoid errors):

onChange?: (event: React.FormEvent<HTMLInputElement>, data: CheckboxProps) => void

the code that inserts new values:

_.invoke(props, 'onClick', e, {
  ...props,
  checked: !checked,
  indeterminate: !!indeterminate,
})

And finally Dropdown does a similar thing, with the type looking like this:

onChange?: (event: React.SyntheticEvent<HTMLElement>, data: DropdownProps) => void

and the insertion of data into the props looking like this:

_.invoke(this.props, 'onChange', e, { ...this.props, value })

which again clashes with an existing value prop:

value?: boolean | number | string | (boolean | number | string)[]

which is a type for what the component can receive, not the type for what the component returns back out.

to summarize

I'd like to change all callback type definitions to look like this, whenever the component has some internal value it needs to make available upstream:

onChange?: (event: React.SyntheticEvent<HTMLElement>, props: DropdownProps, value: DropdownValue) => void

and to change the corresponding js code to be like this:

_.invoke(this.props, 'onChange', e, this.props, value)

but these would be breaking changes, pretty involved, and have some details to figure out. I'm ready to do it, but let me know what approach I should take!

[skondrashov]

I updated my PR, it's not all of the changes I would need to make to go this route - I haven't touched dropdown yet and I would need to make changes in type definition files even for components that don't need js changes, but just to show what they would look like.

[skondrashov]

PR updated and contains all the necessary changes, as far as I can see. They're breaking changes, but I think they're necessary (at least in some similar form) to make the API clean and easy to use with TS.

[XZanderU]

Verificación de tipo (typeof): Usamos typeof para asegurarnos de que data.value sea una cadena. Si es un número, lo convertimos a string usando String(). Control de undefined: Si data.value es undefined, le asignamos una cadena vacía (''), evitando posibles errores. De esta forma, aseguras que la variable s siempre sea una cadena, independientemente del valor de data.value, y eliminas el error de TypeScript.