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

Compute memory leak with nested properties #1676

Closed
dylanrtt opened this Issue May 7, 2015 · 4 comments

Comments

Projects
None yet
3 participants
@dylanrtt
Contributor

dylanrtt commented May 7, 2015

Apparently if you manually add a compute to a Map, and the compute depends on a deep property of that same Map, it can leak bindings if used in conjunction with other event bindings in a certain order.

http://jsbin.com/nadelesulu/1/edit?html,js,console

@daffl daffl added this to the 2.2.6 milestone May 11, 2015

@daffl daffl added the bug label May 11, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer May 13, 2015

Contributor

This is likely due to can.Map thinking that compute represents a define.get.

Contributor

justinbmeyer commented May 13, 2015

This is likely due to can.Map thinking that compute represents a define.get.

@justinbmeyer justinbmeyer self-assigned this May 14, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer May 14, 2015

Contributor

Looked into this. First, I don't think you should be writing your code like this. There's no reason to bind to "change" events on can.Map anymore and you could use the define plugin to describe petInfo.

I'm not sure why you would set a compute as a property value of an attribute either.

All that being said, it would still be nice to make this work. The problem is that this code creates a "cycle" of binding that isn't being detected while unbinding.

Contributor

justinbmeyer commented May 14, 2015

Looked into this. First, I don't think you should be writing your code like this. There's no reason to bind to "change" events on can.Map anymore and you could use the define plugin to describe petInfo.

I'm not sure why you would set a compute as a property value of an attribute either.

All that being said, it would still be nice to make this work. The problem is that this code creates a "cycle" of binding that isn't being detected while unbinding.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer May 14, 2015

Contributor

#1231 is probably related

Contributor

justinbmeyer commented May 14, 2015

#1231 is probably related

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer May 14, 2015

Contributor

Closed by #1696.

Contributor

justinbmeyer commented May 14, 2015

Closed by #1696.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment