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 higher-order components run into trouble with assignability of Readonly intersections #27484

Closed
mattmccutchen opened this issue Oct 1, 2018 · 2 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@mattmccutchen
Copy link
Contributor

mattmccutchen commented Oct 1, 2018

I'm trying to answer this Stack Overflow question on typing a React higher-order component. I've been recommending intersections rather than Exclude for expressing the inner and outer prop types for a higher-order component since TypeScript's type relations and inference seem to handle intersections better than Exclude in general. (I know this practice has problems; if you know a better way, please let me know! For now, Object.assign and JSX spreads still generate intersections.) However, React also uses Readonly for prop types, and I've hit an ugly case in which Readonly is getting in the way of matching up the intersections between the supplied and expected props. Many simpler cases seem to work OK, but this one fails.

I'm not sure if this is a bug but would like the TypeScript team's feedback on the scenario, since the question has been up on Stack Overflow and I doubt anyone from the TypeScript team will ever look at it there. Hopefully, in consideration of my answering hundreds of TypeScript questions on Stack Overflow and helping triage tens of other issues here, I can have the privilege of occasionally escalating questions I can't answer to the TypeScript team. Thanks!

TypeScript Version: master (8feddcd)

Search Terms: readonly mapped type intersection assignable assignability higher order components

Code

import * as React from "react";

function myHigherOrderComponent<P>(Inner: React.ComponentClass<P & {name: string}>): React.ComponentClass<P> {
    return class OuterComponent extends React.Component<P> {
        render() {
            // Type 'Readonly<{ children?: ReactNode; }> & Readonly<P> & { name: "Matt"; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<P & { name: string; }, any, any>> & Readonly<{ children?: ReactNode; }> & Readonly<P & { name: string; }>'.
            //   Type 'Readonly<{ children?: ReactNode; }> & Readonly<P> & { name: "Matt"; }' is not assignable to type 'Readonly<P & { name: string; }>'.
            return <Inner {...this.props} name="Matt"/>;
        }
    };
}

Note: if we replace React.ComponentClass with React.ComponentType, then the issue does not reproduce due to #27385. This may be why it hasn't been noticed so far.

Expected behavior: No error.

Actual behavior: Error as marked.

Playground Link: Link for non-React example

Related Issues: Possibly #27201

@weswigham
Copy link
Member

Minimal repro:

function f<P>(
    a: { weak?: string } & Readonly<P> & { name: "ok" },
    b: Readonly<P & { name: string }>,
    c: Readonly<P> & { name: string }) {
    c = a; // Works
    b = a; // Effectively identical, doesn't work
}

@mattmccutchen
Copy link
Contributor Author

mattmccutchen commented Oct 10, 2018

Right. In essence, the current assignability rules only relate Readonly<T> to exactly T. The decomposition of intersections may mean that T & U is assignable to Readonly<T>, but Readonly<T & U> isn't assignable to T. In #27590, I started to try to extend the assignability rules for mapped types to handle intersections, but I got mired in trying to fix existing bugs in the rules.

I'm starting to think a better solution to the original problem is to change the Object.assign signature (and the analogous thing for spreads in JSX) to strip Readonly from arguments before taking the intersection and also change JSX.LibraryManagedAttributes in @types/react to strip Readonly from the expected props type. That should get Readonly completely out of the picture. We can still extend the assignability rules to handle Readonly intersections later if we want.

I edited the issue title to make clear that I'm looking for a solution to the original scenario of React higher-order components, which may or may not take the form of extending the assignability rules to handle Readonly intersections.

@mattmccutchen mattmccutchen changed the title Readonly interfering with assignability of intersections (affects React higher-order components) React higher-order components run into trouble with assignability of Readonly intersections Oct 10, 2018
@weswigham weswigham added Bug A bug in TypeScript Fixed A PR has been merged for this issue and removed Needs Investigation This issue needs a team member to investigate its status. labels Oct 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

3 participants