Attributes/Converters Issue #174

Closed
thecountofzero opened this Issue Nov 26, 2012 · 5 comments

Comments

Projects
None yet
4 participants
@thecountofzero
Contributor

thecountofzero commented Nov 26, 2012

Using the attributes plugin, I have an attribute of a model that is another type of model. And then an attribute of that second model uses a converter to determine its value.

In my app, it polls the server for fresh data and uses live binding to update the data in view. I noticed that a value that is determined using a converter is wrong after the first go round. It actually seems to be calling the converter twice. The first time with the correct value from the server, and the second time with the converted value. The second (incorrect) value is what is being used.

Please see the following fiddle. To simulate polling, just double click the output frame.

http://jsfiddle.net/thecountofzero/qYdwR/562/

Looking at commits, it seems that the issue was introduced with this commit:

f9896d9

@ghost ghost assigned imjoshdean Nov 26, 2012

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Dec 18, 2012

Contributor

@imjoshdean That commit seems to be related to several bugs reported (namely #207 and #208). It would be great if we can figure out how this is supposed to work or fix it for 1.1.4.

Contributor

daffl commented Dec 18, 2012

@imjoshdean That commit seems to be related to several bugs reported (namely #207 and #208). It would be great if we can figure out how this is supposed to work or fix it for 1.1.4.

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Jan 11, 2013

Contributor

Man this is a tough nut. I narrowed it down to __convert being called multiple times when the model is live bound. Still needs some more digigng though.

Contributor

daffl commented Jan 11, 2013

Man this is a tough nut. I narrowed it down to __convert being called multiple times when the model is live bound. Still needs some more digigng though.

@moschel

This comment has been minimized.

Show comment
Hide comment
@moschel

moschel Jan 31, 2013

Contributor

Tested this against latest locally and its no longer breaking. Tomorrow we'll push out a new 1.1.4 release candidate at http://canjs.us/release/latest/can.jquery.js (this isn't currently actually pushing out every night automated) and this should confirm this issue is fixed.

Contributor

moschel commented Jan 31, 2013

Tested this against latest locally and its no longer breaking. Tomorrow we'll push out a new 1.1.4 release candidate at http://canjs.us/release/latest/can.jquery.js (this isn't currently actually pushing out every night automated) and this should confirm this issue is fixed.

@moschel

This comment has been minimized.

Show comment
Hide comment
@moschel

moschel Jan 31, 2013

Contributor

Actually nevermind what I said above, forgot I had made a local change. Actually these lines in observe.js cause this weird behavior:

if ( self.__convert ) {
  newVal = self.__convert(prop, newVal);
}

__convert is called twice per nested observe, once here and once a few lines down at self._set(prop, newVal). I'm not sure why this was necessary.

f9896d9 introduced this line.

Commenting these lines out fixes this issue and all other can tests still pass. Will talk to @imjoshdean about this tomorrow and get a test in.

Contributor

moschel commented Jan 31, 2013

Actually nevermind what I said above, forgot I had made a local change. Actually these lines in observe.js cause this weird behavior:

if ( self.__convert ) {
  newVal = self.__convert(prop, newVal);
}

__convert is called twice per nested observe, once here and once a few lines down at self._set(prop, newVal). I'm not sure why this was necessary.

f9896d9 introduced this line.

Commenting these lines out fixes this issue and all other can tests still pass. Will talk to @imjoshdean about this tomorrow and get a test in.

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Feb 5, 2013

Contributor

Closed via #266.

Contributor

daffl commented Feb 5, 2013

Closed via #266.

@daffl daffl closed this Feb 5, 2013

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