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

Commit bae289e

Browse files
committed
fix(compiler): remove dependency on AngularJS private APIs
remove use of private and undocumented arguments to $controller update how preAssignBindingsEnabled is handled for AngularJS 1.7+ update documentation Fixes #11319
1 parent 0046467 commit bae289e

File tree

4 files changed

+109
-73
lines changed

4 files changed

+109
-73
lines changed

package-lock.json

Lines changed: 24 additions & 24 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@
2121
"scss": "./dist/angular-material.scss",
2222
"devDependencies": {
2323
"add-stream": "^1.0.0",
24-
"angular": "^1.5.0",
25-
"angular-animate": "^1.5.0",
26-
"angular-aria": "^1.5.0",
27-
"angular-messages": "^1.5.0",
28-
"angular-mocks": "^1.5.0",
29-
"angular-route": "^1.5.0",
30-
"angular-sanitize": "^1.5.0",
31-
"angular-touch": "^1.5.0",
24+
"angular": "^1.7.2",
25+
"angular-animate": "^1.7.2",
26+
"angular-aria": "^1.7.2",
27+
"angular-messages": "^1.7.2",
28+
"angular-mocks": "^1.7.2",
29+
"angular-route": "^1.7.2",
30+
"angular-sanitize": "^1.7.2",
31+
"angular-touch": "^1.7.2",
3232
"angularytics": "^0.4.0",
3333
"canonical-path": "0.0.2",
3434
"cli-color": "^1.0.0",

src/core/services/compiler/compiler.js

Lines changed: 54 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ function MdCompilerProvider($compileProvider) {
102102
* @name $mdCompilerProvider#respectPreAssignBindingsEnabled
103103
*
104104
* @param {boolean=} respected update the `respectPreAssignBindingsEnabled` state if provided,
105-
* otherwise just return the current Material `respectPreAssignBindingsEnabled` state.
105+
* otherwise just return the current Material `respectPreAssignBindingsEnabled` state. Attempting
106+
* to set this to `false` in AngularJS 1.7+ will throw an exception.
106107
* @returns {boolean|MdCompilerProvider} current value if used as getter or itself (chaining)
107108
* if used as setter
108109
*
@@ -112,31 +113,45 @@ function MdCompilerProvider($compileProvider) {
112113
* this doesn't affect directives/components created via regular AngularJS methods which
113114
* constitute most Material and user-created components.
114115
*
115-
* If disabled (`false`), the compiler assigns the value of each of the bindings to the
116-
* properties of the controller object before the constructor of this object is called.
116+
* If disabled (`false`) and using an AngularJS version >= 1.5.10 and < 1.7.0, the compiler
117+
* assigns the value of each of the bindings to the properties of the controller object before
118+
* the constructor of this object is called.
117119
*
118120
* If enabled (`true`) the behavior depends on the AngularJS version used:
119121
*
120-
* - `<1.5.10` - bindings are pre-assigned.
121-
* - `>=1.5.10 <1.7` - behaves like set to whatever `$compileProvider.preAssignBindingsEnabled()` reports.
122-
* If the `preAssignBindingsEnabled` flag wasn't set manually, it defaults to pre-assigning bindings
123-
* with AngularJS `1.5.x` and to calling the constructor first with AngularJS `1.6.x`.
124-
* - `>=1.7` - the compiler calls the constructor first before assigning bindings and
122+
* - `<1.5.10`
123+
* - Bindings are pre-assigned.
124+
* - `>=1.5.10 <1.7`
125+
* - Respects whatever `$compileProvider.preAssignBindingsEnabled()` reports. If the
126+
* `preAssignBindingsEnabled` flag wasn't set manually, it defaults to pre-assigning bindings
127+
* with AngularJS `1.5` and to calling the constructor first with AngularJS `1.6`.
128+
* - `>=1.7`
129+
* - The compiler calls the constructor first before assigning bindings and
125130
* `$compileProvider.preAssignBindingsEnabled()` no longer exists.
126131
*
127-
* The default value is `false` but will change to `true` in AngularJS Material 1.2.
132+
* Defaults
133+
* - The default value is `false` in AngularJS 1.6 and earlier.
134+
* - It is planned to change this to default to `true` in AngularJS Material 1.2.
135+
* - In AngularJS 1.7 and later, the value is always `true`. Trying to set the value to false will
136+
* throw an exception.
128137
*
129-
* It is recommended to set this flag to `true` in AngularJS Material 1.1.x. The only reason
130-
* it's not set that way by default is backwards compatibility. Not setting the flag to `true`
131-
* when AngularJS' `$compileProvider.preAssignBindingsEnabled()` is set to `false`
132-
* (i.e. default behavior in AngularJS 1.6 or newer) makes it hard to unit test
138+
* It is recommended to set this flag to `true` when using AngularJS Material 1.1.x with
139+
* AngularJS versions >= 1.5.10 and < 1.7.0. The only reason
140+
* it's not set that way by default is backwards compatibility.
141+
*
142+
* By not setting the flag to `true` when AngularJS' `$compileProvider.preAssignBindingsEnabled()`
143+
* is set to `false` (i.e. default behavior in AngularJS 1.6 or newer), unit testing of
133144
* Material Dialog/Panel/Toast/BottomSheet controllers using the `$controller` helper
134-
* as it always follows the `$compileProvider.preAssignBindingsEnabled()` value.
145+
* is problematic as it always follows AngularJS' `$compileProvider.preAssignBindingsEnabled()`
146+
* value.
135147
*/
136-
// TODO change it to `true` in Material 1.2.
137-
var respectPreAssignBindingsEnabled = false;
148+
var respectPreAssignBindingsEnabled = angular.version.major === 1 && angular.version.minor >= 7;
138149
this.respectPreAssignBindingsEnabled = function(respected) {
139150
if (angular.isDefined(respected)) {
151+
if (!respected && angular.version.major === 1 && angular.version.minor >= 7) {
152+
throw new Error(
153+
'Disabling respectPreAssignBindingsEnabled is not supported in AngularJS 1.7 or later.');
154+
}
140155
respectPreAssignBindingsEnabled = respected;
141156
return this;
142157
}
@@ -444,27 +459,36 @@ function MdCompilerProvider($compileProvider) {
444459

445460
/**
446461
* Creates and instantiates a new controller with the specified options.
447-
* @param {!Object} options Options that include the controller
462+
* @param {!Object} options Options that include the controller function or string.
448463
* @param {!Object} injectLocals Locals to to be provided in the controller DI.
449464
* @param {!Object} locals Locals to be injected to the controller.
450465
* @returns {!Object} Created controller instance.
451466
*/
452467
MdCompilerService.prototype._createController = function(options, injectLocals, locals) {
453-
// The third and fourth arguments to $controller are considered private and are undocumented:
454-
// https://github.com/angular/angular.js/blob/master/src/ng/controller.js#L86
455-
// Passing `true` as the third argument causes `$controller` to return a function that
456-
// gets the controller instance instead returning of the instance directly. When the
457-
// controller is defined as a function, `invokeCtrl.instance` is the *same instance* as
458-
// `invokeCtrl()`. However, then the controller is an ES6 class, `invokeCtrl.instance` is a
459-
// *different instance* from `invokeCtrl()`.
460-
var invokeCtrl = this.$controller(options.controller, injectLocals, true, options.controllerAs);
461-
462-
if (getPreAssignBindingsEnabled() && options.bindToController) {
463-
angular.extend(invokeCtrl.instance, locals);
468+
var ctrl;
469+
if (angular.version.major === 1 && angular.version.minor >= 7) {
470+
ctrl = this.$controller(options.controller, injectLocals);
471+
} else {
472+
// The third argument to $controller is considered private and undocumented.
473+
// Details: https://github.com/angular/angular.js/blob/v1.6.10/src/ng/controller.js#L102-L109
474+
// Passing `true` as the third argument causes `$controller` to return a function that
475+
// gets the controller instance instead of returning the instance directly. When the
476+
// controller is defined as a function, `invokeCtrl.instance` is the *same instance* as
477+
// `invokeCtrl()`. However, when the controller is an ES6 class, `invokeCtrl.instance` is a
478+
// *different instance* from `invokeCtrl()`.
479+
var invokeCtrl = this.$controller(options.controller, injectLocals, true);
480+
481+
if (getPreAssignBindingsEnabled() && options.bindToController) {
482+
angular.extend(invokeCtrl.instance, locals);
483+
}
484+
485+
// Instantiate and initialize the specified controller.
486+
ctrl = invokeCtrl();
464487
}
465488

466-
// Instantiate and initialize the specified controller.
467-
var ctrl = invokeCtrl();
489+
if (options.controllerAs) {
490+
injectLocals.$scope[options.controllerAs] = ctrl;
491+
}
468492

469493
if (!getPreAssignBindingsEnabled() && options.bindToController) {
470494
angular.extend(ctrl, locals);

src/core/services/compiler/compiler.spec.js

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ describe('$mdCompiler service', function() {
112112
var data = compile({
113113
template: '<span>hello</span>'
114114
});
115-
var scope = $rootScope.$new();
115+
var scope = $rootScope.$new(false);
116116
data.link(scope);
117117
expect(data.element.scope()).toBe(scope);
118118
}));
@@ -127,7 +127,7 @@ describe('$mdCompiler service', function() {
127127
this.injectedOne = one;
128128
}
129129
});
130-
var scope = $rootScope.$new();
130+
var scope = $rootScope.$new(false);
131131
data.link(scope);
132132
expect(data.element.controller()).toBeTruthy();
133133
expect(data.element.controller().injectedOne).toBe(1);
@@ -143,7 +143,7 @@ describe('$mdCompiler service', function() {
143143
}
144144
});
145145

146-
var scope = $rootScope.$new();
146+
var scope = $rootScope.$new(false);
147147
data.link(scope);
148148

149149
expect(ctrlElement).toBe(data.element);
@@ -155,7 +155,7 @@ describe('$mdCompiler service', function() {
155155
controller: function Ctrl() {},
156156
controllerAs: 'myControllerAs'
157157
});
158-
var scope = $rootScope.$new();
158+
var scope = $rootScope.$new(false);
159159
data.link(scope);
160160
expect(scope.myControllerAs).toBe(data.element.controller());
161161
}));
@@ -164,12 +164,24 @@ describe('$mdCompiler service', function() {
164164

165165
});
166166

167-
[
167+
var bindingStatesToTest;
168+
if (angular.version.major === 1 && angular.version.minor >= 7) {
169+
bindingStatesToTest = [
170+
{respectPreAssignBindingsEnabled: true}
171+
];
172+
} else if (angular.version.major === 1 && angular.version.minor === 6) {
173+
bindingStatesToTest = [
168174
{respectPreAssignBindingsEnabled: true},
169175
{respectPreAssignBindingsEnabled: false},
170-
// TODO change `equivalentTo` to `true` in Material 1.2.
171176
{respectPreAssignBindingsEnabled: '"default"', equivalentTo: false}
172-
].forEach(function(options) {
177+
];
178+
} else if (angular.version.major === 1 && angular.version.minor < 6) {
179+
bindingStatesToTest = [
180+
{respectPreAssignBindingsEnabled: false}
181+
];
182+
}
183+
184+
bindingStatesToTest.forEach(function(options) {
173185
var realRespectPreAssignBindingsEnabled = options.respectPreAssignBindingsEnabled;
174186
var respectPreAssignBindingsEnabled = angular.isDefined(options.equivalentTo) ?
175187
options.equivalentTo :
@@ -224,8 +236,8 @@ describe('$mdCompiler service', function() {
224236
expect(isInstantiated).toBe(true);
225237
});
226238

227-
// Bindings are not preassigned only if we respect the AngularJS config and they're
228-
// disabled there. This logic will change in Material 1.2.0.
239+
// Bindings are not pre-assigned if we respect the AngularJS config and pre-assigning
240+
// them is disabled there.
229241
if (respectPreAssignBindingsEnabled && !preAssignBindingsEnabledInAngularJS) {
230242
it('disabled should assign bindings after constructor', function() {
231243
var isInstantiated = false;
@@ -400,7 +412,7 @@ describe('$mdCompiler service', function() {
400412

401413
it('should preserve a previous linked scope', function() {
402414

403-
var scope = $rootScope.$new();
415+
var scope = $rootScope.$new(false);
404416

405417
var data = compile({
406418
contentElement: $compile('<div>With Scope</div>')(scope)
@@ -445,7 +457,7 @@ describe('$mdCompiler service', function() {
445457
beforeEach(inject(function($injector) {
446458
$mdCompiler = $injector.get('$mdCompiler');
447459
$rootScope = $injector.get('$rootScope');
448-
pageScope = $rootScope.$new();
460+
pageScope = $rootScope.$new(false);
449461
}));
450462

451463
it('should assign bindings by $onInit for ES6 classes', function(done) {

0 commit comments

Comments
 (0)