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

Unless checking accesses, do not include unique symbols or numbers in index types #23090

Closed

Conversation

weswigham
Copy link
Member

Fixes the issue of keyof now returning unique symbols that @kpdonn reported in #20721, and @ahejlsberg were you looking at another issue this fixed?

Regarding the implementation - I actually had to add a new relationship (ew) to handle comparing with constraints of generic index types. (To continue to allow, eg, T[typeof sym]).

@weswigham weswigham requested review from rbuckton and ahejlsberg April 2, 2018 21:30
@weswigham weswigham changed the title Unless checking access, do not include unqie symbols or numbers in index types Unless checking accesses, do not include unique symbols or numbers in index types Apr 2, 2018
@kpdonn
Copy link
Contributor

kpdonn commented Apr 2, 2018

FWIW I actually think it makes a lot of sense for keyof to include unique symbol. See #20721 for my reasoning, but I was only pointing it out because it seemed pretty clearly unintentional and not because I thought it was actually a problem.

If it made it through a release with nobody complaining maybe it's a sign it was a good change? 🤷‍♂️

@weswigham
Copy link
Member Author

@kpdonn keyof T can produce symbols, but keyof T is assignable to string - that's a bit of a pickle.

@weswigham
Copy link
Member Author

Closed in favor of #23145.

@weswigham weswigham closed this Apr 4, 2018
@weswigham weswigham deleted the filtermap-keyof-instantiate branch April 11, 2018 23:08
@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants