Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($parse) - Allow setterFn to bind correctly to promise results #2854

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 ){

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coding style

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to adjust. Can you elaborate on the issue?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You seem to be using a different style than the rest of this file (if (is('(){}[].,;:?')) vs if( obj.then ){ for example)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ta. I've gone with the style

if(condition) {
// Block
}

which appears to be commonly used throughout that file?

I was missing a null check (which I've just added), and have expanded on my function comments a bit more. Closer to what's expected? (The if() statement is now identical to other promise check within the same file)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm talking about spacing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right. That's obvious now that I see it. Will adjust.


// 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