Object key signature is not taken into account with destructive assignment on method definition level

[zuzusik]

TypeScript Version: 2.3.2

Code

interface IMyInterface {
  onChange(changes : {[key : string] : {newValue : any, oldValue : any}})
}

class MyClass implements IMyInterface {
  onChange({config}) {
    console.log(config);
  }
}

class MyAnotherClass implements IMyInterface {
  onChange(changes) {
    const {config} = changes;

    console.log(config);
  }
}

Expected behavior:
compiled without errors

Actual behavior:

test.ts(5,7): error TS2420: Class 'MyClass' incorrectly implements interface 'IMyInterface'.
  Types of property 'onChange' are incompatible.
    Type '({config}: { config: any; }) => void' is not assignable to type '(changes: { [key: string]: { newValue: any; oldValue: any; }; }) => any'.
      Types of parameters '__0' and 'changes' are incompatible.
        Type '{ [key: string]: { newValue: any; oldValue: any; }; }' is not assignable to type '{ config: any; }'.
          Property 'config' is missing in type '{ [key: string]: { newValue: any; oldValue: any; }; }'.

Note: this affects Angular >1.5 users when writing component controllers and implementing $onChanges method

[mhegazy]

IMyInterface.onChange says it can be called by any object, as long as its properties have a certain type.

MyClass.onChange has a stricter requirement, saying it has to be called with an object with a property called config.

as a result MyClass is not substituatble for IMyInterface.

The second class MyAnotherClass, says it can be called with any arguments, no requirements here..

If MyClass.onChange does not care if change is not provided, consider making it optional:

class MyClass implements IMyInterface {
  onChange({config = undefined}) {
    console.log(config);
  }
}

or give it an explicit type:

class MyClass implements IMyInterface {
  onChange({config} : any) {
    console.log(config);
  }
}

you can always give type, and you would see a similar error.

class MyAnotherClass implements IMyInterface {
    onChange(changes: { config: number }) {
        const { config } = changes;

        console.log(config);
    }
}

[zuzusik]

MyClass.onChange has a stricter requirement, saying it has to be called with an object with a property called config.

But this doesn't violate typings, and TS says it does:

Property 'config' is missing in type '{ [key: string]: { newValue: any; oldValue: any; }; }'.

doesn't [key : string] mean that it can be any key there (including config)?

[mhegazy]

doesn't [key : string] mean that it can be any key there (including config)?

but it can not be there at all. MyClass.onChange says it has to have config.

[zuzusik]

but again, does it violate typing?
why then const {config} = changes; doesn't say the same Property 'config' is missing in type '{ [key: string]: { newValue: any; oldValue: any; }; }'.?

[mhegazy]

cause changes is declared as any.... so any thing goes. give it a type like { config: number } and you will start seeing errors. use --noImplicitAny to flag these cases.

[zuzusik]

ohh, I though in this case changes aren't any, but { [key: string]: { newValue: any; oldValue: any; }; } because that's how IMyInterface defines them.

should this be matched this way? we are implementing specific method, so it's arguments should be matched with types provided in interface - this sounds logical for me

[mhegazy]

implements clause does not change any thing about your class, it is just an assert that this class has to be assignable to the interface.

[zuzusik]

so, if type is not specified the method's it is considered to be any and so it makes method compliant with interface definition, right?

[mhegazy]

so, if type is not specified the method's it is considered to be any and so it makes method compliant with interface definition, right?

correct. you can find more details about this in #10570

[mhegazy]

Looks like we have some issues here that need to be addressed after all...

interface IMyInterface {
  onChange(changes : {[key : string] : {newValue : any, oldValue : any}})
}

class MyClass implements IMyInterface {  // Error
    onChange({ config }) {
    }
}

class MyClass2 implements IMyInterface {   // OK
    onChange({ config }: { config: any }) {
    }
}

[zuzusik]

Uuufff! Glad that it helped to undercover some real issues at the end. Thanks for detailed explanation. And once again: any is evil :)

[sandersn]

@mhegazy, I think this captures the bug you found without any destructuring. Did I understand it correctly?

interface IMyInterface {
  onChange(changes : {[key : string] : {newValue : any, oldValue : any}})
}

class MyClass2 implements IMyInterface {   // OK, but should be error
    onChange(x: { config: any }) {
    }
}

[mhegazy]

well there are a few issues here we need to understand and find a consistent place for..

these should all behave the same:

interface IMyInterface {
  onChange(changes : {[key : string] : {newValue : any, oldValue : any}})
}

class MyClass1 implements IMyInterface {   // OK, but should be error
    onChange(x: { config: any }) {
    }
}

class MyClass2 implements IMyInterface {   // OK, but should be error
    onChange({ config }) {
    }
}

class MyClass3 implements IMyInterface {   // OK, but should be error
    onChange({ config }: { config: any }) {
    }
}

arguable, remove the error to match things like:

var index: { [key: string]: { newValue: any, oldValue: any } };
var { config } = index;

var tmp = index;
var config = tmp.config;

[sandersn]

Since this is comparing assignability of parameters, there is bivariance. So it shouldn't be an error.