Skip to content

Update lib.es2016.array.include.d.ts #45767

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
wants to merge 3 commits into from
Closed

Conversation

Asaf-S
Copy link

@Asaf-S Asaf-S commented Sep 7, 2021

Array's include function checks whether or not the searchElement exists in it, and returns the appropriate boolean value as an answer.
However, in the current implementation, TypeScript is requesting the type of searchElement to be a member of the array, instead of receiving any value as searchElement.
This in turn forces the user to know in advance whether or not the searchElement is a member of the array, which defeats the whole purpose of using the 'include' function. The way users currently workaround that problem is by overriding the TypeScript mechanism by using "as", as suggested by @sandersn in here: #33200 (comment). See also this StackOverFlow question.

I believe that the purpose of the include function is to allow any variable, even if its type is unknown, to be searched in the array and get a boolean answer, and therefore I suggest the following changes.

Playground link.

Fixes #33200.

Array's `include` function checks whether or not the `searchElement` exists in it, and returns the appropriate boolean value as an answer.
However, in the current implementation, TypeScript is requesting the type of `searchElement` to be a member of the array, instead of receiving any value as `searchElement`.
This in turn forces the user to know in advance whether or not the `searchElement` is a member of the array, which defeats the whole purpose of using the 'include' function. The way users currently workaround that problem is by overriding the TypeScript mechanism by using "as", as suggested by @sandersn in here: microsoft#33200 (comment) . See also https://stackoverflow.com/questions/43804805/check-if-value-exists-in-enum-in-typescript .

I believe that the purpose of the `include` function is to allow any variable, even if its type is unknown, to be searched in the array and get a boolean answer, and therefore I suggest the following changes.

Playground:
https://www.typescriptlang.org/play?#code/LAKApgdgrgtgBAMQEoFUCSAVAynA3qOOAQQAUSAZAUTgF44ByAQ3oBoC4AhIgOR6NoYAjVqAC+oUADMoEAMYAXAJYB7CHEUBnAGqMANooAmWKAAcTygE7ywBhBaiL5ACigawFtBBNR5ALjgyANYQygDuEACU-oLKyrpgjGr4IIQA9ABUEilwGAAWYHCScbphihAA5nD6EAWacPL5cBpQ5eVgGtYGcKGMAJ71yt2WgYwWyjJdjixwiZPyQxaBGtOCPt25-QaDaHC5jABuBfKDyocWFoYFGL0mYFiyFybyAPzsAOobcFtwO3uHAzMNG4rPU9vMGgVXO5PN55nVkOhsNNQrlFPEfvR4CF5s0LLV5ni9Lp+nUnLNQYx5PQNKCCii4gVvBZzG44MpJAENGVKgADMqyXRQAxgHkRV4gdiEPHyKAWNQAeUEACswAoAHT7PRQdpOBGYLARNX8wXCjQuYEwtaMGl67ARADckrg7HSqXY0tlCuVqvkGq1OttBqNchNOqhHi8PgdcFSqRyuTGoRpiTg7jGFn8RAs5VgkHm7PqNwK9CCIXC9HUNOxgK55QgjEE6OOcBMo0YMDA1gsbI58iLDED9DVOX7JYgwTCEArdWr1tr9cbR0GfduA9Q+rVXF4vCHTgATABmAAsAFYImIsrJVBoGWqSuUnJodPojKZzFYbHYHM5A2rSBRKAiaNYzgSgAA9bgUGx-HkewwFAK8IBveI72UB8nz0QxjDMSxOi-RwnHoYQgPtGM43AyDOhguCEOvW970fbRMNfHCP1sewCPoWR6BIsjQIgn1oMKPQ3FopD6LQxjnywt9cM-DjnGgXRdF4kCKMEgx-EkET4JARDkLAVD0KYl9sPfPCFKcXBRFU8iBKgzThN0NwgA
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Sep 7, 2021
@ghost
Copy link

ghost commented Sep 7, 2021

CLA assistant check
All CLA requirements met.

@sandersn
Copy link
Member

sandersn commented Sep 7, 2021

@Asaf-S can you please open a new issue explaining the problem, your solution, and your justification for it? Then link #36554, which tracks all the array definition changes. We want to make all the changes to Array at once.

In the meantime I'd prefer to close this PR since the final PR is likely to be quite different, much larger and some time from now.

@Asaf-S
Copy link
Author

Asaf-S commented Sep 8, 2021

I've added a comment in the aggregating issue, however, I won't open a new issue, as my PR correlates to this existing issue #33200

@typescript-bot
Copy link
Collaborator

It looks like you've sent a pull request to update our 'lib' files. These files aren't meant to be edited by hand, as they consist of last-known good states of the compiler and are generated from 'src'. Unless this is necessary, consider closing the pull request and sending a separate PR to update 'src'.

@typescript-bot typescript-bot added the lib update PR modifies files in the `lib` folder label Sep 8, 2021
@sandersn
Copy link
Member

I don't think I follow; #33200 is about Object.values of enums, but this PR changes Array.includes.
The problem in #33200 also seems to be about string literals not being assignable to string enum members with the same value. But your object sounds more general. Is it? Discussing that and proposing a solution would be good to do in a separate issue from #33200, especially since that one's already closed.

@sandersn
Copy link
Member

This PR is blocked on the #36554 meta-issue, so I'm going to close it since it's getting stale.

@sandersn sandersn closed this Sep 29, 2021
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 lib update PR modifies files in the `lib` folder
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Object.values(Enum) should return an array of Enum's value's type
3 participants