Skip to content

Commit 203c380

Browse files
committed
fix(compiler): respect preAssignBindingsEnabled state
* The `$mdCompiler` now respects the `preAssignBindingsEnabled` state when using AngularJS 1.6 or higher. **BREAKING CHANGE** Developers that are upgrading from AngularJS 1.5 to AngularJS 1.6 may run into issues with their custom controllers. All custom controllers that are are being used inside of Material components, like in the `$mdDialog` or `$mdToast`, can be affected. > Limited to controllers that are used in combination with the `bindToController` and `locals` option. Angular Material starts respecting the [`preAssignBindingsEnabled`](angular/angular.js#15352) state when using AngularJS 1.6+ This will mean that bindings are no longer initialized at `constructor` time. ```js $mdDialog.show({ locals: { myVar: true }, controller: MyController, bindToController: true } function MyController() { // No locals from Angular Material are set yet. e.g myVar is undefined. } MyController.prototype.$onInit = function() { // Bindings are now set in the $onInit lifecycle hook. } ``` To provide consistency, Angular Material components with custom controllers now work with the `$onInit` lifecycle hook (**no** other lifecycle hooks) Fixes angular#10016
1 parent 0c4e895 commit 203c380

File tree

5 files changed

+183
-68
lines changed

5 files changed

+183
-68
lines changed

package.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
"jquery": "^3.0.0",
6262
"jshint": "^2.9.2",
6363
"jshint-summary": "^0.4.0",
64-
"karma": "^1.0.0",
64+
"karma": "1.4.1",
6565
"karma-chrome-launcher": "^2.0.0",
6666
"karma-firefox-launcher": "^1.0.0",
6767
"karma-jasmine": "^1.0.2",
@@ -84,4 +84,4 @@
8484
"merge-base": "git merge-base $(npm run -s current-branch) origin/master",
8585
"squash": "git rebase -i $(npm run -s merge-base)"
8686
}
87-
}
87+
}

src/components/dialog/dialog.js

+30-20
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ function MdDialogProvider($$interimElementProvider) {
604604
});
605605

