Repository navigation
Typeguard for union not working after adding a property to the superclass of one of the union members #35953
Description
This issue appeared in eslint-plugin-jest after updating @typescript-eslint to 2.13.0 (from 2.12.0).
TypeScript Version: 3.7.x-dev.201xxxxx
Search Terms:
typeguard, union
Code
export declare enum AST_NODE_TYPES {
Literal = 'Literal',
TemplateLiteral = 'TemplateLiteral',
}
export type TSNode = StringLiteral | TemplateLiteral;
export interface BaseNode {
stuff: null;
}
export interface LiteralBase extends BaseNode {
raw: string;
value: boolean | number | RegExp | string | null;
regex?: {
pattern: string;
flags: string;
};
}
export interface StringLiteral extends LiteralBase {
type: AST_NODE_TYPES.Literal;
value: string;
}
export interface TemplateLiteral extends BaseNode {
type: AST_NODE_TYPES.TemplateLiteral;
quasis: string[];
}
interface MyStringLiteral<Value extends string = string> extends StringLiteral {
value: Value;
}
interface MyTemplateLiteral<Value extends string = string>
extends TemplateLiteral {
quasis: [Value];
}
type MyStringNode<S extends string = string> =
| MyStringLiteral<S>
| MyTemplateLiteral<S>;
declare function isMyStringNode<V extends string>(
node: TSNode,
specifics?: V,
): node is MyStringNode<V>;
function foo(argument: TSNode) {
if (isMyStringNode(argument)) {
/*
Expected: argument.type is Literal|TemplateLiteral
Actual : argument.type is Literal
This condition will always return 'false' since the types
'AST_NODE_TYPES.Literal' and
'AST_NODE_TYPES.TemplateLiteral' have no overlap.
*/
if (argument.type === AST_NODE_TYPES.TemplateLiteral) {
}
}
}This is a real life reproduction - it can be made smaller by removing the generics, BaseNode, etc, but then it becomes counterproductive as you can argue the solution to be "just use StringLiteral directly".
Expected behavior:
No errors
Actual behavior:
if (argument.type === AST_NODE_TYPES.TemplateLiteral) { is marked as an error, as TS believes argument.type can only be AST_NODE_TYPES.Literal (aka it appears to discard the alternative in the union).
This behaviour is tied to the existence of the value: string property on StringLiteral: if you remove that property, everything works as expected (which is what changed between 2.12.0 & 2.13.0: previously Literal was just an interface w/ type - now it's a union of <Type>Literal interfaces; one for each of the values of LiteralBase#value)
Related Issues:
This is probably a duplicate of #31156, but I've created a new issue as I believe this is a more subtle (ideally fixable) issue
Activity
G-Rath commentedon Jan 2, 2020
Brad Zacher (@bradzacher): you might be interested in seeing this. I don't think there's much
@typescript-eslintcan do, as the issue seems to be the introduction ofvalue: stringonStringLiteral- so it's not even possible forts-estreeto implement the generic itself.AnyhowStep: thanks for the help distilling this into the playground reproduction, and helping double-check my sanity :) I think the reason that
valuecauses the error is that it makes it a tagged type, thus leading into the territory that your original issue was in and so unlikely to be fixable :(RyanCavanaugh commentedon Jan 14, 2020
Did you mean
quasis: string[]instead of[string]by chance? If not, buckle up, because you're in for a ride.What's going on here?
TypeScript says that if you have a UDTG and apply it to some union, then the primary intent of that is to probably to do some filtering operation like this:
If the checked-for type is a subtype of the output of that process, we also narrow to exactly that:
Note that I said "subtype", not "assignable" to. In the OP example,
MyTemplateLiteralis assignable toTemplateLiteral, but is not a subtype of it, due to the tuple member (was this intentional?). However,MyStringLiteralis a subtype of (and assignable to) its based typeStringLiteral, because itsvaluetype is a subtype of its corresponding base type member's.So in your case, TypeScript saw a list of types in a union, filtered them to those that were subtypes, saw that at least one fit, and then didn't bother checking for assignability of the remaining constituents of the union:
This is an important bit of logic, because many things are an assignability target but not a subtype target. For example, if you just hack up the compiler to always use the assignability relation, you can easily introduce new errors:
This has been extremely delicately tweaked over the years to try to produce the most intuitive behavior in other cases, so I'm not sure what the right answer is at this point. Any change is going to be a breaking change for other code. For your specific case, I think the only great answer would either be a type assertion, or to fine-tune your interfaces such that they uniformly do or don't have different assignability/subtyping relationships to their parent types.
RyanCavanaugh commentedon Jan 15, 2020
Anders Hejlsberg (@ahejlsberg) any additional thoughts? I don't see much room for improvement, unfortunately
G-Rath commentedon Jan 15, 2020
Ryan Cavanaugh (@RyanCavanaugh) thanks for such a fantastic write up - I'd be lying if I claimed to understand it completely, but I think I got the bulk of it well enough.
Sadly, the tuple member was intentional, but not strictly required as is: I'm trying to represent that we know for sure the value of the first element in that tuple; if there was a way to represent "array of strings, of which the first value is
T" that'd be fine, but I suspect even if that could be done it might not be a subtype?(btw in case it matters, the actual type of the elements in the tuple is
TSESTree.TemplateElement & { value: { raw: Value; cooked: Value } }.)I'm not sure I understand why the removing of
value: stringfromStringLiteralresolves the problem however; isn'tValuea valid subtype ofboolean | number | RegExp | string | null? (the type ofvalueinherited fromLiteralBase?Regardless, thanks again for the detailed response - I'll have to have a think about the best way to tackle in our codebase :)
Just as a toss away thought:
I guess a solution could be useful to have some form of as-typeguard a la
function x(v: any): v as ythat would make casts slightly safer by typing them to a single point & some logic, but I doubt this is a big enough problem to warrant adding a new feature.bradzacher commentedon Jan 15, 2020
It's moments like this I realise I know a lot less about typescript than I think I do...
repl
G-Rath commentedon Jan 15, 2020
I feel very silly, b/c I tried this quickly but missed out the
[]and so for some reason jumped to "it only works with generics" instead of diving deeper 😂Thanks for not letting me get away with that :)
Sadly, as I suspected
[V, ...string[][]is not a subtype ofstring[], and so doesn't solve the issue :(AnyhowStep commentedon Jan 15, 2020
Ryan Cavanaugh (@RyanCavanaugh) If I understand what I'm reading,
Subtype implies assignable, but assignable does not imply subtype?
Does this mean conditional types only check "assignability" and not "subtype" relations?
A extends B ? "y" : "n"checks ifAis "assignable" toBbut not if it is a "subtype".Does this mean there is no way to check "subtype" relations from within the type system? (Like a "subtype" version of conditional types)
Type guards narrow based on subtype relations, but not assignability?Type guards narrow first on subtype relations, assignability second?
Since we can narrow
PlanttoTree, but notAnimal|PlanttoCat|Tree.Hence, your advice,
AnyhowStep commentedon Jan 20, 2020
Just answering some of my questions,
https://www.typescriptlang.org/docs/handbook/advanced-types.html#conditional-types
https://www.typescriptlang.org/docs/handbook/type-compatibility.html#subtype-vs-assignment