Skip to content

Commit

Permalink
fix($parse) - Allow setterFn to bind correctly to promise results
Browse files Browse the repository at this point in the history
When binding the result of a promise directly to an ng:model directive,
getterFn successfully extracts the data from the promise by traversing
the '$$v' attribute.

However the setterFn was not successfully traversing to the $$v
attribute, and instead writing model updates directly on the promise
itself.

The subsequent apply/digest phase of Angular was then pulling the
original value from the promise and applying it back into the viewState,
thus simulating a read-only field.

In summary, setterFn was writing to the root of the promise, while
getterFn was reading from the '$$v' component.

Fixed by modifying setterFn to unwrap promises if found and read $$v.

If promises have not been resolved yet, bindings will function as they
previously did (act as a read-only field). This appears to be more of a
side effect than intentional design in the existing codebase, however
I kept this behaviour in this patch to minimize the chance of breaking
existing projects.

Closes angular#1827

Also see: http://stackoverflow.com/questions/16883239/angularjs-ngmodel-
field-is-readonly-when-bound-to-q-promise/16883387
  • Loading branch information
James Davies committed Jun 3, 2013
1 parent b6a0777 commit 798905d
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 1 deletion.
16 changes: 15 additions & 1 deletion src/ng/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,20 @@ function parser(text, json, $filter, csp){
// Parser helper functions
//////////////////////////////////////////////////

// Unwrap values from a promise
function unwrapPromise(obj){

// If we'v been passed a promise
if( obj.then ){

// Then unwrap the value. If the value has not been returned yet, bind to a throwaway object. This will simulate a read-only field.
return obj.$$v || {}
}

return obj;
}


function setter(obj, path, setValue) {
var element = path.split('.');
for (var i = 0; element.length > 1; i++) {
Expand All @@ -714,7 +728,7 @@ function setter(obj, path, setValue) {
propertyObj = {};
obj[key] = propertyObj;
}
obj = propertyObj;
obj = unwrapPromise( propertyObj );
}
obj[element.shift()] = setValue;
return setValue;
Expand Down
45 changes: 45 additions & 0 deletions test/ng/parseSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,51 @@ describe('parser', function() {
expect(scope.$eval('greeting')).toBe('hello!');
});

it('should evaluate a resolved object promise and set its value', inject(function($parse) {
var person = {'name': 'Bill Gates'};

deferred.resolve(person);
scope.person = promise;

var getter = $parse( 'person.name' );

expect(getter(scope)).toBe(undefined);
scope.$digest();
expect(getter(scope)).toBe('Bill Gates');

getter.assign(scope, 'Warren Buffet');
expect(getter(scope)).toBe('Warren Buffet');
}));

it('should evaluate a resolved primitive type promise and set its value', inject(function($parse) {
deferred.resolve("Salut!");
scope.greeting = promise;

var getter = $parse( 'greeting' );

expect(getter(scope)).toBe(undefined);
scope.$digest();
expect(getter(scope)).toBe('Salut!');

getter.assign(scope, 'Bonjour');
expect(getter(scope)).toBe('Bonjour');
}));

it('should evaluate an unresolved promise and *not* set its value', inject(function($parse) {


scope.person = promise;

var getter = $parse( 'person.name' );

expect(getter(scope)).toBe(undefined);
scope.$digest();
expect(getter(scope)).toBe(undefined);

getter.assign(scope, 'Warren Buffet');
expect(getter(scope)).toBe(undefined);
}));


it('should evaluated rejected promise and ignore the rejection reason', function() {
deferred.reject('sorry');
Expand Down

0 comments on commit 798905d

Please sign in to comment.