Skip to content

Number methods should support type guards #11670

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

Closed
ghost opened this issue Oct 16, 2016 · 8 comments
Closed

Number methods should support type guards #11670

ghost opened this issue Oct 16, 2016 · 8 comments
Labels
Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript

Comments

@ghost
Copy link

ghost commented Oct 16, 2016

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isInteger

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isSafeInteger

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isFinite

etc

When these methods return true, the value should be asserted to be non-null Number type.

@aluanhaddad
Copy link
Contributor

This sounds like a good idea. One minor correction, these are not methods of Number.prototype but rather of the Number function.

@ghost ghost changed the title Number prototype methods should support type guards Number methods should support type guards Oct 17, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Oct 17, 2016

PRs welcomed. You can find more information about contributing lib.d.ts fixes at https://github.com/Microsoft/TypeScript/blob/master/CONTRIBUTING.md#contributing-libdts-fixes.

@mhegazy mhegazy added Suggestion An idea for TypeScript Help Wanted You can do this Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript labels Oct 17, 2016
@mhegazy mhegazy added this to the Community milestone Oct 17, 2016
@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented Oct 17, 2016

i really like it! (suggestion! accepting prs!)

so what exactly is an integer number as far as the TypeScript types go?

function isInteger(value: number): value is number & .....? { // <--- replace .....? with something meaningful
}

@mhegazy
Copy link
Contributor

mhegazy commented Oct 17, 2016

you would only use number. the argument should allow non-numeric types as well.

declare function isInteger(value: any): value is number ;

@zpdDG4gta8XKpMCd
Copy link

oh, nevermind then

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Oct 19, 2016

@Aleksey-Bykov still a good question. Maybe in the future TypeScript's type system will be so advanced that it will be able to infer discrete vs continuous types O.O

@mhegazy mhegazy modified the milestones: TypeScript 2.1, Community Oct 19, 2016
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Oct 19, 2016
@RyanCavanaugh
Copy link
Member

This turns out to be a bad idea because type guards need to return true for all of the values in the domain of their guarded type, not just some of them. In other words, with the proposed definition:

let x: number | string = ...;
if (typeof x === 'string') {

} else if (isInteger(x)) {
  // x: number
} else {
  // We assume 'x: never' in this block!
  // error
  console.log(x.toFixed(2));
}

RyanCavanaugh added a commit to RyanCavanaugh/TypeScript that referenced this issue Nov 17, 2016
RyanCavanaugh added a commit to RyanCavanaugh/TypeScript that referenced this issue Nov 17, 2016
@aluanhaddad
Copy link
Contributor

I see. That is definitely a problem. This really did seem like a good idea but I guess I didn't really think it through.

RyanCavanaugh added a commit that referenced this issue Nov 17, 2016
Revert "fix #11670, support type guards in NumberConstructor (#11722)"
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants