-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Add 'never' type #8652
Add 'never' type #8652
Conversation
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 |
@DanielRosenwasser The primary use for |
The problem with function f(x: A | B) {
if(isA(x)) {
} else if(isB(x)) {
} else {
x; // appears as type 'never'
}
} |
My objection to Other than that, and bike shedding on the name, the code looks good. |
Agreed with Nathan - the code looks fine, but I have those gripes with the naming. |
Yes, but " |
Interesting. That makes it sounds intended that none of the tests explicitly mention their return type is |
No, we can't get away with |
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 |
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 |
@mhegazy Yes, but an easy fix is to add a |
we said we will be more careful in the future with such changes. |
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. |
Being more precise is one issue. Generating a .d.ts that is not consumable is another. |
I'm not sure where you're going with this. What are you proposing? |
@ahejlsberg I've just submitted an issue (#8655) about better type analysis around |
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? |
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 |
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 |
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 |
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 |
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 |
Discussed with @mhegazy. We're good with documenting this as a breaking change for .d.ts emit and will also document the workaround (add |
What a nice feature! I also quite like the name. |
Note for breaking changes docs: Example: class Base {
method() {
throw new Error("Not Implemented!");
}
}
class Derived extends Base {
method() {
return 0;
}
}
// 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 |
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. |
how about naming it as "impossible" instead of "never" |
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 |
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. |
What's the point? |
@fightingcat the point is that TypeScript lacked a bottom type and that made it difficult to properly enforce parts of a type system. |
@remojansen please use Stack Overflow for questions -- thanks! |
@RyanCavanaugh sorry my mistake will move it to SO. |
This PR introduces a
never
type that represents the type of values that never occur. Specifically,never
is the return type for functions that never return andnever
is the type of variables under type guards that are never true. Thenever
type has the following characteristics:never
is a subtype of and assignable to every type.never
(exceptnever
itself).return
statements, or onlyreturn
statements 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
.never
return type annotation, allreturn
statements (if any) must have expressions of typenever
and the end point of the function must not be reachable.Because
never
is 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
never
type replaces thenothing
type introduced in #8340 and it is effectively the same as thebottom
type proposed in #3076.Some examples of functions returning
never
:Some examples of use of functions returning
never
:Because
never
is assignable to every type, a function returningnever
can be used when a callback returning a more specific type is required:Fixes #3076.
Fixes #8602.