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

[BUGFIX beta] Do not rely prototype extensions (objectAt) #12942

Merged
merged 2 commits into from
Feb 10, 2016
Merged

[BUGFIX beta] Do not rely prototype extensions (objectAt) #12942

merged 2 commits into from
Feb 10, 2016

Conversation

btecu
Copy link
Contributor

@btecu btecu commented Feb 10, 2016

Do not use prototype extension for objectAt.

Related to #9269 and based on the work of @stefanpenner in #10899.

If this is acceptable, will continue addressing one extension at a time for quicker merging.

Do not use prototype extension for `objectAt`.

Related to #9269 and based on the work of @stefanpenner in #10899.

If this is acceptable, will continue addressing one extension at a time for quicker merging.
@mmun
Copy link
Member

mmun commented Feb 10, 2016

@btecu Awesome! Been hoping someone would start moving these over.

@stefanpenner
Copy link
Member

If this is acceptable, will continue addressing one extension at a time for quicker merging.

👍

I believe I ran into some strange cycle issues that made it tricky to remove them all, but since then I have landed some fixes to improve it. It might be possible to do today without grief.

Keep them coming.

@stefanpenner
Copy link
Member

oh, lets make sure objectAt has a unit test.

We should also consider a future where we expose these as public API, similar to get/set, but that can come via RFC/later pr

@btecu
Copy link
Contributor Author

btecu commented Feb 10, 2016

Thanks both! I've added a unit test.

stefanpenner added a commit that referenced this pull request Feb 10, 2016
[BUGFIX beta] Do not rely prototype extensions (objectAt)
@stefanpenner stefanpenner merged commit 58f5a9a into emberjs:master Feb 10, 2016
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.

3 participants