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 definition of Array.prototype[Symbol.unscopables] #42566

Merged
merged 1 commit into from Feb 22, 2023

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Jan 31, 2021

#36540 was incorrectly applied to lib/es2015.symbol.wellknown.d.ts and thus got overwritten once the next TypeScript LKG lib was rebuilt.

#42566 (comment):

The second change is simply copying the Symbol.unscopables definition to ReadonlyArray, and making it so that it doesn’t need to be updated in the future, since when new methods get added to %Array.prototype%, they almost always also have to be added to %Array.prototype[Symbol.unscopables]% for compatibility with legacy code using the sloppy mode‑only with statement.


Actually fixes #34610
Fixes #46616


review?(@sandersn, @rbuckton)

@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.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jan 31, 2021
@ExE-Boss ExE-Boss force-pushed the lib/es2015/fix-array-unscopables branch 2 times, most recently from 0bca8f1 to 5ba4718 Compare February 7, 2021 15:32
@sandersn sandersn added this to Not started in PR Backlog Feb 8, 2021
PR Backlog automation moved this from Not started to Needs merge Feb 19, 2021
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait a minute. This changes more than #36540 does. What do the additional changes do?

PR Backlog automation moved this from Needs merge to Waiting on author Feb 19, 2021
@sandersn sandersn moved this from Waiting on author to Needs review in PR Backlog Feb 19, 2021
@ExE-Boss
Copy link
Contributor Author

@sandersn The second change is simply copying the Symbol.unscopables definition to ReadonlyArray, and making it so that it doesn’t need to be updated in the future, since when new methods get added to %Array.prototype%, they almost always also have to be added to %Array.prototype[Symbol.unscopables]% for compatibility with legacy code using the sloppy mode‑only with statement.

@ExE-Boss ExE-Boss force-pushed the lib/es2015/fix-array-unscopables branch from 5ba4718 to 6c9c073 Compare February 23, 2021 11:34
@sandersn
Copy link
Member

I'd be happy to take the first two changes immediately: removing the extraneous (), and copying unscopeables' definition to ReadonlyArray. But I don't know enough about array method changes to evaluate the third. I'd recommend splitting the third change into a separate PR so @rbuckton can review it.

@MartinJohns
Copy link
Contributor

Related: #46616

@graphemecluster
Copy link
Contributor

Why it is still not merged? If not I am going to put this change in #49855.
@sandersn [Symbol.unscopables] is mutable – try adding and deleting properties in the devtools console.
And I don't think such a small change is worth 2 PRs.

@ExE-Boss
Copy link
Contributor Author

@graphemecluster Probably because there are merge conflicts (again).

@graphemecluster
Copy link
Contributor

Conflicts again :(

@ExE-Boss ExE-Boss force-pushed the lib/es2015/fix-array-unscopables branch from f4c68f0 to cf62705 Compare August 26, 2022 19:05
@rbuckton rbuckton force-pushed the lib/es2015/fix-array-unscopables branch from c14d522 to 2b75010 Compare February 22, 2023 17:24
@rbuckton rbuckton merged commit 6bbdcaa into microsoft:main Feb 22, 2023
PR Backlog automation moved this from Waiting on reviewers to Done Feb 22, 2023
@ExE-Boss ExE-Boss deleted the lib/es2015/fix-array-unscopables branch February 22, 2023 18:36
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
PR Backlog
  
Done
Development

Successfully merging this pull request may close these issues.

Type of Array.prototype[Symbol.unscopables] is wrong Array.prototype[Symbol.unscopables] is not a function
6 participants