Skip to content

Commit c346893

Browse files
devversionmgol
authored andcommitted
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 6662bf2 commit c346893

File tree

5 files changed

+187
-67
lines changed

5 files changed

+187
-67
lines changed

package.json

Lines changed: 2 additions & 2 deletions
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

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ function MdDialogProvider($$interimElementProvider) {
603603
});
604604

605605
/* @ngInject */
606-
function advancedDialogOptions($mdDialog, $mdConstant) {
606+
function advancedDialogOptions() {
607607
return {
608608
template: [
609609
'<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

Lines changed: 28 additions & 18 deletions
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

Lines changed: 53 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,22 @@
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 that the preAssignBindings state causes issues when upgrading from AngularJS 1.5 to
17+
// AngularJS 1.6, AngularJS Material will only respect the preAssignBindingsEnabled state in 1.6
18+
var respectPreAssignState = angular.version.major === 1 && angular.version.minor === 6;
19+
var fallbackValue = !!$compileProvider.preAssignBindingsEnabled;
20+
21+
this.$get = function() {
22+
return respectPreAssignState ? $compileProvider.preAssignBindingsEnabled() : fallbackValue;
23+
};
24+
}
1025

1126
/**
1227
* @ngdoc service
@@ -81,7 +96,9 @@ angular
8196
* </hljs>
8297
*
8398
*/
84-
function MdCompilerService($q, $templateRequest, $injector, $compile, $controller) {
99+
function MdCompilerService($q, $templateRequest, $injector, $compile, $controller,
100+
$$mdPreAssignBindings) {
101+
85102
/** @private @const {!angular.$q} */
86103
this.$q = $q;
87104

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

97114
/** @private @const {!angular.$controller} */
98115
this.$controller = $controller;
116+
117+
/** @private @const {boolean} */
118+
this.preAssignBindingsEnabled = $$mdPreAssignBindings;
99119
}
100120

101121
/**
@@ -244,17 +264,12 @@ MdCompilerService.prototype._compileElement = function(locals, element, options)
244264
// Instantiate controller if the developer provided one.
245265
if (options.controller) {
246266

247-
var injectLocals = angular.extend(locals, {
267+
var injectLocals = angular.extend({}, locals, {
248268
$element: element
249269
});
250270

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();
271+
// Create the specified controller instance.
272+
var ctrl = self._createController(options, injectLocals, locals);
258273

259274
// Unique identifier for AngularJS Route ngView controllers.
260275
element.data('$ngControllerController', ctrl);
@@ -272,6 +287,34 @@ MdCompilerService.prototype._compileElement = function(locals, element, options)
272287

273288
};
274289

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

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

Lines changed: 74 additions & 17 deletions
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,75 @@ 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' }
169+
});
170+
171+
});
172+
173+
describe('with preAssignBindings', function() {
174+
175+
function compileAndLink(options, preAssignBindings) {
176+
var compileData;
177+
178+
inject(function($mdCompiler, $rootScope) {
179+
$mdCompiler.preAssignBindingsEnabled = preAssignBindings;
180+
$mdCompiler.compile(options).then(function(data) {
181+
data.link($rootScope);
182+
compileData = data;
174183
});
175-
var scope = $rootScope.$new();
176-
data.link(scope);
177-
expect(scope.ctrl.name).toBe('Bob');
178-
expect(called).toBe(true);
179-
}));
180184

185+
$rootScope.$apply();
186+
});
187+
188+
return compileData;
189+
}
190+
191+
it('enabled should assign bindings at instantiation', function() {
192+
var isInstantiated = false;
193+
194+
function TestController($scope) {
195+
isInstantiated = true;
196+
expect($scope.$apply).toBeTruthy();
197+
expect(this.name).toBe('Bob');
198+
}
199+
200+
compileAndLink({
201+
template: 'hello',
202+
controller: TestController,
203+
controllerAs: 'ctrl',
204+
bindToController: true,
205+
locals: { name: 'Bob' }
206+
}, true);
207+
208+
expect(isInstantiated).toBe(true);
209+
});
210+
211+
it('disabled should assign bindings after constructor', function() {
212+
setPreAssignBindings(false);
213+
214+
var isInstantiated = false;
215+
216+
function TestController($scope) {
217+
isInstantiated = true;
218+
expect($scope.$apply).toBeTruthy();
219+
expect(this.name).toBeUndefined();
220+
}
221+
222+
TestController.prototype.$onInit = function() {
223+
expect(this.name).toBe('Bob');
224+
};
225+
226+
spyOn(TestController.prototype, '$onInit').and.callThrough();
227+
228+
compileAndLink({
229+
template: 'hello',
230+
controller: TestController,
231+
controllerAs: 'ctrl',
232+
bindToController: true,
233+
locals: { name: 'Bob' }
234+
}, false);
235+
236+
expect(TestController.prototype.$onInit).toHaveBeenCalledTimes(1);
237+
expect(isInstantiated).toBe(true);
181238
});
182239

183240
});

0 commit comments

Comments
 (0)