Binding to an Observe.compute value is broken #372

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@ccummings
Contributor

ccummings commented Apr 30, 2013

var person = new can.Observe({
    name: 'Jeeves'
});
var name = person.compute('name');
name.bind('change', function(ev, newVal, oldVal) {
    ...
});
name('Alfred');

The change handler in this example will be called twice. Once with the newVal as Alfred and a second time with newVal as the Observe instance (person).

Here's how this happens:

When the name compute is updated, we end up in compute.js and run the set function which is defined here in observe.js. This calls attr which fires a change event with newVal === 'Alfred'.

setVal is now the Observe instance since attr returns the Observe, so when the execution reaches here, a change event is triggered with newVal set to the Observe instance.

This code was added by @justinbmeyer here to extend can.compute.

@ccummings

This comment has been minimized.

Show comment
Hide comment
@ccummings

ccummings Apr 30, 2013

Contributor

Seems this issue is a bit more serious than I first thought. It's not just a duplicate event that is emitted, the compute you get from Observe.compute will have its value changed to the Observe If you bind to the change event. So in the example above adding this:

setTimeout(function() {
    console.log(name())
}, 2000);

Would log the Observe because the compute has been set to the Observe instance.

Contributor

ccummings commented Apr 30, 2013

Seems this issue is a bit more serious than I first thought. It's not just a duplicate event that is emitted, the compute you get from Observe.compute will have its value changed to the Observe If you bind to the change event. So in the example above adding this:

setTimeout(function() {
    console.log(name())
}, 2000);

Would log the Observe because the compute has been set to the Observe instance.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Apr 30, 2013

Contributor

Can you add a test and I will fix it tonight.

Sent from my iPhone

On Apr 30, 2013, at 11:24 AM, Curtis Cummings notifications@github.com wrote:

Seems this issue is a bit more serious than I first thought. It's not just a duplicate event that is emitted, the compute you get from Observe.compute will have its value changed to the Observe If you bind to the change event. So in the example above adding this:

setTimeout(function() {
console.log(name())
}, 2000);
Would log the Observe because the compute has been set to the Observe instance.


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Apr 30, 2013

Can you add a test and I will fix it tonight.

Sent from my iPhone

On Apr 30, 2013, at 11:24 AM, Curtis Cummings notifications@github.com wrote:

Seems this issue is a bit more serious than I first thought. It's not just a duplicate event that is emitted, the compute you get from Observe.compute will have its value changed to the Observe If you bind to the change event. So in the example above adding this:

setTimeout(function() {
console.log(name())
}, 2000);
Would log the Observe because the compute has been set to the Observe instance.


Reply to this email directly or view it on GitHub.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer May 1, 2013

Contributor

The test seems to pass for me.

Contributor

justinbmeyer commented May 1, 2013

The test seems to pass for me.

matthewp added a commit that referenced this pull request Oct 28, 2015

Allow relative can-imports
This Fixes our can/view/stache/system plugin to normalize the dynamic
module names before adding them to System.bundle. This is needed because
steal-tools normalizes the bundle names and if the names are relative it
will throw an exception. Fixes #372

matthewp added a commit that referenced this pull request Oct 28, 2015

Allow relative can-imports
This Fixes our can/view/stache/system plugin to normalize the dynamic
module names before adding them to System.bundle. This is needed because
steal-tools normalizes the bundle names and if the names are relative it
will throw an exception. Fixes #372
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment