- Notifications
You must be signed in to change notification settings - Fork13.2k
Add 'never' type#8652
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Add 'never' type#8652
Conversation
DanielRosenwasser commentedMay 17, 2016
I really don't think this name is good for the general user experience. My feeling is that it seems to model several different things which the name doesn't reflect very well on. At least |
ahejlsberg commentedMay 17, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@DanielRosenwasser The primary use for |
RyanCavanaugh commentedMay 17, 2016
The problem with functionf(x:A|B){if(isA(x)){}elseif(isB(x)){}else{x;// appears as type 'never'}} |
sandersn commentedMay 17, 2016
My objection to Other than that, and bike shedding on the name, the code looks good. |
DanielRosenwasser commentedMay 17, 2016
Agreed with Nathan - the code looks fine, but I have those gripes with the naming. |
ahejlsberg commentedMay 17, 2016
Yes, but " |
sandersn commentedMay 17, 2016
Interesting. That makes it sounds intended that none of the tests explicitly mention their return type is |
ahejlsberg commentedMay 17, 2016
No, we can't get away with |
sandersn commentedMay 17, 2016
OK, that makes sense. The d.ts is a good enough reason to have the keyword, although I noticed the rest -- outside a d.ts -- are all error cases where the annotation specifies |
mhegazy commentedMay 17, 2016
This will be a breaking change for back compat for .d.ts emit. in the past we inferred the type of these methods/functions as |
ahejlsberg commentedMay 18, 2016
@mhegazy Yes, but an easy fix is to add a |
mhegazy commentedMay 18, 2016
we said we will be more careful in the future with such changes. |
ahejlsberg commentedMay 18, 2016
I don't see a big issue here. We're being more precise in our type analysis and that may change the outcome of inference. There are other such issues already resulting from the control flow based type analysis (e.g. narrowing occurs in more places which might affect inferred return types). Effectively we can only replicate the old results by keeping the old code and not changing anything. I think it is reasonable to recommend explicit type annotations if you want to freeze the shape of an API. |
mhegazy commentedMay 18, 2016
Being more precise is one issue. Generating a .d.ts that is not consumable is another. |
ahejlsberg commentedMay 18, 2016
I'm not sure where you're going with this. What are you proposing? |
yortus commentedMay 18, 2016
@ahejlsberg I've just submitted an issue (#8655) about better type analysis around |
zpdDG4gta8XKpMCd commentedMay 18, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Sorry for not minding my own business. The official 1.8. has its *.d.ts fixed and shipped, whoever needs it is welcome to get it anytime. Technically speaking nightly build is 1.9.+ with a bump in the major version number which means breaking changes are likely. Can't see any crime here. People who are scared to break their code are warned to proceed with caution or stay where they are. Am I missing the point? |
kitsonk commentedMay 18, 2016
It isn't only the TypeScript team that generates Both@ahejlsberg and@mhegazy have points. My opinion is if this lands for 2.0, there is the |
kitsonk commentedMay 18, 2016
These people (for disclosure, I am the lead for Dojo 2) issue
But instead of acknowledging that there is potentially more breakage than just what version of the I clearly indicated that both Anders and Mohammad had valid points, of which you seemed to suggest in your original statement confusion about why Mohammad was concerned. I am glad he is concerned. I know both Anders and Mohammad consider downstream projects like Dojo 2 and others in their decisions. Ryan further added though that valid point that this isn't the first breakage of definition files that force the larger community to upgrade. I would argue though that intersection and union types were opt-in, so in theory downstream projects could have chosen not to use them, but they were actually key enablers and so I know we chose to opt-in to them. Clearly the compromise of emitting a |
zpdDG4gta8XKpMCd commentedMay 18, 2016
Where did I say that? What I said is: it's a purely engineering problem, and this is what we, engineers, do for living: solve them, and not to seem too arrogant I even outlined how it can be done in typical cases. Now if the budget of TS is big enough to take on supporting backward compatibility for every team that might benefit of it, I am all for it. Doubtfully so. More realistically the budget of TS can be better spent on developing new features merits of which far outweigh the troubles of adopting them. And this is what I am rooting for. Lastly please give an example how |
ahejlsberg commentedMay 18, 2016
I'm not sure generating We already have a breaking change in 2.0 with Alternative solutions are:
I think both of those are worse than just introducing the change. There is real value in having inference produce |
mhegazy commentedMay 18, 2016
In#8252 we said we will add this to our "feature checklist". i am just bringing this up as a violation of this. Given that we rejected the sourceVersion suggestion earlier this weeks, I believe there are two options here, the first is the one you mentioned (that also should apply to |
ahejlsberg commentedMay 18, 2016
Discussed with@mhegazy. We're good with documenting this as a breaking change for .d.ts emit and will also document the workaround (add |
JsonFreeman commentedMay 20, 2016
What a nice feature! I also quite like the name. |
mhegazy commentedMay 20, 2016
Note for breaking changes docs: Example: classBase{method(){thrownewError("Not Implemented!");}}classDerivedextendsBase{method(){return0;}}// error TS2415: Class 'Derived' incorrectly extends base class 'Base'.// Types of property 'method' are incompatible.// Type '() => number' is not assignable to type '() => never'.// Type 'number' is not assignable to type 'never'. Recommendation: Give the base class method a type annotation of the expected return type, be it |
ahejlsberg commentedMay 23, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
With#8767 we have modified the inference rules to fix the breaking change above. I think we can just consider this a new feature and remove the breaking change label. |
usta commentedJun 2, 2016
how about naming it as "impossible" instead of "never" |
kostat commentedJun 2, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I understand that the primary reason to call the type "never" derives from the desire to facilitate the reading the never returning function declaration. For me, it does not reflect the essence of the type. Look:
On the other hand the common definition of such code is unreachable code block. Wouldn't it be better to name the type "unreachable"? I understand that no one will ever declare a variable of 'unreachable' type, but type is type. When I see |
kitsonk commentedJun 2, 2016
Considering this is already merged and documented, all we are doing is 🚲 🏠 and navel gazing something is a semantic opinion. The naming was already debated and settled. |
YowaiCoder commentedJul 20, 2016
What's the point? |
kitsonk commentedJul 21, 2016
@fightingcat the point is that TypeScript lacked abottom type and that made it difficult to properly enforce parts of a type system. |
RyanCavanaugh commentedOct 25, 2016
@remojansen please use Stack Overflow for questions -- thanks! |
remojansen commentedOct 25, 2016
@RyanCavanaugh sorry my mistake will move it to SO. |
Uh oh!
There was an error while loading.Please reload this page.
This PR introduces a
nevertype that represents the type of values that never occur. Specifically,neveris the return type for functions that never return andneveris the type of variables under type guards that are never true. Thenevertype has the following characteristics:neveris a subtype of and assignable to every type.never(exceptneveritself).returnstatements, or onlyreturnstatements with expressions of typenever, and if the end point of the function is not reachable (as determined by control flow analysis), the inferred return type for the function isnever.neverreturn type annotation, allreturnstatements (if any) must have expressions of typeneverand the end point of the function must not be reachable.Because
neveris a subtype of every type, it is always omitted from union types and it is ignored in function return type inference as long as there are other types being returned.The
nevertype replaces thenothingtype introduced in#8340 and it is effectively the same as thebottomtype proposed in#3076.Some examples of functions returning
never:Some examples of use of functions returning
never:Because
neveris assignable to every type, a function returningnevercan be used when a callback returning a more specific type is required:Fixes#3076.
Fixes#8602.