[Typescript] type for ref in <Input/> not sufficiently strict

[tianhuil]

The type for ref in Input is too general. It only requires (among other things) React.RefObject<any>. But the equivalent requirement for <input> is React.RefObject`.

• This is not a v0.x issue.
• I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

In the following app, the poorly typed myRef does not generate an error when passed to <Input/> but does generate a type error when passed to <input/>

import React from 'react';
import Input from '@material-ui/core/Input'
import './App.css';

const App = () => {
  const myRef = React.useRef()

  return (
    <div className="App">
      <input name="foo" ref={myRef}/> // type error
      <Input inputRef={myRef}></Input> // no type error
    </div>
  );
}

export default App;

Current Behavior 😯

It should generate an error on both <Input/> and <input/>. This would be helpful to so typechecking can help automatically check for expressions like myRef.current.value in subsequent usage.

Steps to Reproduce 🕹

Link: clone this repo and run npm i. You'll observe the error in App.tsx.

Context 🔦

This would be helpful to so typechecking can help automatically check for expressions like myRef.current.value in subsequent usage.

Perhaps related to #10750

Your Environment 🌎

Tech	Version
Material-UI	v3.9.2
React	16.8.5
TypeScript	3.3.4000

[eps1lon]

It is currently typed as any. Since the props aren't generic and we need type information about inputComponent to know what type inputRef would be. I'm not sure using unknown would be the better choice here. You could type it as unknown yourself to have full type safety.

That whole logic can be simplified once v4 lands. The longterm goal is to drop inputRef and encourage <Input inputProps={{ ref }} />. Once all our props are generic this should give you proper type safety.

[tianhuil]

Thanks for the quick response @eps1lon. Could you provide a code snippet for

You could type it as unknown yourself to have full type safety.

I'm not sure if you mean fork a copy of @material-ui and change the type signature or if you mean something else?

[eps1lon]

I'm not sure if you mean fork a copy of @material-ui and change the type signature or if you mean something else?

You only have to fork the declaration for Input.d.ts and then use a paths alias in your tsconfig e.g

"paths": {
  "@material-ui/core/Input": ["path/to/forked/Input"]
}

You have to be aware though that the instance in inputRef depends on the type of the inputComponent. If you can create a declaration that covers those cases I would appreciate a PR.

[tianhuil]

Thanks @eps1lon!

the instance in inputRef depends on the type of the inputComponent

From what I can tell, you want to handle two types?

• If multiline is true then it should be of type React.RefObject<HTMLTextAreaElement>
• If multiline is false then it should be of type React.RefObject<HTMLInputElement>

I don't think I can be precise about the multiline condition but React.RefObject<HTMLTextAreaElement | HTMLInputElement> would be a more narrow (but sufficiently flexibe) typing than we have now?

[eps1lon]

I don't think I can be precise about the multiline condition but React.RefObject<HTMLTextAreaElement | HTMLInputElement> would be a more narrow (but sufficiently flexibe) typing than we have now?

That helps but it's not necessarily true if another component is used via inputComponent.

[tianhuil]

Thanks! I don't have a lot of context about the intended use case for <Input/> but from the docs it appears that

You can use third-party libraries to format an input. You have to provide a custom implementation of the element with the inputComponent property. The provided input component should handle the inputRef property. The property should be called with a value implementing the HTMLInputElement interface.

Do you have a list of HTML*Element that you would like to support or is this a thing where you're afraid to break backward compatibility if lots of other people have used custom components that don't satisfy the full type interface? Happy to implement the former but don't really think there's a solution for the latter.

[eps1lon]

I'll double checked with the code and we require a HTMLInputElement at runtime anyway:

https://github.com/mui-org/material-ui/blob/f9896bcd129a1209153106296b3d2487547ba205/packages/material-ui/src/InputBase/InputBase.js#L234-L241

So Ref<HTMLInputElement> for inputRef is totally fine for
https://github.com/mui-org/material-ui/blob/f8c42a32d5de5f35e626a182c4fb81a7ce7ff615/packages/material-ui/src/InputBase/InputBase.d.ts#L20

[eps1lon]

@tianhuil There is a bigger picture issue when typing refs too strict. What happens is that everybody who only cares about inputRef being an HTMLElement (e.g. just for focusing) cannot use this less specific ref. <InputBase ref={React.createRef<HTMLElement>()} /> would be rejected by the type checker.

In the current state of TypeScript we can't offer a sound ref type. Soundness is the responsibility of the type user e.g. in this case you would have to declare your ref as React.createRef<HTMLInputElemen>(). It would make the read safe but you need to verify manually that the write by react is correct. The safest option would be to use React.createRef<unknown> which is probably what we're implementing. Then you can narrow the type manually like you prefer (casting, instanceOf checks, or other type guards).

You can check out https://github.com/eps1lon/typescript-react-ref-issue/blob/626f5197601c35a13e1207a8f4ab7e029454ecb6/index.tsx for a better overview what's currently accepted and rejected by typescript.

[eps1lon]

For components with generic props we infer the exact ref from the type of the component prop. Everything else will use unknown and we let the user control soundness of their ref.

For a more detailed explanation see #15199. Removing this from the v4 milestone until we get better types for refs i.e. write-only or configurable variance or a better solution is proposed.

[eps1lon]

Closing as not actionable for us. For more context see #15199.