mantinedev / mantine

A fully featured React components library
https://mantine.dev
MIT License
26.92k stars 1.9k forks source link

Select - loses initial value when value is a Number and not String #140

Closed cstrat closed 3 years ago

cstrat commented 3 years ago

Reproduction: https://codesandbox.io/s/mantine-select-strange-behaviour-fh9zd?file=/src/App.js

See what I mean in this GIF: 2021-07-07 20 38 48

Strange edge case...

rtivital commented 3 years ago

This is not actually an edge case, Select supports only string values and will not work with numbers, it's made this way to support both controlled and uncontrolled variant – https://github.com/mantinedev/mantine/blob/master/src/mantine-core/src/components/Select/Select.tsx#L140-L146

This is not obvious though and I'll add more information about data structures to Select docs, thanks for reporting

cstrat commented 3 years ago

I am trying to understand why it matters whether its a string or not for controlled -vs- uncontrolled. I can see you're using the hook and have that rule, but why limit it to only strings? Wouldn't the rule only need to check if val !== undefined to know if its a controlled v. uncontrolled input?

rtivital commented 3 years ago

No, it's Select component which is controlled or uncontrolled, not string. val !== undefined will introduce new set of similar issues, for example, when value will be a date or an array From my experience it's much easier to use one type for Select state management values and then convert it outside of component to preferred type as:

cstrat commented 3 years ago

No, it's Select component which is controlled or uncontrolled, not string.

Sorry yes I understand that. If I understand it correctly, the hook allows users to build their own form elements and have them be used in both a controlled & uncontrolled fashion. Maybe I don't fully understand what rule is supposed to be doing.

https://github.com/mantinedev/mantine/blob/master/src/mantine-hooks/src/use-uncontrolled/use-uncontrolled.ts#L27-L30

If its a controlled field, but the value doesn't pass the rule, it still updates the value (like my example), but the UI just misbehaves. The useEffect doesn't do anything if the rule fails. Shouldn't it revert the value to the previous value which passed the rule? It sounds like for the Select you don't want anything but string as the value type... which I am sure is what the HTML spec allows for, so using number for value is probably the wrong thing anyway! 🤣

cstrat commented 3 years ago

I will just update my form to use the number as a string and cast it to number where I need to, no big deal really.

rtivital commented 3 years ago

Yep, I guess it's a small bug in use-uncontrolled hook – https://github.com/mantinedev/mantine/blob/master/src/mantine-hooks/src/use-uncontrolled/use-uncontrolled.ts

This happens because there is no validation in onChange handler, it just validates initial value and value in useEffect

cstrat commented 3 years ago

Very cool hook though!

I didn't fully understand what it was for until I tried to work out what was going on here.

cstrat commented 3 years ago

I will just update my form to use the number as a string and cast it to number where I need to, no big deal really.

Did this, all good, works fine. 👍🏻