-
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
Implicit index signatures #7029
Conversation
…roperties are present
@@ -6263,6 +6265,15 @@ namespace ts { | |||
return !!(type.flags & TypeFlags.Tuple); | |||
} | |||
|
|||
/** | |||
* Return true if type was inferred from an object literal or written as an object type literal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... " with no call/construct signatures"
👍 |
1 similar comment
👍 |
@@ -0,0 +1,15 @@ | |||
tests/cases/compiler/contextualTypeAny.ts(3,5): error TS2322: Type '{ p: string; q: any; }' is not assignable to type '{ [s: string]: number; }'. | |||
Property 'p' is incompatible with index signature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you not report the index signature here? This is probably confusing for users, especially if the index signature type ever gets cut out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dunno. I think it is pretty decent the way it is now and the last line makes it completely clear what isn't compatible with what.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not clear what the type of p
is without the full type, so it'd be better to just report the index signature given that it'd take relatively little effort on our part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I don't see how it would be better if we didn't show the index signature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but the whole point of our elaboration scheme is to not try to show everything on one line. The respective types of p
and the index signature follow in the next elaboration. It's completely analogous to how we report a mismatch between two properties.
Sad that I'm not going to rake in 30-100 Stack Overflow rep every week for my answer on this one anymore. |
@RyanCavanaugh Heh. |
I just tried this out. Shouldn't a type parameter inherit the index signature? I have the following class: export abstract class Component<P, T extends { [index: string]: string }, E extends { [element: string]: DOMElement }> extends EventEmitter { And I was hoping I could just write: export abstract class MyComponent<P, T, E> extends Component<P, T, E> { And it would just inherit the index signature. But instead I need to explicitly write the constrained index signature. Which is quite painful, since I have many subclasses of |
Ok seems like this PR doesn't work for type arguments? Just passing a type, as a type argument, with all members satisfying the index signature doesn't work for me. |
I would also love to have what @tinganho wants. My use case is the following: interface ITest {
[x: string]: string
}
class Test<T extends ITest> {
content: T
}
const myObject = {
test: "hello"
};
class MyTest extends Test<typeof myObject> {
// Error, 'typeof myObject' doesn't implement the index signature...
method() {
console.log(this.content.test)
}
} I'm planning to do this extensively with model-generated object litterals (like If you want to keep forcing the index signature to be respecified in a derived interface, maybe then my issue could be fixed by |
Asked this in #14736 Why this case: function httpService(path: string, headers: { [x: string]: string }) { }
interface MyHeaders {
'Content-Type': string
}
const headers: MyHeaders = {
"Content-Type": "application/x-www-form-urlencoded"
};
httpService("", { "Content-Type": "application/x-www-form-urlencoded" }); // Ok
httpService("", headers); // NOT OK is NOT valid, the error is:
|
Fixes #6041.
With this PR an object literal type is assignable to a type with an index signature if all known properties in the object literal are assignable to that index signature. This makes it possible to pass a variable that was initialized with an object literal as parameter to a function that expects a map or dictionary:
The PR adds the following three assignment compatibility rules (where an object literal type is the inferred type of an object literal or a type declared using an object type literal with no call or construct signatures):
The PR adds corresponding type inference rules.
The PR furthermore removes the rule that adds index signatures to an object literal when it is contextually typed by a type that has index signatures. Instead, the type inferred for an object literal has index signatures only if it contains computed properties. For example:
In the above, the type of
o
is{ [x: string]: Date | Error, [x: number]: Error }
.Removing the automatic contextual index signatures from object literal types has the nice effect of reducing noise in our error messages. Also, we can now report the name of the offending property when an object literal doesn't meet the constraint implied by a target index signature (previously you'd have to deduce it yourself from two incompatible union types).