- Notifications
You must be signed in to change notification settings - Fork13.2k
New 'unknown' top type#24439
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
New 'unknown' top type#24439
Uh oh!
There was an error while loading.Please reload this page.
Conversation
RyanCavanaugh commentedMay 29, 2018
Where'd we end up with mapped conditional types w.r.t |
weswigham commentedMay 29, 2018 • 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.
Does that make sense (is this derived from something else)? If |
ahejlsberg commentedMay 29, 2018
@weswigham No, that probably doesn't make sense. We should be similar to |
ahejlsberg commentedMay 29, 2018
@RyanCavanaugh Do you mean the behavior of |
| // Functions with with an explicitly specified 'void' or 'any' return type don't need any return expressions. | ||
| if (returnType && maybeTypeOfKind(returnType, TypeFlags.Any | TypeFlags.Void)) { | ||
| if (returnType && maybeTypeOfKind(returnType, TypeFlags.AnyOrUnknown | TypeFlags.Void)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think a function returning an explicitunknown should probably have return values? Otherwise you should've writtenvoid orundefined, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Not sure about that.undefined is in the domain ofunknown (just like it is in the domain ofany) and that's really what should guide us here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hm. Fair. Do we handle unions withundefined in accordance with that, then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We do, but with a few wrinkles. If the return type annotation includesvoid,any, orunknown we don't requireanyreturn statements. Otherwise, if the return type includesundefined we require at least onereturn statement somewhere, but don't requirereturn statements to have expressions and allow the end point of the function to be reachable. Otherwise, we require areturn statement with an expression at every exit point.
| >T : T | ||
| boom: T extends any ? true : true | ||
| >boom :T extends any ? true :true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
why did this type collapse totrue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I changed this to eagerly resolve totrue when the extends type isany orunknown (since it always will be).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ah, but what aboutT extends any ? { x: T } : never - that needs to distribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hmm, yes, I suppose we can only optimize when the conditional type isn't distributive.
weswighamMay 30, 2018 • 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There's a better check (than just distributivity) that I have an outstanding PR for: instantiate the check type (and infer types) in the true/false types with wildcards. If the instantiation isn't the input type, you can't simplify. If it is, you can. (Since the type isn't affected by instantiation)
The BaseController state now uses `unknown` rather than `any` as thetype for state properties. `unknown` is more type-safe than `any` incases like this where we don't know what type to expect. See here fordetails [1].This was suggested by@rekmarks during review of#362 [2].[1]:microsoft/TypeScript#24439[2]:#362 (comment)
* Use `unknown` rather than `any` for BaseController stateThe BaseController state now uses `unknown` rather than `any` as thetype for state properties. `unknown` is more type-safe than `any` incases like this where we don't know what type to expect. See here fordetails [1].This was suggested by@rekmarks during review of#362 [2].[1]:microsoft/TypeScript#24439[2]:#362 (comment)* Use type alias for controller state rather than interfaceThe mock controller state in the base controller tests now uses a typealias for the controller state rather than an interface. This wasrequired to get around an incompatibility between`Record<string, unknown>` and interfaces[1].The `@typescript-eslint/consistent-type-definitions` ESLint rule hasbeen disabled, as this problem will be encountered fairly frequently.[1]:microsoft/TypeScript#15300 (comment)
* Use `unknown` rather than `any` for BaseController stateThe BaseController state now uses `unknown` rather than `any` as thetype for state properties. `unknown` is more type-safe than `any` incases like this where we don't know what type to expect. See here fordetails [1].This was suggested by@rekmarks during review of#362 [2].[1]:microsoft/TypeScript#24439[2]:#362 (comment)* Use type alias for controller state rather than interfaceThe mock controller state in the base controller tests now uses a typealias for the controller state rather than an interface. This wasrequired to get around an incompatibility between`Record<string, unknown>` and interfaces[1].The `@typescript-eslint/consistent-type-definitions` ESLint rule hasbeen disabled, as this problem will be encountered fairly frequently.[1]:microsoft/TypeScript#15300 (comment)
* Use `unknown` rather than `any` for BaseController stateThe BaseController state now uses `unknown` rather than `any` as thetype for state properties. `unknown` is more type-safe than `any` incases like this where we don't know what type to expect. See here fordetails [1].This was suggested by@rekmarks during review of#362 [2].[1]:microsoft/TypeScript#24439[2]:#362 (comment)* Use type alias for controller state rather than interfaceThe mock controller state in the base controller tests now uses a typealias for the controller state rather than an interface. This wasrequired to get around an incompatibility between`Record<string, unknown>` and interfaces[1].The `@typescript-eslint/consistent-type-definitions` ESLint rule hasbeen disabled, as this problem will be encountered fairly frequently.[1]:microsoft/TypeScript#15300 (comment)
Uh oh!
There was an error while loading.Please reload this page.
This PR adds a new top type
unknownwhich is the type-safe counterpart ofany. Anything is assignable tounknown, butunknownisn't assignable to anything but itself andanywithout a type assertion or a control flow based narrowing. Likewise, no operations are permitted on anunknownwithout first asserting or narrowing to a more specific type.Note that this PR is technically a breaking change since
unknownbecomes a reserved type name.Fixes#10715.