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

Sub-type type #6218

Closed
rogierschouten opened this issue Dec 23, 2015 · 11 comments
Closed

Sub-type type #6218

rogierschouten opened this issue Dec 23, 2015 · 11 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@rogierschouten
Copy link

I have two use cases that I cannot seem to type correctly in TypeScript. Both use cases involve functions that take a parameter of type "an object with any subset of the members of interface X".

Use case 1: options merging

interface Options {
  a: string;
  b: string;
}

const defaults: Options = {
  a: "a",
  b: "b"
}

function merge<T>(base: T, additional: <subtype of T>): T {
  // clone all members of 'additional' onto 'base'
}

let opts: Options = merge<Options>(defaults, { b: "some other b" });

Use case 2: React state
See also https://facebook.github.io/react/docs/component-api.html

A React component is a class with three generic parameters Props, State and Context. The base class defines a setState(newState: State) method. In reality though, the setState() method accepts any sub-type of State.

This becomes particularly painful when having multiple layers of inheritance in React components:

BaseWidget.tsx

// one React component
class BaseWidget<P extends BaseWidget.Props, S extends BaseWidget.State, C extends BaseWidget.Context> extends ReactComponent<P, S, C> {

    public componentWillMount()
                 // valid React call, TypeScript error 'argument of type foo is not assignable to parameter bar'
        this.setState({ a: "string" });
    }
}

module BaseWidget {
    "use strict";

        export interface Props extends React.Props<BaseWidget<Props, State, Context>> {
    }

        // note that all members must have ? on them to work
    export interface State {
            a?: string;
    }

    export interface Context {
    }
}

export = BaseWidget
@DanielRosenwasser
Copy link
Member

I think you mean a supertype, not a subtype. { b: string } is a supertype of Options.

In TypeScript 1.8, you'll be able to write

interface Options {
    a: string;
    b: string;
}

const defaults: Options = {
    a: "a",
    b: "b"
}


function merge<T extends U, U>(base: T, additional: U): T {
    return base;
}

let opts = merge(defaults, { b: "some other b" })

Try it out by running npm install -g typescript@next.

Does this address your use cases?

@DanielRosenwasser DanielRosenwasser added the Question An issue which isn't directly actionable in code label Dec 23, 2015
@rogierschouten
Copy link
Author

Hi @DanielRosenwasser thanks for your reply. It definitely addresses the first use case but not the second. This is because it is not possible to explicitly mention the supertype (and you were of course right about it being a supertype) and therefore you cannot make a proper signature for the setState method.

Using public setState(newState: S): void would not work; the React library allows any supertype of S, and you have to be able to state that somehow.

@lbguilherme
Copy link

@rogierschouten What about this? (didn't try)

public setState<S extends T>(newState: T): void

@DanielRosenwasser
Copy link
Member

@rogierschouten what if you just made all the properties optional? I think that's the usual approach.

@rogierschouten
Copy link
Author

@lbguilherme do you mean this? It generates the compile errors below.

/**
 * This class is a given by the React framework
 */
class ReactComponent<S> {
    protected _state: S;

    /**
     * This method actually accepts any supertype of S
     */
    protected setState<S extends T>(newState: T): void {
        for (let name in newState) {
            if (newState.hasOwnProperty(name)) {
                this._state[name] = newState[name];
            }
        }
    }

    protected componentWillMount(): void {
        // abstract
    }

}

/**
 * Some state interface declaration. Note all members are optional to allow setState to
 * be called with supertypes of BaseState
 */
interface BaseState {
    a?: string;
}

/**
 * My own base class for certain React widgets
 */
class BaseWidget<S extends BaseState> extends ReactComponent<S> {

    constructor() { 
        super();
        this._state = {};
    }

    protected componentWillMount(): void {
        this.setState({ a: "boo" });
    }
} 
$ tsc v2.ts
v2.ts(11,34): error TS2304: Cannot find name 'T'.
v2.ts(11,47): error TS2304: Cannot find name 'T'.
v2.ts(40,9): error TS2322: Type '{}' is not assignable to type 'S'.

@rogierschouten
Copy link
Author

@DanielRosenwasser your solution only works when you don't have an intermediary base class:

/**
 * This class is a given by the React framework
 */
class ReactComponent<S> {
    protected _state: S;

    /**
     * This method actually accepts any supertype of S
     */
    protected setState(newState: S): void {
        for (let name in newState) {
            if (newState.hasOwnProperty(name)) {
                this._state[name] = newState[name];
            }
        }
    }

    protected componentWillMount(): void {
        // abstract
    }
}

/**
 * Some state interface declaration. Note all members are optional to allow setState to
 * be called with supertypes of BaseState
 */
interface BaseState {
    a?: string;
}

/**
 * My own base class for certain React widgets
 */
class BaseWidget<S extends BaseState> extends ReactComponent<S> {

    constructor() { 
        super();
        this._state = {};
    }

    protected componentWillMount(): void {
        this.setState({ a: "boo" });
    }
} 
$ tsc v1.ts
v1.ts(39,9): error TS2322: Type '{}' is not assignable to type 'S'.
v1.ts(43,23): error TS2345: Argument of type '{ a: string; }' is not assignable to parameter of type 'S'.

This is because the compiler cannot know that the S that the BaseWidget is instantiated with doesn't have any additional required members. It is for this reason we need the supertype designation in the setState parameter so the compiler knows that this doesn't matter.

@lbguilherme
Copy link

@rogierschouten Specifying a T will work:

protected setState<T, S extends T>(newState: T): void 

But... this is too unrestricted. When S is {a: string}, setState({a: 4}) will work just fine. And for setState adding new fields is also not acceptable. Ideally there must be some language construct to turn all fields into optionals, I think flow has something about this.

@rogierschouten
Copy link
Author

@lbguilherme Yes I agree, there should be another language construct. Can somebody maybe remove the Question label from this issue and put something in that reflects this?

@RyanCavanaugh
Copy link
Member

At this point I'd prefer we have a new issue putting forth a concrete proposal (hopefully specifically addressing typing of setState, which is indeed problematic) - there's a lot going on here and it would be confusing to read top-down at this point.

@flyon
Copy link

flyon commented Jan 24, 2016

same issue here. If you use React + Typescript and you have some React components extending other components and you try to properly define the states with interfaces (with all properties being optional) it leads exactly to the problem described in usecase 2 above.

In other words, even though you provide a proper state object {someStateProp:true}, and all properties are optional still leads to the error:

Argument of type '{ someStateProp: boolean; }' is not assignable to parameter of type 'S'.

Sadly the only solution that I could find to make it work for now is to turn off the typecheck by manually editing react.d.ts and replacing S with any: setState(state: any)

@mDibyo
Copy link

mDibyo commented Nov 17, 2016

FYI: Looks like this has been resolved with #12114.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

6 participants