-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
String indexer accepted as var type but not as class member type #178
Comments
Thanks for the report! This is actually behaving according to spec. A similar explanation is given at http://stackoverflow.com/a/22077024/1704166 (the rest of this comment assumes you've read that question+answer or are already familiar with contextual typing) Object literals, by default, do not have a string indexer. However, they can acquire one through contextual typing. I'll walk through each example from the original code: interface ObjectHash {
[key: string]: any;
}
// OK, because the type annotation on 'defaults' provides
// a contextual type on the object literal that initializes it
var defaults: ObjectHash = {
title: "untitled",
}; // This is error, expected, because array literals do not acquire
// an index signature from contextual typing.
defaults = ["a"];
// Would be OK, because assignment statements introduce a contextual
// type on their RHS
defaults = { }; declare class Model {
defaults: ObjectHash
}
// Index signatures of types '{ title: string }' and 'ObjectHash' are incompatibe.
// Why?! Unexpected error.
class MyModel extends Model {
// Initializers of class members do not provide a contextual type,
// even if they are overriding a member of the base class. Possible suggestion?
defaults = {
title: "untitled"
};
} // It works when assigned in the constructor
class MyModel1 extends Model {
constructor() {
// OK because assignment contextually types the RHS
this.defaults = {
title: "untitled"
}
super();
}
} // Trying different way
declare class Model2 {
// 'defaults' is method
defaults(): ObjectHash
}
// Same here. Why is this an error? Is this a bug?
class MyModel2 extends Model2 {
defaults() {
// If defaults() were type-annotated (: ObjectHash), then this would be OK,
// but return statements in overriding methods are not contextually typed
// by the base class. This also might be something we could change?
return {
title: "untitled"
}
}
} |
Thanks Ryan for your good explanation. I come across this every once in a while and every time it takes me a little to get my head around it when I'm pointed to contextual typing. I think it would make sense and is more intuitive for contextual typing to apply when overriding a member of the base class. Same way that it works for assignments. Typing of the member on the base class provides the context. What do you think? Please take this as feature suggestion. |
I'll try to write up a suggestion proposal and link this over |
Awesome! Thanks! Looks like until this feature is implemented there is no straightforward way to strong-type derived class members based on indexers unless the overriding members are explicitly typed with the same type as the member in base class (which feels redundant). Like this case: class MyModel extends Model {
// member already typed as 'ObjectHash' in base class but
// have to declare typing here again for compiler to pass
defaults: ObjectHash = {
title: "untitled"
};
} and this: class MyModel2 extends Model2 {
// member already typed as 'ObjectHash' in base class but
// have to declare typing here again for compiler to pass
defaults(): ObjectHash {
return {
title: "untitled"
}
}
} Fixing this can open up many typing scenarios, for example, in interface EventsHash {
[selector: string]: (eventObject: JQueryEventObject) => void;
}
declare class View {
events(): EventsHash;
delegateEvents(events?: EventsHash): any;
} and type of interface RoutesHash {
[routePattern: string]: (...urlParts: string[]) => void;
}
declare class Route {
route(): RoutesHash;
} which is currently not possible without forcing everyone to explicitly type those members in their derived classes. That would be a huge unnecessary breaking change. |
Hi,
I think this is a bug. Please see comments for details:
The text was updated successfully, but these errors were encountered: