Skip to content

Commit aad2269

Browse files
jeffbcrossnetman92
authored andcommitted
revert: "fix($compile): do not write @-bound properties if attribute 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.
1 parent 5f8f995 commit aad2269

File tree

2 files changed

+27
-124
lines changed

2 files changed

+27
-124
lines changed

src/ng/compile.js

+15-12
Original file line numberDiff line numberDiff line change
@@ -2567,32 +2567,36 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
25672567
lastValue,
25682568
parentGet, parentSet, compare;
25692569

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

25722578
case '@':
2573-
if (!optional && !hasOwnProperty.call(attrs, attrName)) {
2574-
destination[scopeName] = attrs[attrName] = void 0;
2579+
if (!attrs[attrName] && !optional) {
2580+
destination[scopeName] = undefined;
25752581
}
2582+
25762583
attrs.$observe(attrName, function(value) {
2577-
if (isString(value)) {
2578-
destination[scopeName] = value;
2579-
}
2584+
destination[scopeName] = value;
25802585
});
25812586
attrs.$$observers[attrName].$$scope = scope;
2582-
if (isString(attrs[attrName])) {
2587+
if (attrs[attrName]) {
25832588
// If the attribute has been provided then we trigger an interpolation to ensure
25842589
// the value is there for use in the link fn
25852590
destination[scopeName] = $interpolate(attrs[attrName])(scope);
25862591
}
25872592
break;
25882593

25892594
case '=':
2590-
if (!hasOwnProperty.call(attrs, attrName)) {
2591-
if (optional) break;
2592-
attrs[attrName] = void 0;
2595+
if (optional && !attrs[attrName]) {
2596+
return;
25932597
}
2594-
25952598
parentGet = $parse(attrs[attrName]);
2599+
25962600
if (parentGet.literal) {
25972601
compare = equals;
25982602
} else {
@@ -2631,8 +2635,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
26312635
break;
26322636

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

26372640
// Don't assign noop to destination if expression is not valid
26382641
if (parentGet === noop && optional) break;

test/ng/compileSpec.js

+12-112
Original file line numberDiff line numberDiff line change
@@ -2543,17 +2543,10 @@ describe('$compile', function() {
25432543
};
25442544

25452545
expect(func).not.toThrow();
2546-
var scope = element.isolateScope();
2547-
expect(element.find('span').scope()).toBe(scope);
2548-
expect(scope).not.toBe($rootScope);
2549-
2550-
// Not shadowed because optional
2551-
expect(scope.constructor).toBe($rootScope.constructor);
2552-
expect(scope.hasOwnProperty('constructor')).toBe(false);
2553-
2554-
// Shadowed with undefined because not optional
2555-
expect(scope.valueOf).toBeUndefined();
2556-
expect(scope.hasOwnProperty('valueOf')).toBe(true);
2546+
expect(element.find('span').scope()).toBe(element.isolateScope());
2547+
expect(element.isolateScope()).not.toBe($rootScope);
2548+
expect(element.isolateScope()['constructor']).toBe($rootScope.constructor);
2549+
expect(element.isolateScope()['valueOf']).toBeUndefined();
25572550
})
25582551
);
25592552

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

25702563
expect(func).not.toThrow();
2571-
var scope = element.isolateScope();
2572-
expect(element.find('span').scope()).toBe(scope);
2573-
expect(scope).not.toBe($rootScope);
2574-
expect(scope.constructor).toBe('constructor');
2575-
expect(scope.hasOwnProperty('constructor')).toBe(true);
2576-
expect(scope.valueOf).toBe('valueOf');
2577-
expect(scope.hasOwnProperty('valueOf')).toBe(true);
2564+
expect(element.find('span').scope()).toBe(element.isolateScope());
2565+
expect(element.isolateScope()).not.toBe($rootScope);
2566+
expect(element.isolateScope()['constructor']).toBe('constructor');
2567+
expect(element.isolateScope()['valueOf']).toBe('valueOf');
25782568
})
25792569
);
25802570

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

25872577
expect(func).not.toThrow();
2588-
var scope = element.isolateScope();
2589-
expect(element.find('span').scope()).toBe(scope);
2590-
expect(scope).not.toBe($rootScope);
2591-
2592-
// Does not shadow value because optional
2593-
expect(scope.constructor).toBe($rootScope.constructor);
2594-
expect(scope.hasOwnProperty('constructor')).toBe(false);
2595-
2596-
// Shadows value because not optional
2597-
expect(scope.valueOf).toBeUndefined();
2598-
expect(scope.hasOwnProperty('valueOf')).toBe(true);
2578+
expect(element.find('span').scope()).toBe(element.isolateScope());
2579+
expect(element.isolateScope()).not.toBe($rootScope);
2580+
expect(element.isolateScope()['constructor']).toBe($rootScope.constructor);
2581+
expect(element.isolateScope()['valueOf']).toBeUndefined();
25992582
})
26002583
);
26012584

@@ -3593,31 +3576,6 @@ describe('$compile', function() {
35933576
}));
35943577

35953578

3596-
it('should not overwrite @-bound property each digest when not present', function() {
3597-
module(function($compileProvider) {
3598-
$compileProvider.directive('testDir', valueFn({
3599-
scope: {prop: '@'},
3600-
controller: function($scope) {
3601-
$scope.prop = $scope.prop || 'default';
3602-
this.getProp = function() {
3603-
return $scope.prop;
3604-
};
3605-
},
3606-
controllerAs: 'ctrl',
3607-
template: '<p></p>'
3608-
}));
3609-
});
3610-
inject(function($compile, $rootScope) {
3611-
element = $compile('<div test-dir></div>')($rootScope);
3612-
var scope = element.isolateScope();
3613-
expect(scope.ctrl.getProp()).toBe('default');
3614-
3615-
$rootScope.$digest();
3616-
expect(scope.ctrl.getProp()).toBe('default');
3617-
});
3618-
});
3619-
3620-
36213579
describe('bind-once', function() {
36223580

36233581
function countWatches(scope) {
@@ -4479,64 +4437,6 @@ describe('$compile', function() {
44794437
childScope.theCtrl.test();
44804438
});
44814439
});
4482-
4483-
describe('should not overwrite @-bound property each digest when not present', function() {
4484-
it('when creating new scope', function() {
4485-
module(function($compileProvider) {
4486-
$compileProvider.directive('testDir', valueFn({
4487-
scope: true,
4488-
bindToController: {
4489-
prop: '@'
4490-
},
4491-
controller: function() {
4492-
var self = this;
4493-
this.prop = this.prop || 'default';
4494-
this.getProp = function() {
4495-
return self.prop;
4496-
};
4497-
},
4498-
controllerAs: 'ctrl',
4499-
template: '<p></p>'
4500-
}));
4501-
});
4502-
inject(function($compile, $rootScope) {
4503-
element = $compile('<div test-dir></div>')($rootScope);
4504-
var scope = element.scope();
4505-
expect(scope.ctrl.getProp()).toBe('default');
4506-
4507-
$rootScope.$digest();
4508-
expect(scope.ctrl.getProp()).toBe('default');
4509-
});
4510-
});
4511-
4512-
it('when creating isolate scope', function() {
4513-
module(function($compileProvider) {
4514-
$compileProvider.directive('testDir', valueFn({
4515-
scope: {},
4516-
bindToController: {
4517-
prop: '@'
4518-
},
4519-
controller: function() {
4520-
var self = this;
4521-
this.prop = this.prop || 'default';
4522-
this.getProp = function() {
4523-
return self.prop;
4524-
};
4525-
},
4526-
controllerAs: 'ctrl',
4527-
template: '<p></p>'
4528-
}));
4529-
});
4530-
inject(function($compile, $rootScope) {
4531-
element = $compile('<div test-dir></div>')($rootScope);
4532-
var scope = element.isolateScope();
4533-
expect(scope.ctrl.getProp()).toBe('default');
4534-
4535-
$rootScope.$digest();
4536-
expect(scope.ctrl.getProp()).toBe('default');
4537-
});
4538-
});
4539-
});
45404440
});
45414441

45424442

0 commit comments

Comments
 (0)