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

fix(lib/es2015): Fix types of Reflect methods #41987

Merged

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Dec 16, 2020

Unlike #35608, this only fixes the types so that the return types of Reflect.ownKeys(…) and Reflect.getPrototypeOf(…) are correct.

Blocks:


Fixes #34626

/cc @Jessidhia @orta @rbuckton @sandersn

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Dec 16, 2020
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@orta
Copy link
Contributor

orta commented Jan 4, 2021

I compared each function to the MDN docs for the reflect API, and that all seems to look right to me - thanks @ExE-Boss

@orta orta merged commit 562237d into microsoft:master Jan 4, 2021
@ExE-Boss ExE-Boss deleted the lib/es2015/fix-reflect-method-types branch January 4, 2021 10:30
@monfera
Copy link

monfera commented Jan 4, 2021

@ExE-Boss great change, thanks! Looks like lib/lib.es2015.reflect.d.ts needs to be updated for userland code who install via eg. yarn and "typescript": "microsoft/TypeScript#master", in the devDependencies; also tried yarn add -D typescript@next, still getting the old definitions from lib/lib.es2015.reflect.d.ts. Maybe there's some extra build step?

@Luxcium
Copy link

Luxcium commented Jan 6, 2021

I would like to know what is the return type of Reflect.ownKeys(target) is it of Type '(string | number | symbol)[]' or of Type '(string | symbol)[]' ??? (Sorry to be so confused I am the kind of person who likes to annotate all my functions output types and now I am using /* : (string | number | symbol)[] */ yes with the /* and the */ haha !!!

Screenshot_20210105_230211

@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented Jan 6, 2021

@Luxcium The correct return type of Reflect.ownKeys(target) is (string | symbol)[]:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Reflect.ownKeys has overly wide type definition
5 participants