can.Observe makes can.Deferred into an observable #367

Merged
merged 1 commit into from May 6, 2013

Conversation

Projects
None yet
2 participants
@daffl
Contributor

daffl commented May 6, 2013

The current implementation of canMakeObserve is:

canMakeObserve = function( obj ) {
  return obj && (can.isArray(obj) || can.isPlainObject( obj ) || ( obj instanceof can.Observe ));
}

As can.isPlainObject returns true for a can.Deferred, a promise that is part of a model's raw data values will be treated as a plain object to be wrapped as a child can.Observe with observable members itself. This is almost certainly not the intention.

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Apr 23, 2013

Contributor

True, that should not be converted. We'll fix it in the next release.

Contributor

daffl commented Apr 23, 2013

True, that should not be converted. We'll fix it in the next release.

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl May 3, 2013

Contributor

What actually would be really cool is if can.Observe realizes it is a Deferred, waits for it to complete and then sets the attribute with the result data. Not sure if that always applies though.

Contributor

daffl commented May 3, 2013

What actually would be really cool is if can.Observe realizes it is a Deferred, waits for it to complete and then sets the attribute with the result data. Not sure if that always applies though.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer May 3, 2013

Contributor

@daffl It shouldn't be converted, but it also shouldn't replace itself, but that's why I want: #366.

Contributor

justinbmeyer commented May 3, 2013

@daffl It shouldn't be converted, but it also shouldn't replace itself, but that's why I want: #366.

daffl added a commit that referenced this pull request May 6, 2013

Merge pull request #367 from bitovi/deferred-conversion-367
can.Observe makes can.Deferred into an observable

@daffl daffl merged commit 39d549c into master May 6, 2013

1 check passed

default The Travis build passed
Details

@daffl daffl deleted the deferred-conversion-367 branch May 6, 2013

@justinbmeyer justinbmeyer restored the deferred-conversion-367 branch May 6, 2013

@justinbmeyer justinbmeyer deleted the deferred-conversion-367 branch May 6, 2013

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