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: defend against for/in use on arrays #5661

Merged
merged 2 commits into from
Feb 25, 2022

Conversation

EisenbergEffect
Copy link
Contributor

Pull Request

📖 Description

Some folks incorrectly use for/in on Array instances. This is a bad practice but people still do it and there is lots of legacy code in existence that makes this mistake which may never be updated.

This PR makes our Array observation bookkeeping properties non-numerable so that for/in does not return them in the event that a developer makes the above mistake or is using a 3rd party library which makes this mistake.

🎫 Issues

👩‍💻 Reviewer Notes

This is a very small change that shifts the code from simple field assignments to defineProperty calls, so we can provide descriptor data and make the properties non-enumerable.

📑 Test Plan

All existing tests continue to pass.

✅ Checklist

General

  • I have included a change request file using $ yarn change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

⏭ Next Steps

A second matching PR will be incoming soon for FASTElement 2.0.

@EisenbergEffect EisenbergEffect merged commit 76a9c86 into master Feb 25, 2022
@EisenbergEffect EisenbergEffect deleted the users/eisenbergeffect/defend-against-forin branch February 25, 2022 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: fast-element's Array.prototype is being patched twice resulting on duplicate splices
3 participants