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 release] Fix #12475 #12908

Merged
merged 3 commits into from
Feb 4, 2016
Merged

[BUGFIX release] Fix #12475 #12908

merged 3 commits into from
Feb 4, 2016

Conversation

mmun
Copy link
Member

@mmun mmun commented Feb 3, 2016

Fixes #12475 by reverting the ArrayProxy code to use before-observers. In order to re-remove these before observers we will need to remove the @array:before event (AKA the willChange part of array.addArrayObserver({ willChange, didChange }))

For more details see #12860 (comment).

@stefanpenner
Copy link
Member

good hunting, I'm sorta sad about the fix. But... as discussed it appears a better fix may be a breaking change.

also would like @krisselden 👀 on this

@mmun
Copy link
Member Author

mmun commented Feb 3, 2016

Me too 😭

@rwjblue
Copy link
Member

rwjblue commented Feb 3, 2016

OK, 🍏 build now...

@krisselden
Copy link
Contributor

I'm definitely sad about the fix, I think we can do something else but we have to fix the regression in the mean time.

@stefanpenner
Copy link
Member

@krisselden good to merge for now?

@mmun
Copy link
Member Author

mmun commented Feb 4, 2016

@stefanpenner: @krisselden approved on Slack.

I believe your solution is best for now @mmun

rwjblue added a commit that referenced this pull request Feb 4, 2016
@rwjblue rwjblue merged commit d5b6c8a into emberjs:master Feb 4, 2016
@rwjblue rwjblue deleted the fix-12475 branch February 4, 2016 22:25
@runspired
Copy link
Contributor

many tears are shed.

@mmun
Copy link
Member Author

mmun commented Feb 4, 2016

On the plus side, this bug got me more familiar with ember-metal. I'm going to spike out lazy array observers and see how bad it breaks my apps :)

@lozjackson
Copy link

Thankyou very much @mmun for fixing this.. I've been stuck on 2.0.3 because of this bug. I'm now on 2.4.1 and much happier. :)

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.

None yet

6 participants