From 579a3271698381c49803210c2efaa3b1f9e802bb Mon Sep 17 00:00:00 2001 From: Michael Prentice Date: Thu, 16 Jul 2020 19:05:34 -0400 Subject: [PATCH] refactor($mdCompilerProvider): remove deprecated $mdCompilerProvider.respectPreAssignBindingsEnabled() - remove use of `$controller`'s private and unsupported third argument - this means that locals would only be available in a controller's `$onInit()` - update tests BREAKING CHANGE: The deprecated `$mdCompilerProvider.respectPreAssignBindingsEnabled()` API has been removed. We no longer respect AngularJS's `$compileProvider.preAssignBindingsEnabled()` value as this API was removed in AngularJS `1.7.0`. If you had the recommended configuration for AngularJS 1.6.x: ```js $compileProvider.preAssignBindingsEnabled(false); $mdCompilerProvider.respectPreAssignBindingsEnabled(true); ``` Then you should remove both of these calls as they are now the defaults in AngularJS `1.7.0` and AngularJS Material `1.2.0`. If you had the recommended configuration for AngularJS 1.7+: ```js $mdCompilerProvider.respectPreAssignBindingsEnabled(true); ``` Then you should remove this call as it is now the default in AngularJS Material `1.2.0`. If you were using a backwards-compatible configuration for AngularJS 1.6+: ```js $mdCompilerProvider.respectPreAssignBindingsEnabled(false); ``` Then you will need to remove this call and may need to refactor your Controllers for AngularJS Material components like `$mdDialog`, `$mdPanel`, `$mdToast`, or `$mdBottomSheet`. For example: ```js $mdDialog.show({ locals: { myVar: true }, controller: MyController, bindToController: true } function MyController() { // No locals from Angular Material are available. e.g myVar is undefined. // You would need to move anything accessing locals in here to $onInit(). } MyController.prototype.$onInit = function() { // Bindings are now available in the $onInit lifecycle hook. } ``` --- src/core/services/compiler/compiler.js | 232 ++------------------ src/core/services/compiler/compiler.spec.js | 193 +++++----------- 2 files changed, 62 insertions(+), 363 deletions(-) diff --git a/src/core/services/compiler/compiler.js b/src/core/services/compiler/compiler.js index 0a54f95dc49..2c3a9429255 100644 --- a/src/core/services/compiler/compiler.js +++ b/src/core/services/compiler/compiler.js @@ -8,191 +8,8 @@ angular .module('material.core') .provider('$mdCompiler', MdCompilerProvider); -/** - * @ngdoc service - * @name $mdCompilerProvider - * @module material.core.compiler - * @description - * The `$mdCompiler` is able to respect the AngularJS `$compileProvider.preAssignBindingsEnabled` - * state when using AngularJS versions greater than or equal to 1.5.10 and less than 1.7.0. - * See the [AngularJS documentation for `$compileProvider.preAssignBindingsEnabled` - * ](https://code.angularjs.org/1.6.10/docs/api/ng/provider/$compileProvider#preAssignBindingsEnabled) - * for more information. - * - * To enable/disable whether the controllers of dynamic AngularJS Material components - * (i.e. dialog, panel, toast, bottomsheet) respect the AngularJS - * `$compileProvider.preAssignBindingsEnabled` flag, call the AngularJS Material method: - * `$mdCompilerProvider.respectPreAssignBindingsEnabled(boolean)`. - * - * This AngularJS Material *flag* doesn't affect directives/components created via regular - * AngularJS methods. These constitute the majority of AngularJS Material and user-created - * components. Only dynamic construction of elements such as Dialogs, Panels, Toasts, BottomSheets, - * etc. may be affected. Invoking `$mdCompilerProvider.respectPreAssignBindingsEnabled(true)` - * will effect **bindings** in controllers created by AngularJS Material's services like - * `$mdDialog`, `$mdPanel`, `$mdToast`, or `$mdBottomSheet`. - * - * See - * - * $mdCompilerProvider.respectPreAssignBindingsEnabled - * - * for the details of how the different versions and settings of AngularJS affect this behavior. - * - * @usage - * - * Respect the AngularJS Compiler Setting - * - * - * app.config(function($mdCompilerProvider) { - * $mdCompilerProvider.respectPreAssignBindingsEnabled(true); - * }); - * - * - * @example - * Using the default (backwards compatible) values for AngularJS 1.6 - * - AngularJS' `$compileProvider.preAssignBindingsEnabled(false)` - * - AngularJS Material's `$mdCompilerProvider.respectPreAssignBindingsEnabled(false)` - *

