Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React: Type inference not working for ref callback #16019

Closed
simonvizzini opened this issue May 23, 2017 · 14 comments
Closed

React: Type inference not working for ref callback #16019

simonvizzini opened this issue May 23, 2017 · 14 comments
Labels
Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter
Milestone

Comments

@simonvizzini
Copy link

Type inference is not working for the ref callback prop.
I believe this is the same issue as in #7088, which was fixed over a year ago.

TypeScript Version: 2.3.2

Code

// A *self-contained* demonstration of the problem follows...
class Other extends React.Component<any, any> {
	render() {
		return <div />;
	}
}

class Container extends React.Component<any, any> {
	private _ref: Other;

	render() {
                // Parameter 'ref' implicitly has an 'any' type.
		return <Other ref={ ref => this._ref = ref } />;
	}
}

Expected behavior:
Type of parameter ref should be inferred as Test

Actual behavior:
Parameter 'ref' implicitly has an 'any' type.

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label May 23, 2017
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 2.4 milestone May 23, 2017
@mhegazy
Copy link
Contributor

mhegazy commented May 24, 2017

The issue here is that the type of props is any, since Other is declared to extend from React.Component<any, any>. The result here is that all references to props are practically unchecked.

if this component does not have any properties, then declare it as extends React.Component<{}, any>.

if the intention is to allow any property to be defined on this component, then declare at as extends React.Component<Record<string, any>, any>.

@mhegazy mhegazy added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Bug A bug in TypeScript labels May 24, 2017
@mhegazy mhegazy removed this from the TypeScript 2.4 milestone May 24, 2017
@simonvizzini
Copy link
Author

Thank you, @mhegazy! In my concrete case I actually did have a concrete Props interface defined, I just didn't think about this when trying to come up with a minimal example (I'm still learning the nuances of the type system). Nonetheless, I'm simply unable to properly type the ref callbacks, especially when defining Props interfaces that extend other, generic props (e.g. interface Props extends React.HTMLProps<HTMLInput>). Typescript gave funny errors along the lines of "got type A but expected B", where A extends from B, and when I changed the type def to type to B it said "got B but expected C", where B again extends from C...

It's most likely a misunderstanding on my side, still learning ts and so I'll come up with a more concrete example today or tomorrow.

@mhegazy
Copy link
Contributor

mhegazy commented May 24, 2017

this works for me. if you have a sample we can look more.

interface ContainerProps { 
    someProp: number;
}
class Container extends React.Component<ContainerProps, any> {
    render() {
        return <div ref={p => 0} />; // p is HTMLDivElement
    }
}

var x = <Container someProp={3} ref={p => 0} /> // p is Container

@simonvizzini
Copy link
Author

simonvizzini commented May 30, 2017

Thank you, your example indeed works. Finally I had time to come up with a better example that doesn't work for me. I'm probably doing something wrong I think:

interface MyInputProps extends React.ChangeTargetHTMLProps<HTMLInputElement> {
    onValid?(): boolean;
    onInvalid?(): boolean;
}

class MyInput extends React.Component<MyInputProps, any> {
    render() {
        const { onValid, onInvalid, ...inputProps } = this.props;
        return (
            <input { ...inputProps } />
        );
    }
}

var x = <MyInput ref={p => 0} /> // p is any

When I try to explicitly set a type like ref={(p: MyInput) => 0} then I get following compiler error:

