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

Commit

Permalink
fix(compiler): respect preAssignBindingsEnabled state
Browse files Browse the repository at this point in the history
* 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 #10016
  • Loading branch information
devversion committed Mar 10, 2017
1 parent 0c4e895 commit 203c380
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 68 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"jquery": "^3.0.0",
"jshint": "^2.9.2",
"jshint-summary": "^0.4.0",
"karma": "^1.0.0",
"karma": "1.4.1",
"karma-chrome-launcher": "^2.0.0",
"karma-firefox-launcher": "^1.0.0",
"karma-jasmine": "^1.0.2",
Expand All @@ -84,4 +84,4 @@
"merge-base": "git merge-base $(npm run -s current-branch) origin/master",
"squash": "git rebase -i $(npm run -s merge-base)"
}
}
}
50 changes: 30 additions & 20 deletions src/components/dialog/dialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ function MdDialogProvider($$interimElementProvider) {
});

/* @ngInject */
function advancedDialogOptions($mdDialog, $mdConstant) {
function advancedDialogOptions() {
return {
template: [
'<md-dialog md-theme="{{ dialog.theme || dialog.defaultTheme }}" aria-label="{{ dialog.ariaLabel }}" ng-class="dialog.css">',
Expand All @@ -631,30 +631,40 @@ function MdDialogProvider($$interimElementProvider) {
' </md-dialog-actions>',
'</md-dialog>'
].join('').replace(/\s\s+/g, ''),
controller: function mdDialogCtrl() {
var isPrompt = this.$type == 'prompt';

if (isPrompt && this.initialValue) {
this.result = this.initialValue;
}

this.hide = function() {
$mdDialog.hide(isPrompt ? this.result : true);
};
this.abort = function() {
$mdDialog.cancel();
};
this.keypress = function($event) {
if ($event.keyCode === $mdConstant.KEY_CODE.ENTER) {
$mdDialog.hide(this.result);
}
};
},
controller: MdDialogController,
controllerAs: 'dialog',
bindToController: true,
};
}

/**
* Controller for the md-dialog interim elements
* @ngInject
*/
function MdDialogController($mdDialog, $mdConstant) {
// For compatibility with AngularJS 1.6+, we should always use the $onInit hook in
// interimElements. The $mdCompiler simulates the $onInit hook for all versions.
this.$onInit = function() {
var isPrompt = this.$type == 'prompt';

if (isPrompt && this.initialValue) {
this.result = this.initialValue;
}

this.hide = function() {
$mdDialog.hide(isPrompt ? this.result : true);
};
this.abort = function() {
$mdDialog.cancel();
};
this.keypress = function($event) {
if ($event.keyCode === $mdConstant.KEY_CODE.ENTER) {
$mdDialog.hide(this.result);
}
};
};
}

/* @ngInject */
function dialogDefaultOptions($mdDialog, $mdAria, $mdUtil, $mdConstant, $animate, $document, $window, $rootElement,
$log, $injector, $mdTheming, $interpolate, $mdInteraction) {
Expand Down
46 changes: 28 additions & 18 deletions src/components/toast/toast.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,24 +321,7 @@ function MdToastProvider($$interimElementProvider) {
' </md-button>' +
' </div>' +
'</md-toast>',
controller: /* @ngInject */ function mdToastCtrl($scope) {
var self = this;

if (self.highlightAction) {
$scope.highlightClasses = [
'md-highlight',
self.highlightClass
]
}

$scope.$watch(function() { return activeToastContent; }, function() {
self.content = activeToastContent;
});

this.resolve = function() {
$mdToast.hide( ACTION_RESOLVE );
};
},
controller: MdToastController,
theme: $mdTheming.defaultTheme(),
controllerAs: 'toast',
bindToController: true
Expand All @@ -354,6 +337,33 @@ function MdToastProvider($$interimElementProvider) {

return $mdToast;

/**
* Controller for the Toast interim elements.
* @ngInject
*/
function MdToastController($mdToast, $scope) {
// For compatibility with AngularJS 1.6+, we should always use the $onInit hook in
// interimElements. The $mdCompiler simulates the $onInit hook for all versions.
this.$onInit = function() {
var self = this;

if (self.highlightAction) {
$scope.highlightClasses = [
'md-highlight',
self.highlightClass
]
}

$scope.$watch(function() { return activeToastContent; }, function() {
self.content = activeToastContent;
});

this.resolve = function() {
$mdToast.hide( ACTION_RESOLVE );
};
}
}

/* @ngInject */
function toastDefaultOptions($animate, $mdToast, $mdUtil, $mdMedia) {
var SWIPE_EVENTS = '$md.swipeleft $md.swiperight $md.swipeup $md.swipedown';
Expand Down
62 changes: 52 additions & 10 deletions src/core/services/compiler/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,21 @@
*/
angular
.module('material.core')
.service('$mdCompiler', MdCompilerService);
.service('$mdCompiler', MdCompilerService)
.provider('$$mdPreAssignBindings', PreAssignBindingsProvider);

/**
* Provider that is used to report the preAssignBindingsEnabled state to the $mdCompiler service.
*/
function PreAssignBindingsProvider($compileProvider) {
// To avoid breaking changes in Material, AngularJS Material will only respect the
// preAssignBindingsEnabled state in AngularJS 1.6.X and higher.
var respectPreAssignState = angular.version.major === 1 && angular.version.minor > 5;

this.$get = function() {
return respectPreAssignState ? $compileProvider.preAssignBindingsEnabled() : true;
};
}

/**
* @ngdoc service
Expand Down Expand Up @@ -81,7 +95,9 @@ angular
* </hljs>
*
*/
function MdCompilerService($q, $templateRequest, $injector, $compile, $controller) {
function MdCompilerService($q, $templateRequest, $injector, $compile, $controller,
$$mdPreAssignBindings) {

/** @private @const {!angular.$q} */
this.$q = $q;

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

/** @private @const {!angular.$controller} */
this.$controller = $controller;

/** @private @const {boolean} */
this.preAssignBindingsEnabled = $$mdPreAssignBindings;
}

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

var injectLocals = angular.extend(locals, {
var injectLocals = angular.extend({}, locals, {
$element: element
});

var invokeCtrl = self.$controller(options.controller, injectLocals, true, options.controllerAs);

if (options.bindToController) {
angular.extend(invokeCtrl.instance, locals);
}

var ctrl = invokeCtrl();
// Create the specified controller instance.
var ctrl = self._createController(options, injectLocals, locals);

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

};

/**
* Creates and instantiates a new controller with the specified options.
* @param {!Object} options Options that include the controller
* @param {!Object} injectLocals Locals to to be provided in the controller DI.
* @param {!Object} locals Locals to be injected to the controller.
* @returns {!Object} Created controller instance.
* @private
*/
MdCompilerService.prototype._createController = function(options, injectLocals, locals) {
var invokeCtrl = this.$controller(options.controller, injectLocals, true, options.controllerAs);

if (this.preAssignBindingsEnabled && options.bindToController) {
angular.extend(invokeCtrl.instance, locals);
}

// Instantiate and initialize the specified controller.
var ctrl = invokeCtrl();

if (!this.preAssignBindingsEnabled && options.bindToController) {
angular.extend(invokeCtrl.instance, locals);
}

// Call the $onInit hook if it's present on the controller.
angular.isFunction(ctrl.$onInit) && ctrl.$onInit();

return ctrl;
};

/**
* Fetches an element removing it from the DOM and using it temporary for the compiler.
* Elements which were fetched will be restored after use.
Expand Down
89 changes: 71 additions & 18 deletions src/core/services/compiler/compiler.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ describe('$mdCompiler service', function() {
return compileData;
}

function setPreAssignBindings(preAssignBindingsEnabled) {
module(function($compileProvider) {
$compileProvider.preAssignBindingsEnabled(preAssignBindingsEnabled);
});
}


describe('setup', function() {

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

it('should work with bindToController', inject(function($rootScope) {
var called = false;
var data = compile({
template: 'hello',
controller: function($scope) {
expect(this.name).toBe('Bob');
expect($scope.$apply).toBeTruthy(); // test DI working properly
called = true;
},
controllerAs: 'ctrl',
bindToController: true,
locals: { name: 'Bob' }
});
var scope = $rootScope.$new();
data.link(scope);
expect(scope.ctrl.name).toBe('Bob');
expect(called).toBe(true);
}));
});

});

describe('with preAssignBindings', function() {

function compileAndLink(options) {
var data = compile(options);

inject(function($rootScope) {
data.link($rootScope);
});

return data;
}

it('enabled should assign bindings at instantiation', function() {
setPreAssignBindings(true);

var isInstantiated = false;

function TestController($scope) {
isInstantiated = true;
expect($scope.$apply).toBeTruthy();
expect(this.name).toBe('Bob');
}

compileAndLink({
template: 'hello',
controller: TestController,
controllerAs: 'ctrl',
bindToController: true,
locals: { name: 'Bob' }
});

expect(isInstantiated).toBe(true);
});

it('disabled should assign bindings after constructor', function() {
setPreAssignBindings(false);

var isInstantiated = false;

function TestController($scope) {
isInstantiated = true;
expect($scope.$apply).toBeTruthy();
expect(this.name).toBeUndefined();
}

TestController.prototype.$onInit = function() {
expect(this.name).toBe('Bob');
};

spyOn(TestController.prototype, '$onInit').and.callThrough();

compileAndLink({
template: 'hello',
controller: TestController,
controllerAs: 'ctrl',
bindToController: true,
locals: { name: 'Bob' }
});

expect(TestController.prototype.$onInit).toHaveBeenCalledTimes(1);
expect(isInstantiated).toBe(true);
});

});
Expand Down

0 comments on commit 203c380

Please sign in to comment.