- * - * - * $mdDialog.show({ - * locals: { - * myVar: true - * }, - * controller: MyController, - * bindToController: true - * } - * - * function MyController() { - * // Locals from Angular Material are available. e.g myVar is true. - * } - * - * MyController.prototype.$onInit = function() { - * // Bindings are also available in the $onInit lifecycle hook. - * } - * - * - * Recommended Settings for AngularJS 1.6 - * - AngularJS' `$compileProvider.preAssignBindingsEnabled(false)` - * - AngularJS Material's `$mdCompilerProvider.respectPreAssignBindingsEnabled(true)` - *

- * - * - * $mdDialog.show({ - * locals: { - * myVar: true - * }, - * controller: MyController, - * bindToController: true - * } - * - * function MyController() { - * // No locals from Angular Material are available. e.g myVar is undefined. - * } - * - * MyController.prototype.$onInit = function() { - * // Bindings are now available in the $onInit lifecycle hook. - * } - * - * - */ MdCompilerProvider.$inject = ['$compileProvider']; -function MdCompilerProvider($compileProvider) { - - var provider = this; - - /** - * @ngdoc method - * @name $mdCompilerProvider#respectPreAssignBindingsEnabled - * - * @param {boolean=} respected update the `respectPreAssignBindingsEnabled` state if provided, - * otherwise just return the current Material `respectPreAssignBindingsEnabled` state. - * @returns {boolean|MdCompilerProvider} current value, if used as a getter, or itself (chaining) - * if used as a setter. - * - * @description - * Call this method to enable/disable whether Material-specific (dialog/panel/toast/bottomsheet) - * controllers respect the AngularJS `$compileProvider.preAssignBindingsEnabled` flag. Note that - * this doesn't affect directives/components created via regular AngularJS methods which - * constitute most Material and user-created components. - * - * If disabled (`false`), the compiler assigns the value of each of the bindings to the - * properties of the controller object before the constructor of this object is called. - * The ability to disable this settings is **deprecated** and will be removed in - * AngularJS Material 1.2.0. - * - * If enabled (`true`) the behavior depends on the AngularJS version used: - * - * - `<1.5.10` - * - Bindings are pre-assigned. - * - `>=1.5.10 <1.7` - * - Respects whatever `$compileProvider.preAssignBindingsEnabled()` reports. If the - * `preAssignBindingsEnabled` flag wasn't set manually, it defaults to pre-assigning bindings - * with AngularJS `1.5` and to calling the constructor first with AngularJS `1.6`. - * - `>=1.7` - * - The compiler calls the constructor first before assigning bindings and - * `$compileProvider.preAssignBindingsEnabled()` no longer exists. - * - * Defaults - * - The default value is `false` in AngularJS 1.6 and earlier. - * - It is planned to fix this value to `true` and not allow the `false` value in - * AngularJS Material 1.2.0. - * - * It is recommended to set this flag to `true` when using AngularJS Material 1.1.x with - * AngularJS versions >= 1.5.10. The only reason it's not set that way by default is backwards - * compatibility. - * - * By not setting the flag to `true` when AngularJS' `$compileProvider.preAssignBindingsEnabled()` - * is set to `false` (i.e. default behavior in AngularJS 1.6 or newer), unit testing of - * Material Dialog/Panel/Toast/BottomSheet controllers using the `$controller` helper - * is problematic as it always follows AngularJS' `$compileProvider.preAssignBindingsEnabled()` - * value. - */ - var respectPreAssignBindingsEnabled = false; - this.respectPreAssignBindingsEnabled = function(respected) { - if (angular.isDefined(respected)) { - respectPreAssignBindingsEnabled = respected; - return this; - } - - return respectPreAssignBindingsEnabled; - }; - - /** - * @private - * @description - * This function returns `true` if AngularJS Material-specific (dialog/panel/toast/bottomsheet) - * controllers have bindings pre-assigned in controller constructors and `false` otherwise. - * - * Note that this doesn't affect directives/components created via regular AngularJS methods - * which constitute most Material and user-created components; their behavior can be checked via - * `$compileProvider.preAssignBindingsEnabled()` in AngularJS `>=1.5.10 <1.7.0`. - * - * @returns {*} current preAssignBindingsEnabled state - */ - function getPreAssignBindingsEnabled() { - if (!respectPreAssignBindingsEnabled) { - // respectPreAssignBindingsEnabled === false - // We're ignoring the AngularJS `$compileProvider.preAssignBindingsEnabled()` value in this case. - return true; - } - - // respectPreAssignBindingsEnabled === true - - // This check is needed because $compileProvider.preAssignBindingsEnabled does not exist prior - // to AngularJS 1.5.10, is deprecated in AngularJS 1.6.x, and removed in AngularJS 1.7.x. - if (typeof $compileProvider.preAssignBindingsEnabled === 'function') { - return $compileProvider.preAssignBindingsEnabled(); - } - - // Flag respected but not present => apply logic based on AngularJS version used. - if (angular.version.major === 1 && angular.version.minor < 6) { - // AngularJS <1.5.10 - return true; - } - - // AngularJS >=1.7.0 - return false; - } +function MdCompilerProvider() { this.$get = ["$q", "$templateRequest", "$injector", "$compile", "$controller", function($q, $templateRequest, $injector, $compile, $controller) { @@ -205,10 +22,10 @@ function MdCompilerProvider($compileProvider) { * @module material.core.compiler * @description * The $mdCompiler service is an abstraction of AngularJS's compiler, that allows developers - * to easily compile an element with options like in a Directive Definition Object. + * to compile an element with options like in a Directive Definition Object. * * > The compiler powers a lot of components inside of AngularJS Material. - * > Like the `$mdPanel` or `$mdDialog`. + * > Like the `$mdPanel` or `$mdDialog` services. * * @usage * @@ -243,11 +60,11 @@ function MdCompilerProvider($compileProvider) { * }); * * - * > Content Element is a significant performance improvement when the developer already knows that the - * > compiled element will be always the same and the scope will not change either. + * > Content Element is a significant performance improvement when the developer already knows + * > that the compiled element will be always the same and the scope will not change either. * - * The `contentElement` option also supports DOM elements which will be temporary removed and restored - * at its old position. + * The `contentElement` option also supports DOM elements which will be temporary removed and + * restored at its old position. * * * var domElement = document.querySelector('#myElement'); @@ -357,8 +174,8 @@ function MdCompilerProvider($compileProvider) { }; /** - * Instead of compiling any template, the compiler just fetches an existing HTML element from the DOM and - * provides a restore function to put the element back it old DOM position. + * Instead of compiling any template, the compiler just fetches an existing HTML element from the + * DOM and provides a restore function to put the element back it old DOM position. * @param {!Object} options Options to be used for the compiler. */ MdCompilerService.prototype._prepareContentElement = function(options) { @@ -487,33 +304,10 @@ function MdCompilerProvider($compileProvider) { * @returns {!Object} Created controller instance. */ MdCompilerService.prototype._createController = function(options, injectLocals, locals) { - var ctrl; - var preAssignBindingsEnabled = getPreAssignBindingsEnabled(); - // The third argument to $controller is considered private and undocumented: - // https://github.com/angular/angular.js/blob/v1.6.10/src/ng/controller.js#L102-L109. - // TODO remove the use of this third argument in AngularJS Material 1.2.0. - // Passing `true` as the third argument causes `$controller` to return a function that - // gets the controller instance instead of returning the instance directly. When the - // controller is defined as a function, `invokeCtrl.instance` is the *same instance* as - // `invokeCtrl()`. However, when the controller is an ES6 class, `invokeCtrl.instance` is a - // *different instance* from `invokeCtrl()`. - if (preAssignBindingsEnabled) { - var invokeCtrl = this.$controller(options.controller, injectLocals, true); - - if (options.bindToController) { - angular.extend(invokeCtrl.instance, locals); - } - - // Use the private API callback to instantiate and initialize the specified controller. - ctrl = invokeCtrl(); - } else { - // If we don't need to pre-assign bindings, avoid using the private API third argument and - // related callback. - ctrl = this.$controller(options.controller, injectLocals); + var ctrl = this.$controller(options.controller, injectLocals); - if (options.bindToController) { - angular.extend(ctrl, locals); - } + if (options.bindToController) { + angular.extend(ctrl, locals); } if (options.controllerAs) { @@ -534,7 +328,7 @@ function MdCompilerProvider($compileProvider) { */ MdCompilerService.prototype._fetchContentElement = function(options) { var contentEl = options.contentElement; - var restoreFn = null; + var restoreFn; if (angular.isString(contentEl)) { contentEl = document.querySelector(contentEl); diff --git a/src/core/services/compiler/compiler.spec.js b/src/core/services/compiler/compiler.spec.js index 88b5234dd05..a9641aa3845 100644 --- a/src/core/services/compiler/compiler.spec.js +++ b/src/core/services/compiler/compiler.spec.js @@ -2,7 +2,7 @@ describe('$mdCompiler service', function() { beforeEach(module('material.core')); function compile(options) { - var compileData; + var compileData = null; inject(function($mdCompiler, $rootScope) { $mdCompiler.compile(options).then(function(data) { compileData = data; @@ -12,7 +12,6 @@ describe('$mdCompiler service', function() { return compileData; } - describe('setup', function() { it('element should use templateUrl', inject(function($templateCache) { @@ -159,155 +158,71 @@ describe('$mdCompiler service', function() { data.link(scope); expect(scope.myControllerAs).toBe(data.element.controller()); })); - }); - }); - var bindingStatesToTest; - if (angular.version.major === 1 && angular.version.minor >= 6) { - bindingStatesToTest = [ - {respectPreAssignBindingsEnabled: true}, - {respectPreAssignBindingsEnabled: false}, - // TODO change `equivalentTo` to `true` in Material 1.2.0. - {respectPreAssignBindingsEnabled: '"default"', equivalentTo: false} - ]; - } else if (angular.version.major === 1 && angular.version.minor < 6) { - bindingStatesToTest = [ - {respectPreAssignBindingsEnabled: false} - ]; - } - - bindingStatesToTest.forEach(function(options) { - var realRespectPreAssignBindingsEnabled = options.respectPreAssignBindingsEnabled; - var respectPreAssignBindingsEnabled = angular.isDefined(options.equivalentTo) ? - options.equivalentTo : - realRespectPreAssignBindingsEnabled; - - describe('with respectPreAssignBindingsEnabled set to ' + realRespectPreAssignBindingsEnabled, function() { - var preAssignBindingsEnabledInAngularJS = angular.version.minor < 6; - - beforeEach(function() { - module(function($mdCompilerProvider) { - // Don't set the value so that the default state can be tested. - if (typeof realRespectPreAssignBindingsEnabled === 'boolean') { - $mdCompilerProvider.respectPreAssignBindingsEnabled(realRespectPreAssignBindingsEnabled); - } - }); - }); - - function compileAndLink(options) { - var compileData; + function compileAndLink(options) { + var compileData = null; - inject(function($mdCompiler, $rootScope) { - $mdCompiler.compile(options).then(function(data) { - data.link($rootScope); - compileData = data; - }); - - $rootScope.$apply(); - }); - - return compileData; - } - - it('should call $onInit even if bindToController is set to false', function() { - var isInstantiated = false; - - function TestController($scope, name) { - isInstantiated = true; - expect($scope.$apply).toBeTruthy(); - expect(name).toBe('Bob'); - } - - TestController.prototype.$onInit = jasmine.createSpy('$onInit'); - - compileAndLink({ - template: 'hello', - controller: TestController, - bindToController: false, - locals: {name: 'Bob'} - }); - - expect(TestController.prototype.$onInit).toHaveBeenCalledTimes(1); - expect(isInstantiated).toBe(true); + inject(function($mdCompiler, $rootScope) { + $mdCompiler.compile(options).then(function(data) { + data.link($rootScope); + compileData = data; }); - // Bindings are not pre-assigned if we respect the AngularJS config and pre-assigning - // them is disabled there. This logic will change in AngularJS Material 1.2.0. - if (respectPreAssignBindingsEnabled && !preAssignBindingsEnabledInAngularJS) { - it('disabled should assign bindings after constructor', function() { - 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'); - }; + $rootScope.$apply(); + }); - spyOn(TestController.prototype, '$onInit').and.callThrough(); + return compileData; + } - compileAndLink({ - template: 'hello', - controller: TestController, - controllerAs: 'ctrl', - bindToController: true, - locals: {name: 'Bob'} - }); + it('should call $onInit even if bindToController is set to false', function() { + var isInstantiated = false; - expect(TestController.prototype.$onInit).toHaveBeenCalledTimes(1); - expect(isInstantiated).toBe(true); - }); - } else { - it('enabled should assign bindings at instantiation', function() { - var isInstantiated = false; - - function TestController($scope) { - isInstantiated = true; - expect($scope.$apply).toBeTruthy(); - expect(this.name).toBe('Bob'); - } + function TestController($scope, name) { + isInstantiated = true; + expect($scope.$apply).toBeTruthy(); + expect(name).toBe('Bob'); + } - compileAndLink({ - template: 'hello', - controller: TestController, - controllerAs: 'ctrl', - bindToController: true, - locals: {name: 'Bob'} - }); + TestController.prototype.$onInit = jasmine.createSpy('$onInit'); - expect(isInstantiated).toBe(true); - }); + compileAndLink({ + template: 'hello', + controller: TestController, + bindToController: false, + locals: {name: 'Bob'} + }); - it('enabled should assign bindings at instantiation even if $onInit defined', function() { - var isInstantiated = false; + expect(TestController.prototype.$onInit).toHaveBeenCalledTimes(1); + expect(isInstantiated).toBe(true); + }); - function TestController($scope) { - isInstantiated = true; - expect($scope.$apply).toBeTruthy(); - expect(this.name).toBe('Bob'); - } + it('should assign bindings after constructor', function() { + var isInstantiated = false; - TestController.prototype.$onInit = jasmine.createSpy('$onInit'); + function TestController($scope) { + isInstantiated = true; + expect($scope.$apply).toBeTruthy(); + expect(this.name).toBeUndefined(); + } - compileAndLink({ - template: 'hello', - controller: TestController, - controllerAs: 'ctrl', - bindToController: true, - locals: {name: 'Bob'} - }, true); + TestController.prototype.$onInit = function() { + expect(this.name).toBe('Bob'); + }; - expect(TestController.prototype.$onInit).toHaveBeenCalledTimes(1); - expect(isInstantiated).toBe(true); - }); - } + 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); }); describe('with contentElement', function() { @@ -436,21 +351,11 @@ describe('$mdCompiler service', function() { }); - describe('with respectPreAssignBindingsEnabled and not preAssignBindingsEnabled', function() { + describe('with ES6 classes', function() { var $mdCompiler, pageScope, $rootScope; beforeEach(module('material.core')); - beforeEach(module(function($mdCompilerProvider, $compileProvider) { - $mdCompilerProvider.respectPreAssignBindingsEnabled(true); - - // preAssignBindingsEnabled is removed in Angular 1.7, so we only explicitly turn it - // on if the option exists. - if ($compileProvider.hasOwnProperty('preAssignBindingsEnabled')) { - $compileProvider.preAssignBindingsEnabled(false); - } - })); - beforeEach(inject(function($injector) { $mdCompiler = $injector.get('$mdCompiler'); $rootScope = $injector.get('$rootScope');