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] Move reduceComputed instanceMetas into object's meta #4559

Merged
merged 1 commit into from
Mar 25, 2014

Conversation

mmun
Copy link
Member

@mmun mmun commented Mar 19, 2014

Closes #4552. Also opens the door to new types of computed properties that require storing extra data related to caching, eg. throttled and debounced computed properties.

@wycats
Copy link
Member

wycats commented Mar 21, 2014

@hjdivad does this break anything?

@hjdivad
Copy link
Member

hjdivad commented Mar 25, 2014

This seems totally reasonable. 🤘 :shipit:.

@rjackson I'm surprised to see this unmerged. Isn't this one of the things we looked at yesterday?

@hjdivad
Copy link
Member

hjdivad commented Mar 25, 2014

@rjackson ah I recall now; we just wanted to see a simple example to demonstrate it really does deal with the memory issue.

Still, this seems like a definite improvement to me.

@mmun mmun changed the title [BUGFIX reduce-computed] Move instanceMetas into object's meta [BUGFIX beta] Move reduceComputed instanceMetas into object's meta Mar 25, 2014
@rwjblue
Copy link
Member

rwjblue commented Mar 25, 2014

@hjdivad - Sooo.... To shipit or hold off for now?

@mmun - Do you think you can create a demo JSBin to prove this is fixed?

@hjdivad
Copy link
Member

hjdivad commented Mar 25, 2014

@rjackson I'm 👍 on shipping it. Demo is mostly good to verify the instanceMetas aren't also leaking elsewhere.

rwjblue added a commit that referenced this pull request Mar 25, 2014
[BUGFIX beta] Move reduceComputed instanceMetas into object's meta
@rwjblue rwjblue merged commit 6c27dee into emberjs:master Mar 25, 2014
@mmun mmun deleted the cache-meta branch March 25, 2014 21:46
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.

reduceComputed leaks instanceMetas
4 participants