fix(input): allow number proptype on input value prop

[ismay]

We currently don't allow numbers for the Input value prop, if you look at the prop-types. This adds the number type for <input type="number" />.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/number

[cypress]

---

Test summary

482 • 0 • 0 • 0

---

Run details

Project	ui
Status	Passed
Commit	1a0391b
Started	Jul 2, 2020 2:04 PM
Ended	Jul 2, 2020 2:14 PM
Duration	10:01 💡
OS	Linux Ubuntu Linux - 18.04
Browser	Electron 80

View run in Cypress Dashboard ➡️

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

[Mohammer5]

I'm not sure numbers work as values, if I'm not mistaken, they don't work and are transformed to strings by the dom element, so when the input changes, it's value is a string, not a number (would need to be confirmed though)

[ismay]

I'm not sure numbers work as values, if I'm not mistaken, they don't work and are transformed to strings by the dom element, so when the input changes, it's value is a string, not a number (would need to be confirmed though)

Yeah that's right, Hendrik mentioned this as well. The thing is, I'm currently getting a number type for this field from the backend (through the data engine). So I'm getting a type warning there at the moment, since the input expects a string (it does work with a number btw.). I guess we'll have to decide which course of action we recommend:

1. Cast the number from the backend to a string before supplying it to the input
2. Allow both numbers and string for the input

Since the value of a number input in the onchange event is a string, I'm leaning towards the first solution, as that way we're not using multiple types for the same input, which seems confusing.

[HendrikThePendric]

Since the value of a number input in the onchange event is a string, I'm leaning towards the first solution, as that way we're not using multiple types for the same input, which seems confusing.

When @ismay and I discussed this last Thursday, I was initially also leaning to the first solution as well. But one thing Ismay said made me see thing another way. Apparently where have been cases in apps where the following has happened:

• Numerical values have been updated via an input, making their values of type string
• These string values have then been sent to the server
• The server has stored them as type number

So, my point is that under quite a few circumstances it's fine to be blissfully unaware of this value-type changing going on. I think most of our validators are also robust enough to deal with both numerical values of type number and string. Probably, a developer would only need to be aware of this value-type changing when he starts doing calculations based on the value, or wants to do custom validation logic.

In my mind, only allowing propTypes.string and forcing the dev to cast numbers to string is probably the "purest" solution.

However, allowing propTypes.number as well, adds a lot of convenience and I don't actually think it will have significant drawbacks. The errors by posting a string instead of a number or trying to do numerical operations on a string are quite likely just as clear of a hint to the developer as the propTypes warning would be, and without the inconvenience that disallowing propTypes.number would bring.

[ismay]

Since the value of a number input in the onchange event is a string, I'm leaning towards the first solution, as that way we're not using multiple types for the same input, which seems confusing.

When @ismay and I discussed this last Thursday, I was initially also leaning to the first solution as well. But one thing Ismay said made me see thing another way. Apparently where have been cases in apps where the following has happened:

* Numerical values have been updated via an input, making their values of type string

* These string values have then been sent to the server

* The server has stored them as type number

So, my point is that under quite a few circumstances it's fine to be blissfully unaware of this value-type changing going on. I think most of our validators are also robust enough to deal with both numerical values of type number and string. Probably, a developer would only need to be aware of this value-type changing when he starts doing calculations based on the value, or wants to do custom validation logic.

In my mind, only allowing propTypes.string and forcing the dev to cast numbers to string is probably the "purest" solution.

However, allowing propTypes.number as well, adds a lot of convenience and I don't actually think it will have significant drawbacks. The errors by posting a string instead of a number or trying to do numerical operations on a string are quite likely just as clear of a hint to the developer as the propTypes warning would be, and without the inconvenience that disallowing propTypes.number would bring.

Yeah I see your point as well. I don't really have a clear preference at the moment. Leaving the app to deal with casting the number values to strings gets quite annoying and maybe a bit error prone as well (recursively mapping over an object you've received from the backend and converting the numbers to strings).

edit: removed the mention of Austin, since he's on holiday. We can talk about this tomorrow, when JG is back.

[Mohammer5]

Leaving the app to deal with casting the number values to strings gets quite annoying and maybe a bit error prone as well

react-final-form has the format & parse props on the Field, so this can easily be handled on the Field level. We're encouraging devs to use RFF, so we could consider non-RFF cases to be edge cases.

As long as the input field will always call the onChange function with a string value, I don't think we should change the prop types. If we instead ensure that, if a number is passed in, a number will be passed to the onChange, I'd me more ok with it. But allowing numbers and getting back strings seems to be a very confusing, non-standard, pattern

[ismay]

Leaving the app to deal with casting the number values to strings gets quite annoying and maybe a bit error prone as well

react-final-form has the format & parse props on the Field, so this can easily be handled on the Field level. We're encouraging devs to use RFF, so we could consider non-RFF cases to be edge cases.

Ah, I completely forgot about that. Yeah, that seems like a proper solution to me. I'll close this then 👍

[HendrikThePendric]

Good suggestion @Mohammer5