-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Type inference in switch statements based on class token #17397
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
if
statement
Type discriminants can only be unit types - types for which there is exactly one value. For example, For rather obvious reasons, it's only safe to discriminate on the basis of unit types. For example, this declaration is legal and completely breaks the export class SetLoginUser2 extends SetLoginUser {
readonly type = SetLoginUser2;
constructor(public user: number) { super(user) }
} The class constructor function (or any function for that matter) is not a unit type. There may be multiple constructor functions that return the same class (sounds weird but it is true) so we don't consider the |
But using the string keys violates liskov substutability too doesn't it? So that's not a very good reason.
I actually came across this recently when I asserted that I didn't think that this inference could be made (and webstorm 2016.? agreed with me coincidentally) but was shot down by a developer in another issue against another repo when I was shown that webstorm was buggy and that this was the typescript behaviour.
Now I agree that a function is not a unit type, but a module, function pair is a unit type, isn't it? There can only be one interface Type<T> {
<module_x>;
new (...args): T;
} Then that type, would be a unit type? But it's impossible to express "I belong to this module" in the type system (nor would I think you want to), so Angular uses this pattern everywhere in their DI framework and as far as I know it hasn't caused any real ambiguity (sure, there's theoretical ambiguity due to |
Sorry, no I'm 100% wrong as usual. I didn't check whether the first example was actually valid typescript before posting -- but now that I understand that the language prevents overriding a "unit typed" readonly declaration in a subclass your example became a whole lot more straight forward. Yes, working as intended. You guys think about this stuff far more than I do. |
Wait, so if it was possible to declare a final/sealed class in typescript, then this would this be possible? Obviously I'm not saying "add sealed classes" or anything, just wondering... |
Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed. |
Could you at least answer my question about "final" types? I understand why this issue was closed. I also understand that final types are unlikely to ever appear in typescript because any class can be "implemented" by an arbitrary object, so I'm not going to go open a new issue if you reply "yes, that's right". It's merely for my own edification. |
Final classes are #8306, which is currently tagged as Won't Fix. |
Yeah, that doesn't answer the question I asked about them at all. I'm not interested in seeing them implemented, I just wanted to know that if they were implemented, would it mean you'd be able to treat a "final" class as a unit type. Or, since apparently you can implement them using a private constructor (according to #8306), does that mean that types with a private constructor can be used as unit types and therefore it's possible to apply the inference I originally suggested to them? But thanks I guess... |
That is a "how long is a piece of string question"... You seem to be asking, if something that is not currently planned to be implemented was implemented how would it work. |
Basically why I'm asking is that we use this pattern a lot in our angular code base, and it's never caused us problems (although we enforce a pretty strict "interface, don't subclass" policy). I've got a pending issue in our issue tracker which says we should change any occurrences of this because typescript team thinks it's stupid, but I wanted to know whether it could be changed in order to make it more "correct". I don't think we'll be making all our constructors private any time soon (lol), but since we treat every class as implicitly final, I just want to know that we're not causing ourselves problems down the road. The type inference would just be a convenience, we're fine with typecasting in the case statements. ps. I don't think it's a "how long is a piece of string question", what I'm asking is that if our internal coding style prohibits using the "extends" keyword then can we safely assume that the inference I described above holds (even if we have to manually cast the result), or is the pattern broken for some other reason? |
@ovangle did you try to use constructor based switching https://github.com/ngrx/example-app/issues/110#issuecomment-326866631 ( |
TypeScript Version: 2.4.2
Code
Some common interfaces used in code examples below:
Typescript infers the correct type to use in a case statement, if the switch is on a
readonly
property of the object and the value isconst
. e.g.Expected behavior:
Since typescript is able to infer the type based on the value of a string constant, I'd expect it to infer the type based on the type token as well,
Actual behaviour:
However, it fails with a TS2339 error. A class/function definition is effectively
const
, isn't it? So why shouldn't it work in exactly the same way as a explicitly typedconst
value?The text was updated successfully, but these errors were encountered: