Skip to content

Fix Number.is* to accept unknown again #34932

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

Merged
merged 1 commit into from
Mar 27, 2020
Merged

Fix Number.is* to accept unknown again #34932

merged 1 commit into from
Mar 27, 2020

Conversation

shicks
Copy link
Contributor

@shicks shicks commented Nov 5, 2019

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.

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.
@shicks
Copy link
Contributor Author

shicks commented Nov 5, 2019

I looked for a test that could be updated but I have no idea what would be appropriate - I found no unit tests that pertained to any of these declarations. If there's something I can update to ensure it sticks this time, please advise.

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Nov 5, 2019

Time to add new tests!

Which brings me to my wishlist. If only there was code coverage for types...

Then we'd know how many % of those lib.d.ts files actually have tests

@sandersn sandersn added the For Backlog Bug PRs that fix a backlog bug label Feb 3, 2020
@sandersn sandersn requested review from sandersn and orta March 10, 2020 21:47
@SancheZz
Copy link

Guys. I'm sorry. But what are you waiting for? Why don't you merge PR? The correct behavior has been broken and needs to be restored.

@orta
Copy link
Contributor

orta commented Mar 27, 2020

Cool, checked over the history and these fixes look good IMO

@orta orta merged commit 6d7539a into microsoft:master Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Number.is* should accept unknown
5 participants