Proposal (breaking change): remove inputProps from Input, TextField, etc.

[pelotom]

The Input component has both

inputComponent?: React.ReactNode;

and

inputProps?:
    | React.TextareaHTMLAttributes<HTMLTextAreaElement>
    | React.InputHTMLAttributes<HTMLInputElement>;

The first thing to note is that the type for inputComponent is wrong, it should be

inputComponent?: string | React.ComponentType<...>;

where ... is whatever properties could actually be passed down by the parent component to the child component. That brings us to inputProps. What are they here for? If you look at most other components, for example Button, there is no prop like this, only

  component?: string | React.ComponentType<ButtonProps>;

and the way you would inject whatever custom props your overriding component needs would be

<Button component={buttonProps => <MyButton {...buttonProps} myCustomProp={3} />} />

So why isn't Input the same way, and do we really need the inputProps prop? Besides consistency with the rest of the code base, a good reason not to have inputProps is that it's impossible to make type safe. We basically have to say

inputComponent?: string | React.ComponentType<any>;
inputProps?: any;

So I'd like to advocate removing inputProps and instead having a closed set of specified props that Input can pass to inputComponent.

See also #9313.

[rosskevin]

From https://github.com/mui-org/material-ui/issues/9313#issuecomment-347729788

For custom inputs, it would turn:

Case 1 - open ended (non-type checked) inputProps

import * as MaskedInput from 'react-text-mask'

      <TextField
        InputProps={{
          inputComponent: MaskedInput,
        }}
        inputProps={{
          guide: false,
          mask,
          placeholderChar: '\u2000',
        }}
        type="tel"
        value={value}
      />

into

Case 2: type checked inputs allowing entirely custom inputComponent

import { InputProps as MuiInputProps } from 'material-ui/Input'

      <TextField
        InputProps={{
          inputComponent: (inputProps: MuiInputProps) => (
            <MaskedInput {...inputProps} guide={false} mask={mask} placeholderChar="\u2000" />
          ),
        }}
        type="tel"
        value={value}
      />

This seems like a good change to me. This change is more obvious with typescript use, as our flow types are currently open ended and it is much harder to compose types from types in flow, whereas our typescript code is quite good at this.

I've done a cursory review of the usage in the codebase and this seems like a feasible and warranted change. I'm willing to take it on. @oliviertassinari your thoughts?

[rosskevin]

As an aside, with this change, inputComponent looks like it will be redundant, and just need component like others.

[rosskevin]

I'm going to move forward with this because the typescript for inputComponent is also wrong. Any objections please speak now.

[rosskevin]

So here is what I've found:

a) Why does inputProps (lowercase) exist on Input.js?

Input.js has a div wrapper with potential adornment, and finally the component (usually input). So inputProps exists because by design ...rest is always spread on the root which is div. This has a bit of a strangeness to it because many props are explicitly listed then positioned on the input element anyway.

In this regard, flow is far from the typescript definitions - our TS definitions make it much more apparent where you could use/apply elements (and validate them). The reason this issue came about was from looking at the TS definitions and seeing they don't make a ton of sense for this component.

b) Why does InputProps, inputProps, and inputComponent exist in TextField.js?

• InputProps gives you access to pass down anything that Input.js accepts, also including inputProps and inputComponent.
• inputProps exists as an exposure of Input.inputProps.
• inputComponent exists as an exposure of Input.inputComponent and has been renamed to avoid conflicts.

Refactoring

TextField is a convenience component only, and many underlying props are flattened for ease of use. Because a) (above) is consistent with the rest of the codebase, the only refactoring I see here is to remove advanced Input props exposed in the TextField props simply to prevent confusion.

1. Remove TextField.inputProps, since it can be accessed via InputProps.
2. Remove TextField.InputClassName - redundant with inputClassName and InputProps.className
3. Rename Input.inputComponent to component.

