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

Unexpected props type for React component with constructor #25581

Closed
OliverJAsh opened this issue Jul 11, 2018 · 7 comments
Closed

Unexpected props type for React component with constructor #25581

OliverJAsh opened this issue Jul 11, 2018 · 7 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@OliverJAsh
Copy link
Contributor

TypeScript Version: 2.9.2

Search Terms: react component props type constructor implicit any hoc

Code

type RouteComponentProps = {
    history: {};
};

type Omit<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>>;

declare function withRouter<P extends RouteComponentProps>(
    component: React.ComponentType<P>,
): React.ComponentClass<Omit<P, keyof RouteComponentProps>>;

type FooProps = { foo: string } & RouteComponentProps;
class Foo extends React.Component<FooProps> {
    constructor(props) {
        super(props);
    }
}

// Expect error due to missing props, got one :-)
const a = <Foo />;
// Expect error due to missing props, got one :-)
const b = <Foo foo="1" />;

// However, after using a (correctly defined) HOC, the props type seems to go completely

const Foo2 = withRouter(Foo);

// Expect error due to missing props, but got none :-(
const c = <Foo2 />;

… unless we comment out the constructor, or annotate the props param:

class Foo extends React.Component<FooProps> {}

const Foo2 = withRouter(Foo);

// Expect error due to missing props, got one :-)
const c = <Foo2 />;
class Foo extends React.Component<FooProps> {
    constructor(props: Props) {
        super(props);
    }
}

const Foo2 = withRouter(Foo);

// Expect error due to missing props, got one :-)
const c = <Foo2 />;

Why does the presence of the constructor change the props type only after the component is ran through the HOC?

@OliverJAsh
Copy link
Contributor Author

OliverJAsh commented Jul 14, 2018

I tried to further narrow down the issue. This is where I got to:

// src/main-tsx.tsx

declare function getComponentProps<P>(
    component: React.ComponentType<P> | React.Component<P>,
): P;

type GetComponentProps<T> = T extends React.ComponentType<infer P> | React.Component<infer P> ? P : never

type FooProps = { foo: string };
class Foo extends React.Component<FooProps> {
    // props param is implicitly `any`
    // Workaround: if we annotate the param, `FooPropsInferred2` will have the expected type
    // constructor(props: FooProps) {
    constructor(props) {
        super(props);
    }
}

declare const fooProps: FooProps;
// $ExpectType FooProps
fooProps;

declare const fooProps2: GetComponentProps<Foo>;
// $ExpectType FooProps
fooProps2;

const x = getComponentProps(Foo);
declare const fooProps3: typeof x;
// $ExpectType FooProps
fooProps3;
$ typings-checker --project tsconfig.json src/main-tsx.tsx
src/main-tsx.tsx:32: Expected type
  { foo: string }
but got:
  any

src/main-tsx.tsx: 2 / 3 checks passed.

@mattmccutchen
Copy link
Contributor

mattmccutchen commented Jul 26, 2018

I narrowed the issue down further and removed the React dependency:

class Component<P> {
    props: P;
    constructor(props: P) {
        this.props = props;
    }
}

interface ComponentClass<P> {
    new (props: P): Component<P>;
}

declare function getComponentProps<P>(
    component: ComponentClass<P>,
): P;

declare function getComponentProps2<T>(
    component: T,
): T extends ComponentClass<infer P> ? P : never;

type FooProps = { foo: string };
class Foo extends Component<FooProps> {
    // props param is implicitly `any`
    // Workaround: if we annotate the param, `FooPropsInferred2` will have the expected type
    // constructor(props: FooProps) {
    constructor(props) {
        super(props);
    }
}

const x = getComponentProps(Foo);  // any
const y = getComponentProps2(Foo);  // FooProps

In each case, there are two inferences for P: an any from the constructor parameter and a FooProps from the props of the constructed object. It looks like the root cause of the difference is that inference in conditional types is always performed with the equivalent of strictFunctionTypes enabled, while I'm assuming strictFunctionTypes was at its default value of off for the getComponentProps call. Glossing over a bunch of detail I don't fully understand, strictFunctionTypes affects the way the two inferences are prioritized. If you enable strictFunctionTypes in tsconfig.json, then the discrepancy goes away.

I assume the inference was implemented the way it was for a reason, though I don't know the reason. I think the solution to this case is "use noImplicitAny"; I don't think it merits changing anything in the compiler.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 26, 2018

thanks @mattmccutchen for the detailed analysis. implicit any is not any different from explicit any from a type perspective. inference will proceed from both the same way.

I think the underlying issue here is an expectation that derived class methods, properties, and constuctors "inherit" their types from the base class. that is unfortunately not the case, this is tracked by #23911 (previously discussed in #1373, #3667, and #10610).

@mhegazy mhegazy added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jul 26, 2018
@OliverJAsh
Copy link
Contributor Author

the root cause of the difference is that inference in conditional types is always performed with the equivalent of strictFunctionTypes enabled

This is a surprising inconsistency. Is this intentional for some reason, or is it a bug?

@OliverJAsh
Copy link
Contributor Author

If you enable strictFunctionTypes in tsconfig.json, then the discrepancy goes away.

I just tried this on TypeScript 2.9.2 and the discrepancy does not go away when strict is enabled, so I'm not sure this does explain the problem. Can we take another look at this?

@mattmccutchen
Copy link
Contributor

mattmccutchen commented Aug 3, 2018

I just tried this on TypeScript 2.9.2 and the discrepancy does not go away when strict is enabled, so I'm not sure this does explain the problem.

You are right. I don't know what I was thinking before.

New analysis: in both cases, P has a contravariant candidate of any and a covariant candidate of FooProps, but in the first case, the inference context contains a signature because inference is being done for the call to getComponentProps, while in the second case, the inference context does not contain a signature because inference is being done in the conditional type. When the inference context contains a signature, both covariant and contravariant candidates are considered and the result is any; when it doesn't, the covariant candidate takes priority. See this code from getInferredType:

                const signature = context.signature;
                if (signature) {
                    if (inference.contraCandidates) {
                        // If we have contravariant inferences we find the best common subtype and treat
                        // that as a single covariant candidate.
                        inference.candidates = append(inference.candidates, getContravariantInference(inference));
                        inference.contraCandidates = undefined;
                    }
                    if (inference.candidates) {
                        inferredType = getCovariantInference(inference, context, signature);
                    }
                    // ...
                }
                else {
                    inferredType = getTypeFromInference(inference);
                }

where:

        function getTypeFromInference(inference: InferenceInfo) {
            return inference.candidates ? getUnionType(inference.candidates, UnionReduction.Subtype) :
                inference.contraCandidates ? getIntersectionType(inference.contraCandidates) :
                emptyObjectType;
        }

The conclusion is still the same: I assume the inference algorithm was designed this way for a reason (though I'm not familiar with it), and you should use noImplicitAny to avoid surprises. The TypeScript checker is a pile of hacks designed to do what people want in as great a percentage of cases as possible, and it does a remarkably good job of that; the trade-off is that there will inevitably be inconsistencies like this.

@OliverJAsh
Copy link
Contributor Author

Closing as apparently this is working as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants