Skip to content

Commit

Permalink
revert: "fix($compile): do not write @-bound properties if attribute …
Browse files Browse the repository at this point in the history 
…is not present"

This reverts commit 8a1eb16.

This commit broke the tabs component on the material project,
which caused internal breakages. Will open a separate issue to
look into the issue.
  • Loading branch information
@jeffbcross
jeffbcross committed Jul 2, 2015
1 parent 4da1cc3 commit d193c3a
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 124 deletions.
27 changes: 15 additions & 12 deletions src/ng/compile.js
View file
Expand Up @@ -2567,32 +2567,36 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
lastValue,
parentGet, parentSet, compare;

if (!hasOwnProperty.call(attrs, attrName)) {
// In the case of user defined a binding with the same name as a method in Object.prototype but didn't set
// the corresponding attribute. We need to make sure subsequent code won't access to the prototype function
attrs[attrName] = undefined;
}

switch (mode) {

case '@':
if (!optional && !hasOwnProperty.call(attrs, attrName)) {
destination[scopeName] = attrs[attrName] = void 0;
if (!attrs[attrName] && !optional) {
destination[scopeName] = undefined;
}

attrs.$observe(attrName, function(value) {
if (isString(value)) {
destination[scopeName] = value;
}
destination[scopeName] = value;
});
attrs.$$observers[attrName].$$scope = scope;
if (isString(attrs[attrName])) {
if (attrs[attrName]) {
// If the attribute has been provided then we trigger an interpolation to ensure
// the value is there for use in the link fn
destination[scopeName] = $interpolate(attrs[attrName])(scope);
}
break;

case '=':
if (!hasOwnProperty.call(attrs, attrName)) {
if (optional) break;
attrs[attrName] = void 0;
if (optional && !attrs[attrName]) {
return;
}

parentGet = $parse(attrs[attrName]);

if (parentGet.literal) {
compare = equals;
} else {
Expand Down Expand Up @@ -2631,8 +2635,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
break;

case '&':
// Don't assign Object.prototype method to scope
parentGet = attrs.hasOwnProperty(attrName) ? $parse(attrs[attrName]) : noop;
parentGet = $parse(attrs[attrName]);

// Don't assign noop to destination if expression is not valid
if (parentGet === noop && optional) break;
Expand Down
124 changes: 12 additions & 112 deletions test/ng/compileSpec.js
View file
Expand Up @@ -2543,17 +2543,10 @@ describe('$compile', function() {
};

expect(func).not.toThrow();
var scope = element.isolateScope();
expect(element.find('span').scope()).toBe(scope);
expect(scope).not.toBe($rootScope);

// Not shadowed because optional
expect(scope.constructor).toBe($rootScope.constructor);
expect(scope.hasOwnProperty('constructor')).toBe(false);

// Shadowed with undefined because not optional
expect(scope.valueOf).toBeUndefined();
expect(scope.hasOwnProperty('valueOf')).toBe(true);
expect(element.find('span').scope()).toBe(element.isolateScope());
expect(element.isolateScope()).not.toBe($rootScope);
expect(element.isolateScope()['constructor']).toBe($rootScope.constructor);
expect(element.isolateScope()['valueOf']).toBeUndefined();
})
);

Expand All @@ -2568,13 +2561,10 @@ describe('$compile', function() {
};

expect(func).not.toThrow();
var scope = element.isolateScope();
expect(element.find('span').scope()).toBe(scope);
expect(scope).not.toBe($rootScope);
expect(scope.constructor).toBe('constructor');
expect(scope.hasOwnProperty('constructor')).toBe(true);
expect(scope.valueOf).toBe('valueOf');
expect(scope.hasOwnProperty('valueOf')).toBe(true);
expect(element.find('span').scope()).toBe(element.isolateScope());
expect(element.isolateScope()).not.toBe($rootScope);
expect(element.isolateScope()['constructor']).toBe('constructor');
expect(element.isolateScope()['valueOf']).toBe('valueOf');
})
);

Expand All @@ -2585,17 +2575,10 @@ describe('$compile', function() {
};

expect(func).not.toThrow();
var scope = element.isolateScope();
expect(element.find('span').scope()).toBe(scope);
expect(scope).not.toBe($rootScope);

// Does not shadow value because optional
expect(scope.constructor).toBe($rootScope.constructor);
expect(scope.hasOwnProperty('constructor')).toBe(false);

// Shadows value because not optional
expect(scope.valueOf).toBeUndefined();
expect(scope.hasOwnProperty('valueOf')).toBe(true);
expect(element.find('span').scope()).toBe(element.isolateScope());
expect(element.isolateScope()).not.toBe($rootScope);
expect(element.isolateScope()['constructor']).toBe($rootScope.constructor);
expect(element.isolateScope()['valueOf']).toBeUndefined();
})
);

Expand Down Expand Up @@ -3593,31 +3576,6 @@ describe('$compile', function() {
}));


it('should not overwrite @-bound property each digest when not present', function() {
module(function($compileProvider) {
$compileProvider.directive('testDir', valueFn({
scope: {prop: '@'},
controller: function($scope) {
$scope.prop = $scope.prop || 'default';
this.getProp = function() {
return $scope.prop;
};
},
controllerAs: 'ctrl',
template: '<p></p>'
}));
});
inject(function($compile, $rootScope) {
element = $compile('<div test-dir></div>')($rootScope);
var scope = element.isolateScope();
expect(scope.ctrl.getProp()).toBe('default');

$rootScope.$digest();
expect(scope.ctrl.getProp()).toBe('default');
});
});


describe('bind-once', function() {

function countWatches(scope) {
Expand Down Expand Up @@ -4479,64 +4437,6 @@ describe('$compile', function() {
childScope.theCtrl.test();
});
});

describe('should not overwrite @-bound property each digest when not present', function() {
it('when creating new scope', function() {
module(function($compileProvider) {
$compileProvider.directive('testDir', valueFn({
scope: true,
bindToController: {
prop: '@'
},
controller: function() {
var self = this;
this.prop = this.prop || 'default';
this.getProp = function() {
return self.prop;
};
},
controllerAs: 'ctrl',
template: '<p></p>'
}));
});
inject(function($compile, $rootScope) {
element = $compile('<div test-dir></div>')($rootScope);
var scope = element.scope();
expect(scope.ctrl.getProp()).toBe('default');

$rootScope.$digest();
expect(scope.ctrl.getProp()).toBe('default');
});
});

it('when creating isolate scope', function() {
module(function($compileProvider) {
$compileProvider.directive('testDir', valueFn({
scope: {},
bindToController: {
prop: '@'
},
controller: function() {
var self = this;
this.prop = this.prop || 'default';
this.getProp = function() {
return self.prop;
};
},
controllerAs: 'ctrl',
template: '<p></p>'
}));
});
inject(function($compile, $rootScope) {
element = $compile('<div test-dir></div>')($rootScope);
var scope = element.isolateScope();
expect(scope.ctrl.getProp()).toBe('default');

$rootScope.$digest();
expect(scope.ctrl.getProp()).toBe('default');
});
});
});
});


Expand Down

0 comments on commit d193c3a

Please sign in to comment.