We could go much further and only allow uppercase props InputLabelProps | InputProps | SelectProps | FormHelperTextProps, but that would likely defeat the purpose of convenience.

So I'll move forward with 1 and 2 above.

[rosskevin]

So I think this is cleaner/clearer, but it does come at the expense of this usage:

Old

<TextField type="number" inputProps={{ min: "0", max: "10", step: "1" }} />

New

<TextField type="number" InputProps={ {inputProps: { min: "0", max: "10", step: "1" }} } />

Still, the pattern seems much more obvious here, and we are less likely to wonder why we have both InputProps and inputProps.

[oliviertassinari]

So why isn't Input the same way, and do we really need the inputProps prop?

@pelotom It's only here for ease of use. I do think we have this "hack" at many other locations. I'm fine removing it. It's reducing the API surface and making the library more TypeScript compatible 👍 .

This allows Input.inputComponent to be renamed to component.

@rosskevin Does this make the API more consistent? I mean, the component property was always altering the root component.

[rosskevin]

For those that may find this in the future, the case 2 from above is now

import { InputProps as MuiInputProps } from 'material-ui/Input'

      <TextField
        InputProps={{
          component: (inputProps: MuiInputProps) => (
            <MaskedInput {...inputProps} guide={false} mask={mask} placeholderChar="\u2000" />
          ),
        }}
        type="tel"
        value={value}
      />

[rosskevin]

@oliviertassinari I've submitted the PR, I think it's a good change. With regards to component, it is not like the promise to spread unused on the root component, and indeed the root component is not configurable. I regard Input.component to be self evident, whereas Input.inputComponent to seem a bit redundant. That's also a question about exposing or hiding implementation details.

[mbrookes]

I regard Input.component to be self evident

Declaration of Input-dependence? 😄

[semiadam]

This is a really bizarre change! In version 22 and before, to apply a step property to an input we needed to add this prop: inputProps: { step: '0.5' }

I think that was already confusing because it was not clear why some of native properties were directly available on TextField (like nameand type) and some were not (like step, min, max). If I wanted to make things more intuitive/less confusing, I'd make TextField act more like a native element, so users can intuitively get things to work. Something as simple as: step={0.5}

However, in the new release beta.23, the props should be set like this: InputProps={{ inputProps: { step: '0.5' } }}

Based on what I read the reasoning behind this change is to eliminate confusion. IMHO this is several times more confusing, and in reality users should keep digging documentation and copy/paste this every time they need it. Of course I'm a single user and have one vote! Others may think differently. I appreciate your great work on this amazing library regardless.

[oliviertassinari]

@semiadam You make a good point. The first use case for providing properties on the TextField element is to alter the native input. The inputProps property what making it easily discoverable in the API documentation. Maybe it's just a api documentation issue.

[rosskevin]

Reasoning: TextField is a convenience wrapper for the most common cases (80%). It cannot be all things to all people, otherwise the API would grow out of control. Given it had some advanced props propagating, but not all, it was confusing and limiting. Restricting this convenience wrapper to just 80% and allowing advanced use only through the uppercase component props reduces confusion and reduces the surface area.

I agree that you would think inputProps belongs on TextField, but consistency-wise, it does not, as evidenced by the pattern of use of every other component contained and controlled by TextField.

I hope that explanation helps. I spent a good bit of time untangling the naming and patterns here to arrive at the current state.

[oliviertassinari]

@rosskevin Any lead for improving the documentation? Maybe we can add a not about it in the header of https://material-ui.com/api/text-field/#textfield

TextField is a convenience wrapper for the most common cases (80%). It cannot be all things to all people, otherwise the API would grow out of control. If you wish to alter the properties applied to the native input, you can do as follow:

// ... example

An example: https://material-ui.com/api/form-control/

[rosskevin]

Not a bad idea. We should probably highlight the structure of it and point to the underlying components. I think a literal display of source structure might be useful, all under a header like "Advanced Configuration"

[oliviertassinari]

I'm on the documentation part.