Skip to content
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

Number.is* should accept unknown #34931

Closed
shicks opened this issue Nov 5, 2019 · 4 comments · Fixed by #34932
Closed

Number.is* should accept unknown #34931

shicks opened this issue Nov 5, 2019 · 4 comments · Fixed by #34932
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript
Milestone

Comments

@shicks
Copy link
Contributor

shicks commented Nov 5, 2019

TypeScript Version: 3.8.0-dev.20191102

Search Terms: Number isInteger isSafeInteger isFinite isNaN

Code

declare function use(number: number): void;
declare const maybeNumber: string | number;
if (Number.isInteger(maybeNumber)) {
  use(maybeNumber as number);
}

Expected behavior:
Type checks

Actual behavior:
Fails to type check because Number.isInteger is typed to take only number.

Playground Link:
http://www.typescriptlang.org/play/index.html?ts=3.8.0-dev.20191102&ssl=3&ssc=2&pln=1&pc=1#code/CYUwxgNghgTiAEAzArgOzAFwJYHtX2QGcQAKVZAWwCMQYAuec62gSgYDcctgBuAKFCRYCMHkIZ4FKAE8aAOUo168cTCyoA5vAA+jRbX5ZE8EguYwAdFkIBJVBhAbaJKbJBmlLFvADefeATELjLy+jDwUIR65iz8AL5AA

Related Issues:

@shicks
Copy link
Contributor Author

shicks commented Nov 5, 2019

Reading further, it looks like #24436 updated the generated lib, not the version in src.

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript labels Nov 5, 2019
@RyanCavanaugh
Copy link
Member

Thank you for the good research effort on this!

@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Nov 5, 2019
@DanielRosenwasser
Copy link
Member

So do we want this change? I'm kind of glad it didn't happen (even for bad reasons) because I think the current behavior saves users more than it frustrates, and the workaround is relatively simple (unary +).

@RyanCavanaugh
Copy link
Member

@DanielRosenwasser I think it's good. Number.isInteger("0") is false, IOW no fishy coercion occurs

orta pushed a commit that referenced this issue Mar 27, 2020
Fixes #34931

This essentially reapplies #24436 which incorrectly updated the generated files rather than the sources, and so was wiped out by 2f73986.

Note that this is specifically _not_ the same as #4002, which pertains to the global functions that coerce their arguments and therefore should _not_ accept `unknown`.  This change was already accepted 18 months ago, but was applied incorrectly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants