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

Generic type mismatch with Readonly<T> #18094

Closed
dpogue opened this issue Aug 28, 2017 · 4 comments
Closed

Generic type mismatch with Readonly<T> #18094

dpogue opened this issue Aug 28, 2017 · 4 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@dpogue
Copy link

dpogue commented Aug 28, 2017

This feels like #16985 in a slightly different form.
/cc @ahejlsberg

TypeScript Version: 2.5.1, but probably broken since 2.4.1

Code

interface MyInterface {
  something: number;
}

class MyClass<T extends MyInterface> {
  doIt(data : Readonly<T>) {}
}

interface II2 extends MyInterface {
  foo: string;
}

class MySubClass extends MyClass<II2> {}

function fn(arg: typeof MyClass) {};

fn(MySubClass);

Expected behavior:
No compilation error.

Actual behavior:

error TS2345: Argument of type 'typeof MySubClass' is not assignable to parameter of type 'typeof MyClass'.
  Type 'MySubClass' is not assignable to type 'MyClass<T>'.
    Types of property 'doIt' are incompatible.
      Type '(data: Readonly<II2>) => void' is not assignable to type '(data: Readonly<T>) => void'.
        Types of parameters 'data' and 'data' are incompatible.
          Type 'Readonly<T>' is not assignable to type 'Readonly<II2>'.
            Property 'foo' is missing in type 'Readonly<T>'.
@ahejlsberg
Copy link
Member

This is working as intended and is an effect of #16368. The type checker is pointing out that it is not safe to treat the doIt(data: Readonly<II2>) method in MySubClass as the more general doIt(data: Readonly<T>) in MyClass<T>. Effectively, you can't treat a method that expects the specific type II2 as a method that accepts any type that derives from MyInterface.

@ahejlsberg ahejlsberg added the Working as Intended The behavior described is the intended behavior; this is not a bug label Aug 28, 2017
@dpogue
Copy link
Author

dpogue commented Aug 28, 2017

This feels like it limits generics to the point where they're hardly useful anymore...

What we're trying to model is a "Model" class that wraps some data handled by a JSON API:

interface IModelData {
  href : string; // All models have an href
}

class ModelBase<DataType extends IModelData> {
  persist(data : Readonly<DataType>) {
    return callToServer(data['href'], data);
  }
}


interface IUserData extends IModelData {
  username : string;
}

class User extends ModelBase<IUserData> {}

What is the correct way to model this in TypeScript that allows proper typechecking?

@ahejlsberg
Copy link
Member

One way to model it is to have a non-generic base class:

interface IModelData {
  href : string; // All models have an href
}

abstract class ModelBase {
  abstract persist(data: IModelData): void;
}

class GenericBase<DataType extends IModelData> extends ModelBase {
  persist(data : Readonly<DataType>) {
    return callToServer(data['href'], data);
  }
}

interface IUserData extends IModelData {
  username : string;
}

class User extends GenericBase<IUserData> {}

function fn(arg: typeof ModelBase) {};

fn(User);

Another way is to not use typeof ModelBase but instead write out the actual factory type:

interface IModelData {
  href : string; // All models have an href
}

class ModelBase<DataType extends IModelData> {
  persist(data : Readonly<DataType>) {
    return callToServer(data['href'], data);
  }
}

interface IUserData extends IModelData {
  username : string;
}

class User extends ModelBase<IUserData> {}

function fn(arg: new() => ModelBase<IModelData>) {}

fn(User);

With either approach you're ensuring that the arg parameter of fn is a contract that is satisfied by every subclass constructor function.

@mhegazy
Copy link
Contributor

mhegazy commented Sep 12, 2017

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@mhegazy mhegazy closed this as completed Sep 12, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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