606606
/* @ngInject */
607-
function advancedDialogOptions($mdDialog, $mdConstant) {
607+
function advancedDialogOptions() {
608608
return {
609609
template: [
610610
'<md-dialog md-theme="{{ dialog.theme || dialog.defaultTheme }}" aria-label="{{ dialog.ariaLabel }}" ng-class="dialog.css">',
@@ -631,30 +631,40 @@ function MdDialogProvider($$interimElementProvider) {
631631
' </md-dialog-actions>',
632632
'</md-dialog>'
633633
].join('').replace(/\s\s+/g, ''),
634-
controller: function mdDialogCtrl() {
635-
var isPrompt = this.$type == 'prompt';
636-
637-
if (isPrompt && this.initialValue) {
638-
this.result = this.initialValue;
639-
}
640-
641-
this.hide = function() {
642-
$mdDialog.hide(isPrompt ? this.result : true);
643-
};
644-
this.abort = function() {
645-
$mdDialog.cancel();
646-
};
647-
this.keypress = function($event) {
648-
if ($event.keyCode === $mdConstant.KEY_CODE.ENTER) {
649-
$mdDialog.hide(this.result);
650-
}
651-
};
652-
},
634+
controller: MdDialogController,
653635
controllerAs: 'dialog',
654636
bindToController: true,
655637
};
656638
}
657639

640+
/**
641+
* Controller for the md-dialog interim elements
642+
* @ngInject
643+
*/
644+
function MdDialogController($mdDialog, $mdConstant) {
645+
// For compatibility with AngularJS 1.6+, we should always use the $onInit hook in
646+
// interimElements. The $mdCompiler simulates the $onInit hook for all versions.
647+
this.$onInit = function() {
648+
var isPrompt = this.$type == 'prompt';
649+
650+
if (isPrompt && this.initialValue) {
651+
this.result = this.initialValue;
652+
}
653+
654+
this.hide = function() {
655+
$mdDialog.hide(isPrompt ? this.result : true);
656+
};
657+
this.abort = function() {
658+
$mdDialog.cancel();
659+
};
660+
this.keypress = function($event) {
661+
if ($event.keyCode === $mdConstant.KEY_CODE.ENTER) {
662+
$mdDialog.hide(this.result);
663+
}
664+
};
665+
};
666+
}
667+
658668
/* @ngInject */
659669
function dialogDefaultOptions($mdDialog, $mdAria, $mdUtil, $mdConstant, $animate, $document, $window, $rootElement,
660670
$log, $injector, $mdTheming, $interpolate, $mdInteraction) {

src/components/toast/toast.js

+28-18
Original file line numberDiff line numberDiff line change
@@ -321,24 +321,7 @@ function MdToastProvider($$interimElementProvider) {
321321
' </md-button>' +
322322
' </div>' +
323323
'</md-toast>',
324-
controller: /* @ngInject */ function mdToastCtrl($scope) {
325-
var self = this;
326-
327-
if (self.highlightAction) {
328-
$scope.highlightClasses = [
329-
'md-highlight',
330-
self.highlightClass
331-
]
332-
}
333-
334-
$scope.$watch(function() { return activeToastContent; }, function() {
335-
self.content = activeToastContent;
336-
});
337-
338-
this.resolve = function() {
339-
$mdToast.hide( ACTION_RESOLVE );
340-
};
341-
},
324+
controller: MdToastController,
342325
theme: $mdTheming.defaultTheme(),
343326
controllerAs: 'toast',
344327
bindToController: true
@@ -354,6 +337,33 @@ function MdToastProvider($$interimElementProvider) {
354337

355338
return $mdToast;
356339

340+
/**
341+
* Controller for the Toast interim elements.
342+
* @ngInject
343+
*/
344+
function MdToastController($mdToast, $scope) {
345+
// For compatibility with AngularJS 1.6+, we should always use the $onInit hook in
346+
// interimElements. The $mdCompiler simulates the $onInit hook for all versions.
347+
this.$onInit = function() {
348+
var self = this;
349+
350+
if (self.highlightAction) {
351+
$scope.highlightClasses = [
352+
'md-highlight',
353+
self.highlightClass
354+
]
355+
}
356+
357+
$scope.$watch(function() { return activeToastContent; }, function() {
358+
self.content = activeToastContent;
359+
});
360+
361+
this.resolve = function() {
362+
$mdToast.hide( ACTION_RESOLVE );
363+
};
364+
}
365+
}
366+
357367
/* @ngInject */
358368
function toastDefaultOptions($animate, $mdToast, $mdUtil, $mdMedia) {
359369
var SWIPE_EVENTS = '$md.swipeleft $md.swiperight $md.swipeup $md.swipedown';

src/core/services/compiler/compiler.js

+52-10
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,21 @@
66
*/
77
angular
88
.module('material.core')
9-
.service('$mdCompiler', MdCompilerService);
9+
.service('$mdCompiler', MdCompilerService)
10+
.provider('$$mdPreAssignBindings', PreAssignBindingsProvider);
11+
12+
/**
13+
* Provider that is used to report the preAssignBindingsEnabled state to the $mdCompiler service.
14+
*/
15+
function PreAssignBindingsProvider($compileProvider) {
16+
// To avoid breaking changes in Material, AngularJS Material will only respect the
17+
// preAssignBindingsEnabled state in AngularJS 1.6.X and higher.
18+
var respectPreAssignState = angular.version.major === 1 && angular.version.minor > 5;
19+
20+
this.$get = function() {
21+
return respectPreAssignState ? $compileProvider.preAssignBindingsEnabled() : true;
22+
};
23+
}
1024

1125
/**
1226
* @ngdoc service
@@ -81,7 +95,9 @@ angular
8195
* </hljs>
8296
*
8397
*/
84-
function MdCompilerService($q, $templateRequest, $injector, $compile, $controller) {
98+
function MdCompilerService($q, $templateRequest, $injector, $compile, $controller,
99+
$$mdPreAssignBindings) {
100+
85101
/** @private @const {!angular.$q} */
86102
this.$q = $q;
87103

@@ -96,6 +112,9 @@ function MdCompilerService($q, $templateRequest, $injector, $compile, $controlle
96112

97113
/** @private @const {!angular.$controller} */
98114
this.$controller = $controller;
115+
116+
/** @private @const {boolean} */
117+
this.preAssignBindingsEnabled = $$mdPreAssignBindings;
99118
}
100119

101120
/**
@@ -244,17 +263,12 @@ MdCompilerService.prototype._compileElement = function(locals, element, options)
244263
// Instantiate controller if the developer provided one.
245264
if (options.controller) {
246265

247-
var injectLocals = angular.extend(locals, {
266+
var injectLocals = angular.extend({}, locals, {
248267
$element: element
249268
});
250269

251-
var invokeCtrl = self.$controller(options.controller, injectLocals, true, options.controllerAs);
252-
253-
if (options.bindToController) {
254-
angular.extend(invokeCtrl.instance, locals);
255-
}
256-
257-
var ctrl = invokeCtrl();
270+
// Create the specified controller instance.
271+
var ctrl = self._createController(options, injectLocals, locals);
258272

259273
// Unique identifier for Angular Route ngView controllers.
260274
element.data('$ngControllerController', ctrl);
@@ -272,6 +286,34 @@ MdCompilerService.prototype._compileElement = function(locals, element, options)
272286

273287
};
274288

289+
/**
290+
* Creates and instantiates a new controller with the specified options.
291+
* @param {!Object} options Options that include the controller
292+
* @param {!Object} injectLocals Locals to to be provided in the controller DI.
293+
* @param {!Object} locals Locals to be injected to the controller.
294+
* @returns {!Object} Created controller instance.
295+
* @private
296+
*/
297+
MdCompilerService.prototype._createController = function(options, injectLocals, locals) {
298+
var invokeCtrl = this.$controller(options.controller, injectLocals, true, options.controllerAs);
299+
300+
if (this.preAssignBindingsEnabled && options.bindToController) {
301+
angular.extend(invokeCtrl.instance, locals);
302+
}
303+
304+
// Instantiate and initialize the specified controller.
305+
var ctrl = invokeCtrl();
306+
307+
if (!this.preAssignBindingsEnabled && options.bindToController) {
308+
angular.extend(invokeCtrl.instance, locals);
309+
}
310+
311+
// Call the $onInit hook if it's present on the controller.
312+
angular.isFunction(ctrl.$onInit) && ctrl.$onInit();
313+
314+
return ctrl;
315+
};
316+
275317
/**
276318
* Fetches an element removing it from the DOM and using it temporary for the compiler.
277319
* Elements which were fetched will be restored after use.

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

+71-18
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ describe('$mdCompiler service', function() {
1212
return compileData;
1313
}
1414

15+
function setPreAssignBindings(preAssignBindingsEnabled) {
16+
module(function($compileProvider) {
17+
$compileProvider.preAssignBindingsEnabled(preAssignBindingsEnabled);
18+
});
19+
}
20+
21+
1522
describe('setup', function() {
1623

1724
it('element should use templateUrl', inject(function($templateCache) {
@@ -159,25 +166,71 @@ describe('$mdCompiler service', function() {
159166
expect(scope.myControllerAs).toBe(data.element.controller());
160167
}));
161168

162-
it('should work with bindToController', inject(function($rootScope) {
163-
var called = false;
164-
var data = compile({
165-
template: 'hello',
166-
controller: function($scope) {
167-
expect(this.name).toBe('Bob');
168-
expect($scope.$apply).toBeTruthy(); // test DI working properly
169-
called = true;
170-
},
171-
controllerAs: 'ctrl',
172-
bindToController: true,
173-
locals: { name: 'Bob' }
174-
});
175-
var scope = $rootScope.$new();
176-
data.link(scope);
177-
expect(scope.ctrl.name).toBe('Bob');
178-
expect(called).toBe(true);
179-
}));
169+
});
170+
171+
});
172+
173+
describe('with preAssignBindings', function() {
174+
175+
function compileAndLink(options) {
176+
var data = compile(options);
177+
178+
inject(function($rootScope) {
179+
data.link($rootScope);
180+
});
181+
182+
return data;
183+
}
184+
185+
it('enabled should assign bindings at instantiation', function() {
186+
setPreAssignBindings(true);
187+
188+
var isInstantiated = false;
189+
190+
function TestController($scope) {
191+
isInstantiated = true;
192+
expect($scope.$apply).toBeTruthy();
193+
expect(this.name).toBe('Bob');
194+
}
195+
196+
compileAndLink({
197+
template: 'hello',
198+
controller: TestController,
199+
controllerAs: 'ctrl',
200+
bindToController: true,
201+
locals: { name: 'Bob' }
202+
});
203+
204+
expect(isInstantiated).toBe(true);
205+
});
206+
207+
it('disabled should assign bindings after constructor', function() {
208+
setPreAssignBindings(false);
209+
210+
var isInstantiated = false;
211+
212+
function TestController($scope) {
213+
isInstantiated = true;
214+
expect($scope.$apply).toBeTruthy();
215+
expect(this.name).toBeUndefined();
216+
}
217+
218+
TestController.prototype.$onInit = function() {
219+
expect(this.name).toBe('Bob');
220+
};
221+
222+
spyOn(TestController.prototype, '$onInit').and.callThrough();
223+
224+
compileAndLink({
225+
template: 'hello',
226+
controller: TestController,
227+
controllerAs: 'ctrl',
228+
bindToController: true,
229+
locals: { name: 'Bob' }
230+
});
180231

232+
expect(TestController.prototype.$onInit).toHaveBeenCalledTimes(1);
233+
expect(isInstantiated).toBe(true);
181234
});
182235

183236
});

0 commit comments

Comments
 (0)