-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Optional chaining union type with interfaces #35263
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
Comments
But it would not be safe to assume that
|
In this case (if we are talking about I probably understand how union types work and why currently this behavior happens but all in all I have a feeling that I'm still correct with optional chaining at whole. Just try look at the example from the standpoint of data coming to the |
Sure, but then you also have somewhere:
It is compatible to TypeScript has structural typing, so you always have to assume there are more unknown properties. Assuming that What you want is not type-safe. |
Why it is not safe? Such construction in case when city is defined just narrows types for |
Providing an example of what I mean, based on your code. No additional interfaces are declared.
With your suggestion you could access |
WHY ??? :) It's type
|
Why do you think that? How could the compiler infer the |
Hm, so I was mislead by you. I thought you introduced C and D and added them to union type |
Yeah, I'm sorry about that. That's why I wrote "no additional interfaces are declared". :-) Here's a link to the full example on the Playground.
What kind of error? This can't be a compilation error, because all types are fully compatible. |
Hm. We are talking about case when optional chaining can calculate type for |
Typescript must complain about such situation, isn't it? :) |
It's just a "hack" to make it work with current features, but the result would be absolutely the same as with optional chaining.
The only type information for
I really don't know what you think the bug is here. Please try to be more explicit in what exactly you think should be happening. It's all valid TypeScript code and works as it should. I'm afraid I can't describe any simpler why this is a really bad idea and not type-safe at all. If you still don't understand the problem, then we'll have to wait for other people to chime in. :-) |
Ok, thanks for your time and patience. Can we remain this issue open to allow others to chime in?:) |
Sure, absolutely. I'm no moderator and not part of the TypeScript team. I'd still love to know what you think is a bug and what should be happening: #35263 (comment) |
If thinking in terms of data then it's quite okay to assume that |
Sure. |
TypeScript has a structural type-system. So if the structure is compatible, it can be passed around.
The object That's why it can be passed along just fine, because |
That doesn't matter. A union That's why you can only access properties that are present in both types ( And this is the fundamental reason why I'm against this issue, and why I think you're not understanding my concerns. |
Thanks for explanation. But again, in this case as If it can guess type by value of existing property why typescript can't guess type by presence of property? |
But at runtime it will have that property, so at runtime the conditional operator will access that property just fine. And at runtime it will have the wrong type. Anyway, I'm out now. It's clear that I can't describe the issue comprehensible to you. :-( I'm sorry. |
No problem. Sorry for taking your time |
btw, you might have missed this :) |
Duplicate of #33736 ? |
not fully sure |
FYI people can comment on closed issues; a GitHub issue being closed does not prevent people from seeing it or commenting. We use open/closed to track "Is there concrete engineering work to do here". Anyway after looking at the examples here, everything you've shown is the intended behavior. I think Stack Overflow or another venue might be a better forum for any future questions here - the issue tracker is for bugs; the odds that something that surprises you is because of a long-hidden bug in type relationships is really quite low. Hope that helps! |
i understood u means but isnt that a bug or problem with typescript? pls check this code @RyanCavanaugh i think this should be a feature request and a bug fix |
I was thinking the same that @mdbetancourt posted but with the first example. interface Common {
name: string
age: number
}
interface A extends Common{
address: string
}
interface B extends Common{
city: string
}
interface C extends A {
city: number;
}
type MyType = A | B
const func = (arg: MyType) => {
const res = 'city' in arg? arg.city : undefined;
}
const c: C = {
name: 'Bob',
age: 50,
address: 'Fake street',
city: 200
};
func(c);
|
The difference in behavior is intentional, because these are different syntaxes. Having different behavior do different things is how you can write different programs. You might legitimately write if (foo.length) { as a shorthand for if ("length" in foo) { Here it's obvious that you're trying to type-test, and even though it isn't 100.0% correct, if we stopped you from writing this the next thing you're going to do is clearly to write a type assertion, so there's really no point. |
Oh I understand now. Makes sense that |
If anyone else is looking for this, here is a hacky workaround. type OptionalUnion<T1, T2> = {
[P in keyof Omit<T1 & T2, keyof (T1 | T2)>]?: (T1 & T2)[P]
} & (T1 | T2)
interface Common {
name: string
age: number
}
interface A extends Common{
address: string
}
interface B extends Common{
city: string
}
type MyType = OptionalUnion<A, B>
const func = (arg?: MyType) => {
// (property) B.city?: string | undefined
const res = arg?.city
} |
It's a bit odd to me to close this with the reasoning that this is intented behavior. It might be intented but it's not expected and there is currently no nice solution apart from just casting it to something specific. The expectation is that on a Union type the optional chaining would only check that at least one of the options fullfills the chaining, not all of them. Would love for this to be reopened. I think the examples on here already speak for themselves. The |
With type checking as is, simple code becomes unnecessarily convoluted, e.g. What could be: const assignmentKeyEqualsValue =
parent?.key.name === parent?.value?.name; becomes: const assignmentKeyEqualsValue =
'key' in parent &&
'name' in parent.key &&
'name' in parent.value &&
parent.key.name === parent.value.name; For no benefit as far as I can tell. |
Another way to solve this in a very type-safe way (it being 100.0% correct), would be to define the 'missing' properties on all the types that make up the union, setting them to interface Common {
name: string
age: number
}
interface A extends Common{
address: string
city?: never
}
interface B extends Common{
city: string
address?: never // not strictly necessary for this example
}
type MyType = A | B
const func = (arg?: MyType) => {
const res = arg?.city // Works like expected and intended
} This works because we have eliminated the possibility of it being any other type than string, when it does in fact exist. const myObj = {
name: 'Bob',
age: 50,
address: 'Fake street',
city: 200
};
func(myObj) // Errors because it should This is also more readable than the hacky workaround mentioned above imho (and did I mention it is more type-safe). |
@KantiKuijk This is not "100 % correct ". A property typed |
@MartinJohns You are right. I was referring to this comment with my 100.0% sentence, I should have maybe skipped that pun. |
This comment was marked as resolved.
This comment was marked as resolved.
My bad @jcalz, I changed my earlier comment, thanks! |
TypeScript Version: 3.7.2
Search Terms:
optional chaining
optional chaining union
Code
Expected behavior:
Optional chaining works well in this situation as
arg
might be ofB
type.Actual behavior:
Fails with TS error
city does not exist on type A
.Playground Link: http://www.typescriptlang.org/play/?ssl=1&ssc=1&pln=21&pc=1#code/JYOwLgpgTgZghgYwgAgMIHsC2n0mQbwChkTkQ5MIAuZAZzClAHNjS4nqyBXTAI2kIBfQoVCRYiFAEFkEAB6QQAE1posOEEVLI4SpVAi1aNeoxAtho8NHhJkAIVkKIy1Rmy4tpBMDABPEwZmIRFCfwAHFABZPwAVP0jkAF5kGQAfBxEEXHpkGC4QBGTkAAo4KCYAfhoY+MiASmSAPgJWEmyQXINVFPKqgDoffxDCIA
Related Issues: no
The text was updated successfully, but these errors were encountered: