s-yadav / react-number-format

React component to format numbers in an input or as a text.
MIT License
3.88k stars 409 forks source link

The `type` prop should accept `string` as indicated in documentation #799

Closed jakeboone02 closed 1 year ago

jakeboone02 commented 1 year ago

Describe the issue and the actual behavior

There is a disconnect between the documentation for the type prop, which states that it accepts any string, and the actual TypeScript types, which narrow the accepted values to "text", "tel", or "password":

https://github.com/s-yadav/react-number-format/blob/e2179f9e9d2c4ddd1ab714ff385412b53e9ed3e7/src/types.ts#L68-L69

Describe the expected behavior

Since string is the correct type for the type property of HTML5 <input> elements, and browsers fall back to the default behavior of type="text" when the type is outside the list of supported values, I believe the type prop should be typed as type?: string.

In my testing, all strings work as expected during runtime. I have a branch ready for PR if this change would be accepted.

Provide a CodeSandbox link illustrating the issue

https://codesandbox.io/s/type-demo-forked-p4lh8w?file=/src/App.js

Provide steps to reproduce this issue

Please check the browsers where the issue is seen

Not specific to a browser; issue is with TypeScript types.

Original issue: mantinedev/mantine#4821.

rwb196884 commented 1 year ago

The documentation doesn't say what the possible values are, only that they include text | tel | password.

jakeboone02 commented 1 year ago

Right, that's my point. I believe the documentation is more correct as far as the intent, but the actual TypeScript types don't allow anything other than 'text' | 'tel' | 'password'. The code should change, not the documentation.

s-yadav commented 1 year ago

So the type here 'text' | 'tel' | 'password' is intentional. type: 'number' will not work as it applies non-numeric formatting in the input itself. And other types also do not make much sense for it as they come with custom HTML input behaviour. Maybe type 'search' can fit here. But this hasn't been asked till now.

try this, this won't work <NumericFormat type="number" thousandSeparator value={123} />