message: 'Type '{ ref: (p: MyInput) => number; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<MyInput> & Readonly<{ children?: ReactNode; }> & R...'.
  Type '{ ref: (p: MyInput) => number; }' is not assignable to type 'Readonly<MyInputProps>'.
    Types of property 'ref' are incompatible.
      Type '(p: MyInput) => number' is not assignable to type 'Ref<HTMLInputElement>'.
        Type '(p: MyInput) => number' is not assignable to type '(instance: HTMLInputElement) => any'.
          Types of parameters 'p' and 'instance' are incompatible.
            Type 'HTMLInputElement' is not assignable to type 'MyInput'.
              Property 'render' is missing in type 'HTMLInputElement'.'
source: 'ts'

When I try to set the type to HTMLInputElement I get:

message: 'Type '{ ref: (p: MyInput) => number; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<MyInput> & Readonly<{ children?: ReactNode; }> & R...'.
  Type '{ ref: (p: MyInput) => number; }' is not assignable to type 'Readonly<MyInputProps>'.
    Types of property 'ref' are incompatible.
      Type '(p: MyInput) => number' is not assignable to type 'Ref<HTMLInputElement>'.
        Type '(p: MyInput) => number' is not assignable to type '(instance: HTMLInputElement) => any'.
          Types of parameters 'p' and 'instance' are incompatible.
            Type 'HTMLInputElement' is not assignable to type 'MyInput'.'
source: 'ts'

Clearly I have no idea what's going on here and I'm just randomly throwing types at my code until the compiler stops complaining 😆

@simonvizzini
Copy link
Author

simonvizzini commented May 30, 2017

I'm so sorry for spamming your inbox :) I think I finally figured out why TS is complaining (somehow I always missed the Property 'render' is missing in type 'HTMLInputElement'.' part of the error message, which gave me a hint in the right direction):

When I change the props interface to this:

interface MyInputProps extends React.ChangeTargetHTMLProps<MyInput & HTMLInputElement> {}

(note the MyInput & HTMLInputElement) then the compiler is happy. Automatic type inference of p is still any (not sure if this is a bug or a limitation), but at least I can explicitly define the type in the ref callback now. Could you confirm if I'm doing this right?

@mhegazy
Copy link
Contributor

mhegazy commented May 31, 2017

React.ChangeTargetHTMLProps extends React.ClassAttributes which overrides ref to be a union type. here is the definition from react declaration file on DT.

    type Ref<T> = string | ((instance: T) => any);

    interface ClassAttributes<T> extends Attributes {
        ref?: Ref<T>;
    }

The result here is that there is no contextual type for the function, and this p is any. this looks like a bug that we should fix. this should behave the same way it does with something like:

declare var x: string | ((a: HTMLInputElement) =>void);
x = a => { };

@mhegazy mhegazy added Bug A bug in TypeScript and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels May 31, 2017
@mhegazy mhegazy added this to the TypeScript 2.4 milestone May 31, 2017
@simonvizzini
Copy link
Author

Awesome, thank you!

@yuit
Copy link
Contributor

yuit commented Jun 7, 2017

ref in this case come from MyInputProps as it extends React.ChangeTargetHTMLProps....normally ref is the property of intrinsicClassAttribute which compiler will intersect with users' declared property before returning the intersection result as type of the tag-name's attributes.... We should instantiate such the property

Update: The compiler did instantiate ref property with instance of the component (see Mohamed's comment as example) .... @simonvizzini is there a reason why you do this MyInputProps extends React.ChangeTargetHTMLProps<HTMLInputElement> ?

I think the issues here are that:

  • we should show HTMLInputElement | MyInput instead of any
  • we should error in such case where type of ref parameter doesn't match the instance type of the component it is on. As instance parameter in ref function is always instance of the component you specified the ref (Doc).

@simonvizzini
Copy link
Author

Hi @yuit, I'm sorry, but somehow I got never notified about the edit and mention in your comment.

The reason I'm doing MyInputProps extends React.ChangeTargetHTMLProps<HTMLInputElement> is that so I don't have to manually specify "onChange" in the props interface. I'm not sure if this is good or correct, what do you think?

@yuit
Copy link
Contributor

yuit commented Jun 29, 2017

@simonvizzini no problem at all. Just to understand it better, are you plan to pass onChange property in like

interface MyInputProps extends React.ChangeTargetHTMLProps<HTMLInputElement> {
    onValid?(): boolean;
    onInvalid?(): boolean;
}

class MyInput extends React.Component<MyInputProps, any> {
    render() {
        const { onValid, onInvalid, ...inputProps } = this.props;
        return (
            <input { ...inputProps } />
        );
    }
}

function someFunction(...) {...}
var x = <MyInput ref={p => 0}  onChange={someFunction} /> // p is any

@yuit yuit added the Domain: JSX/TSX Relates to the JSX parser and emitter label Jul 11, 2017
@simonvizzini
Copy link
Author

Hi @yuit, sorry once more for the late reply- I got kind of lost in other tasks and I didn't keep track of this issue.

To answer your question, the intention is to intercept the onChange event, do some validation and then call the onChange callback property, if it has been passed down. Like this:

interface MyInputProps extends React.ChangeTargetHTMLProps<HTMLInputElement> {
    onValid?(): boolean;
    onInvalid?(): boolean;
}

class MyInput extends React.Component<MyInputProps, any> {
    render() {
        const { onValid, onInvalid, onChange, ...inputProps } = this.props;
        return (
            <input
                { ...inputProps }
                onChange={ this._onChange }
            />
        );
    }
    
    private _onChange = (event: React.ChangeEvent<HTMLInputElement>) => {
        // do some validation first...

        // ... then trigger onChange callback, if present:
        if (this.props.onChange) {
            this.props.onChange(event);
        }
    }
}

function someFunction(...) {...}
var x = <MyInput ref={p => 0}  onChange={someFunction} />

Hope this makes sense. Let me know if you have any other questions.

@DanielRosenwasser DanielRosenwasser added this to the TypeScript 2.6 milestone Aug 3, 2017
@timc13
Copy link

timc13 commented Apr 19, 2018

would love to see this get fixed!

@ImAbhishekTomar
Copy link

i am also facing similar type of issue.

ERROR


(JSX attribute) ref: React.Dispatch<(prevState: undefined) => undefined>
Type 'Dispatch<(prevState: undefined) => undefined>' is not assignable to type 'LegacyRef<HTMLButtonElement>'.
  Type 'Dispatch<(prevState: undefined) => undefined>' is not assignable to type '(instance: HTMLButtonElement) => void'.
    Types of parameters 'value' and 'instance' are incompatible.
      Type 'HTMLButtonElement' is not assignable to type '(prevState: undefined) => undefined'.
        Type 'HTMLButtonElement' provides no match for the signature '(prevState: undefined): undefined'.ts(2322)
index.d.ts(140, 9): The expected type comes from property 'ref' which is declared here on type 'IntrinsicAttributes & Props<"button", ButtonRenderPropArg, ButtonPropsWeControl>'

Screenshot 2021-06-04 at 3 05 48 AM

@Andarist
Copy link
Contributor

Andarist commented Nov 1, 2024

The reported case is about the parameter not being contextually typed for this ref. It works today so I think it's fair to close this issue

cc @RyanCavanaugh @jakebailey

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

11 participants