Wrapping can.compute in can.Map breaks live-binding #530

Closed
simpleTechs opened this Issue Nov 7, 2013 · 4 comments

Comments

Projects
None yet
2 participants
@ghost

ghost commented Nov 7, 2013

When a can.compute is wrapped in a can.Map and then live-bound, the view will not update when the compute changes.
I have an app that has a global can.Map for storing the state, so when I now put in a can.compute anywhere in this Map (manually or by accessing can.route.data for example, which is part of this state), the compute will never update the view.

See here for a very basic example: http://jsfiddle.net/simpleFabian/FLj7b

Is this behaviour intended in some way?
Can I work around it temporarily in any way? (as in using a helper, maybe?)

Thanks in advance!

@ghost ghost assigned imjoshdean Nov 14, 2013

@imjoshdean

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Nov 14, 2013

Contributor

Man...this was a tricky one to track down. It turns out that when you wrap a compute in a can.Map, upon setting a value for that compute, it unintentionally changes that attribute's value to the result of the compute.

Contributor

imjoshdean commented Nov 14, 2013

Man...this was a tricky one to track down. It turns out that when you wrap a compute in a can.Map, upon setting a value for that compute, it unintentionally changes that attribute's value to the result of the compute.

@imjoshdean

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Nov 14, 2013

Contributor

So after talking with @justinbmeyer we came to the conclusion that this is intended...ish. .attr(prop, val) is intended to set the value of a property, regardless of whether or not the value is a compute.

What you want to do, using your fiddle as an example, is:

this.options.testMap.attr('test')('after2');

This will get the compute on testMap, then you can call it with the value you want to change it to. This is where my previously stated "ish" comes into play. Currently if you do .attr(prop) on a compute, it will return the compute's value and not the compute itself. I'm cleaning up the fix to this and writing a test for it.

This should still be in 2.0.2 which should be landing later today.

Contributor

imjoshdean commented Nov 14, 2013

So after talking with @justinbmeyer we came to the conclusion that this is intended...ish. .attr(prop, val) is intended to set the value of a property, regardless of whether or not the value is a compute.

What you want to do, using your fiddle as an example, is:

this.options.testMap.attr('test')('after2');

This will get the compute on testMap, then you can call it with the value you want to change it to. This is where my previously stated "ish" comes into play. Currently if you do .attr(prop) on a compute, it will return the compute's value and not the compute itself. I'm cleaning up the fix to this and writing a test for it.

This should still be in 2.0.2 which should be landing later today.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 14, 2013

Hey Josh,

thanks for looking into this!
I just want to make sure we're talking about the same issue:
What I talk about is not line 16 in my fiddle, but rather line 11.
See, when calling c1('after') the mustache updates accordingly - but calling c2('after') does not.
The thing is, my compute is updated outside of the map, but bound to from inside. (Does this even make sense to you? :-))

I'm not quite sure if that's the same thing you are into here, but that may totally be on me - is it?

Best regards,
Fabian

ghost commented Nov 14, 2013

Hey Josh,

thanks for looking into this!
I just want to make sure we're talking about the same issue:
What I talk about is not line 16 in my fiddle, but rather line 11.
See, when calling c1('after') the mustache updates accordingly - but calling c2('after') does not.
The thing is, my compute is updated outside of the map, but bound to from inside. (Does this even make sense to you? :-))

I'm not quite sure if that's the same thing you are into here, but that may totally be on me - is it?

Best regards,
Fabian

@imjoshdean

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Nov 14, 2013

Contributor

Hey Fabian,

I'm pretty sure we're on the same page. After this code I committed get's merged, your example will work in 2.0.2 just by changing

this.options.testMap.attr('test', 'after2');

to

this.options.testMap.attr('test')('after2');

The issue with c2('after') not updating accordingly was fixed in 2.0.1 (Give http://jsfiddle.net/FLj7b/1/ a looksie, which uses 2.0.1).

Contributor

imjoshdean commented Nov 14, 2013

Hey Fabian,

I'm pretty sure we're on the same page. After this code I committed get's merged, your example will work in 2.0.2 just by changing

this.options.testMap.attr('test', 'after2');

to

this.options.testMap.attr('test')('after2');

The issue with c2('after') not updating accordingly was fixed in 2.0.1 (Give http://jsfiddle.net/FLj7b/1/ a looksie, which uses 2.0.1).

imjoshdean added a commit that referenced this issue Nov 14, 2013

Merge pull request #546 from bitovi/530-mapComputes
Fixes #530 doing .attr get on a compute returns compute and not compute's value
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment