Skip to content

Commit

Permalink
feat(compile): '=' binding can now be optional
Browse files Browse the repository at this point in the history
If you bind using '=' to a non-existant parent property, the compiler
will throw a NON_ASSIGNABLE_MODEL_EXPRESSION exception, which is right
because the model doesn't exist.

This enhancement allow to specify that a binding is optional so it
won't complain if the parent property is not defined. In order to mantain
backward compability, the new behaviour must be specified using '=?' instead
of '='. The local property will be undefined is these cases.

Closes angular#909
Closes angular#1435
  • Loading branch information
lrlopez committed Jan 26, 2013
1 parent 69be39f commit c07c4b8
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 3 deletions.
4 changes: 3 additions & 1 deletion docs/content/guide/directive.ngdoc
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,9 @@ compiler}. The attributes are:
Given `<widget my-attr="parentModel">` and widget definition of
`scope: { localModel:'=myAttr' }`, then widget scope property `localModel` will reflect the
value of `parentModel` on the parent scope. Any changes to `parentModel` will be reflected
in `localModel` and any changes in `localModel` will reflect in `parentModel`.
in `localModel` and any changes in `localModel` will reflect in `parentModel`. If the parent
scope property doesn't exist, it will throw a NON_ASSIGNABLE_MODEL_EXPRESSION exception. You
can avoid this behavior using `=?` or `=?attr` in order to flag the property as optional.

* `&` or `&attr` - provides a way to execute an expression in the context of the parent scope.
If no `attr` name is specified then the attribute name is assumed to be the same as the
Expand Down
8 changes: 6 additions & 2 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -714,13 +714,14 @@ function $CompileProvider($provide) {
$element = attrs.$$element;

if (newIsolateScopeDirective) {
var LOCAL_REGEXP = /^\s*([@=&])\s*(\w*)\s*$/;
var LOCAL_REGEXP = /^\s*([@=&])(\??)\s*(\w*)\s*$/;

var parentScope = scope.$parent || scope;

forEach(newIsolateScopeDirective.scope, function(definiton, scopeName) {
var match = definiton.match(LOCAL_REGEXP) || [],
attrName = match[2]|| scopeName,
attrName = match[3] || scopeName,
optional = (match[2] == '?'),
mode = match[1], // @, =, or &
lastValue,
parentGet, parentSet;
Expand All @@ -736,6 +737,9 @@ function $CompileProvider($provide) {
}

case '=': {
if (optional && (attrs[attrName] == null)) {

This comment has been minimized.

Copy link
@IgorMinar

IgorMinar Feb 25, 2013

why null? it should be !attrs[attrName]

return;
}
parentGet = $parse(attrs[attrName]);
parentSet = parentGet.assign || function() {
// reset the change, or we will throw this exception on every $digest
Expand Down
104 changes: 104 additions & 0 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1775,6 +1775,9 @@ describe('$compile', function() {
ref: '=',
refAlias: '= ref',
reference: '=',
optref: '=?',
optrefAlias: '=? optref',
optreference: '=?',
expr: '&',
exprAlias: '&expr'
},
Expand Down Expand Up @@ -1917,6 +1920,107 @@ describe('$compile', function() {
});


describe('optional object reference', function() {
it('should update local when origin changes', inject(function() {

This comment has been minimized.

Copy link
@IgorMinar

IgorMinar Feb 25, 2013

I would keep just this and the last spec. all the other once are duplicates and don't really add much value.

compile('<div><span my-component optref="name">');
expect(componentScope.optRef).toBe(undefined);
expect(componentScope.optRefAlias).toBe(componentScope.optRef);

$rootScope.name = 'misko';
$rootScope.$apply();
expect(componentScope.optref).toBe($rootScope.name);
expect(componentScope.optrefAlias).toBe($rootScope.name);

$rootScope.name = {};
$rootScope.$apply();
expect(componentScope.optref).toBe($rootScope.name);
expect(componentScope.optrefAlias).toBe($rootScope.name);
}));


it('should update local when origin changes', inject(function() {
compile('<div><span my-component optRef="name">');
expect(componentScope.optref).toBe(undefined);
expect(componentScope.optrefAlias).toBe(componentScope.optref);

componentScope.optref = 'misko';
$rootScope.$apply();
expect($rootScope.name).toBe('misko');
expect(componentScope.optref).toBe('misko');
expect($rootScope.name).toBe(componentScope.optref);
expect(componentScope.optrefAlias).toBe(componentScope.optref);

componentScope.name = {};
$rootScope.$apply();
expect($rootScope.name).toBe(componentScope.optref);
expect(componentScope.optrefAlias).toBe(componentScope.optref);
}));


it('should update local when both change', inject(function() {
compile('<div><span my-component optref="name">');
$rootScope.name = {mark:123};
componentScope.optref = 'misko';

$rootScope.$apply();
expect($rootScope.name).toEqual({mark:123})
expect(componentScope.optref).toBe($rootScope.name);
expect(componentScope.optrefAlias).toBe($rootScope.name);

$rootScope.name = 'igor';
componentScope.optref = {};
$rootScope.$apply();
expect($rootScope.name).toEqual('igor')
expect(componentScope.optref).toBe($rootScope.name);
expect(componentScope.optrefAlias).toBe($rootScope.name);
}));

it('should complain on non assignable changes', inject(function() {
compile('<div><span my-component optref="\'hello \' + name">');
$rootScope.name = 'world';
$rootScope.$apply();
expect(componentScope.optref).toBe('hello world');

componentScope.optref = 'ignore me';
expect($rootScope.$apply).
toThrow("Non-assignable model expression: 'hello ' + name (directive: myComponent)");
expect(componentScope.optref).toBe('hello world');
// reset since the exception was rethrown which prevented phase clearing
$rootScope.$$phase = null;

$rootScope.name = 'misko';
$rootScope.$apply();
expect(componentScope.optref).toBe('hello misko');
}));

// regression
it('should stabilize model', inject(function() {
compile('<div><span my-component optreference="name">');

var lastRefValueInParent;
$rootScope.$watch('name', function(ref) {
lastRefValueInParent = ref;
});

$rootScope.name = 'aaa';
$rootScope.$apply();

componentScope.optreference = 'new';
$rootScope.$apply();

expect(lastRefValueInParent).toBe('new');
}));

it('should not throw exception when reference does not exist', inject(function() {
compile('<div><span my-component>');

expect(componentScope.optref).toBe(undefined);
expect(componentScope.optrefAlias).toBe(undefined);
expect(componentScope.optreference).toBe(undefined);
}));
});


describe('executable expression', function() {
it('should allow expression execution with locals', inject(function() {
compile('<div><span my-component expr="count = count + offset">');
Expand Down

0 comments on commit c07c4b8

Please sign in to comment.