You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Conditional type check on type inferred from mapped type false negative #33366
// Simple mapped type and inferrencetypeMapped<T>={[KinkeyofT]: {name: T[K]}};typeInferFromMapped<T>=TextendsMapped<infer R> ? R : never;// Object literal type and associated mapped type// Note that in the real code we don't have a direct reference to LiteralTypetypeLiteralType={first: "first";second: "second";}typeMappedLiteralType={first: {name: "first"},second: {name: "second"},};typeInferred=InferFromMapped<MappedLiteralType>;// UNEXPECTED resolves to falsetypeTest1=InferredextendsRecord<any,string> ? true : false;// true as expectedtypeTest2=LiteralTypeextendsRecord<any,string> ? true : false;// true as expected; currently using this as a workaroundtypeTest3=InferredextendsRecord<keyofInferred,string> ? true : false;
Expected behavior:
As Inferred and LiteralType are equivalent, we would expect Inferred extends Record<any, string> to resolve to true just as the case with LiteralType does.
Actual behavior:
Test1 resolves to false. This has the same behavior for similar expressions of the record (e.g. Record<string, string>, { [key: string]: string }); interestingly though, if we write the record Record<keyof Inferred, string> we get the expected behavior, this is the current workaround I'm using.
We also get the expected behavior if we instead defined MappedLiteralType as Mapped<LiteralType>. However in the code I encountered this, we have no direct access to LiteralType, only Inferred; I added LiteralType to the example only for demonstration of the expected behavior.
I looked into this a bit more, from what I can tell the issue is in isObjectTypeWithInferableIndex (in checker.ts):
/** * Return true if type was inferred from an object literal, written as an object type literal, or is the shape of a module * with no call or construct signatures. */functionisObjectTypeWithInferableIndex(type: Type){returntype.symbol&&(type.symbol.flags&(SymbolFlags.ObjectLiteral|SymbolFlags.TypeLiteral|SymbolFlags.Enum|SymbolFlags.ValueModule))!==0&&!typeHasCallOrConstructSignatures(type);}
This method is called when comparing the indexes in indexTypesRelatedTo, and returns false for the instantiated ReverseMappedType because its symbol is undefined. Changing it the following fixes the issue (with no failing tests)
/** * Return true if type was inferred from an object literal, written as an object type literal, or is the shape of a module * with no call or construct signatures. */functionisObjectTypeWithInferableIndex(type: Type){// ReverseMappedType does not have a symbol but will always have an inferable indexif(getObjectFlags(type)&ObjectFlags.ReverseMapped){returntrue;}returntype.symbol&&(type.symbol.flags&(SymbolFlags.ObjectLiteral|SymbolFlags.TypeLiteral|SymbolFlags.Enum|SymbolFlags.ValueModule))!==0&&!typeHasCallOrConstructSignatures(type);}
I don't know if this is the best approach, one other way I tried which also seems to work is to add a TypeLiteral symbol to the ReverseMappedType when it is created in createReverseMappedType:
This approach seems a bit better to me; though it does seem a bit weird to flag an inferred mapped type as a TypeLiteral, a MappedType already has this flag so perhaps it is alright. You could also take this approach, but instead with an entirely separate new SymbolFlag just for this case. (this does not seem like a great idea to me but there might be issues I'm unaware of in putting TypeLiteral on ReverseMappedType)
I would appreciate the perspective of someone is more familiar with any potential implications of these fixes, if all looks good I can make a pr with the change alongside a test.
Khalid Sabry (@ksabry) close! isObjectTypeWithInferableIndex is definitely what needed to be changed, specifically, for a reverse mapped type, we needed to decide that if the index on the reverse mapped type source type was inferable, the index on the reverse mapped type itself is. I've already opened #33450 with the fix - thanks for the help!
Activity
jack-williams commentedon Sep 11, 2019
Definitely looks like some index signatures on not getting set when inferring from mapped types. The issue here is orthogonal to conditional types:
Furthermore, the following works correctly:
ksabry commentedon Sep 13, 2019
I looked into this a bit more, from what I can tell the issue is in
isObjectTypeWithInferableIndex(in checker.ts):This method is called when comparing the indexes in
indexTypesRelatedTo, and returns false for the instantiatedReverseMappedTypebecause its symbol is undefined. Changing it the following fixes the issue (with no failing tests)I don't know if this is the best approach, one other way I tried which also seems to work is to add a
TypeLiteralsymbol to the ReverseMappedType when it is created increateReverseMappedType:->
This approach seems a bit better to me; though it does seem a bit weird to flag an inferred mapped type as a TypeLiteral, a MappedType already has this flag so perhaps it is alright. You could also take this approach, but instead with an entirely separate new SymbolFlag just for this case. (this does not seem like a great idea to me but there might be issues I'm unaware of in putting TypeLiteral on ReverseMappedType)
I would appreciate the perspective of someone is more familiar with any potential implications of these fixes, if all looks good I can make a pr with the change alongside a test.
weswigham commentedon Sep 16, 2019
Khalid Sabry (@ksabry) close!
isObjectTypeWithInferableIndexis definitely what needed to be changed, specifically, for a reverse mapped type, we needed to decide that if the index on the reverse mapped type source type was inferable, the index on the reverse mapped type itself is. I've already opened #33450 with the fix - thanks for the help!