From 205827276b4cedf191e29ec30120523790d0d6da Mon Sep 17 00:00:00 2001 From: Ricardo Mendes Date: Wed, 19 Jul 2017 17:41:41 +0100 Subject: [PATCH] Remove ControllerContentModelAliasDeprecation for deprecatingAlias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So, this is kind of a fun one. From what I can tell, `ControllerContentModelAliasDeprecation` was introduced, due to Controllers having a `content` property, which `model` was aliased to. This was done because `ObjectController` and `ArrayController` were proxying controllers. But, in Ember, routes set up the `model` property of its respective controller. What this meant is that if someone declared a `content` property in their controller, `model` would—seemingly unrelated—break, hence the `ControllerContentModelAliasDeprecation` mixin. Nowadays it is fine to override `content` because the source of truth was reverse, and `model` is now the primary property, being `content` the alias. It is, then, time to remove the old deprecation and instead deprecate the alias itself, so that in the future users can define a `content` property in their controllers if they so desire. Another API bites the dust. Here's to removing yet more code in the future, and make Ember sparkle-clean. --- features.json | 2 +- .../ember-runtime/lib/mixins/controller.js | 13 +++-- ...troller_content_model_alias_deprecation.js | 49 ------------------- .../tests/controllers/controller_test.js | 29 ++++++----- 4 files changed, 25 insertions(+), 68 deletions(-) delete mode 100644 packages/ember-runtime/lib/mixins/controller_content_model_alias_deprecation.js diff --git a/features.json b/features.json index 5c4ee8a2222..d9caf942b7a 100644 --- a/features.json +++ b/features.json @@ -35,10 +35,10 @@ "ember-views.render-double-modify": "2.0.0", "ember-routing.router-resource": "2.0.0", "ember-routing.top-level-render-helper": "2.11.0", + "ember-runtime.controller-content": "2.16.0", "ember-runtime.controller-proxy": "2.0.0", "ember-runtime.action-handler-_actions": "2.0.0", "ember-runtime.enumerable-contains": "2.7.0", - "ember-runtime.will-merge-mixin": "2.0.0", "ember-runtime.frozen-copy": "2.0.0", "ember-runtime.freezable-init": "2.0.0", "ember-string-utils.fmt": "2.0.0", diff --git a/packages/ember-runtime/lib/mixins/controller.js b/packages/ember-runtime/lib/mixins/controller.js index c5363b6d4ff..5ada9d2be34 100644 --- a/packages/ember-runtime/lib/mixins/controller.js +++ b/packages/ember-runtime/lib/mixins/controller.js @@ -1,6 +1,6 @@ import { Mixin, alias } from 'ember-metal'; +import { deprecatingAlias } from '../computed/computed_macros'; import ActionHandler from './action_handler'; -import ControllerContentModelAliasDeprecation from './controller_content_model_alias_deprecation'; /** @class ControllerMixin @@ -8,7 +8,7 @@ import ControllerContentModelAliasDeprecation from './controller_content_model_a @uses Ember.ActionHandler @private */ -export default Mixin.create(ActionHandler, ControllerContentModelAliasDeprecation, { +export default Mixin.create(ActionHandler, { /* ducktype as a controller */ isController: true, @@ -38,12 +38,15 @@ export default Mixin.create(ActionHandler, ControllerContentModelAliasDeprecatio @property model @public - */ + */ model: null, /** @private */ - content: alias('model') - + content: deprecatingAlias('model', { + id: 'ember-runtime.controller.content-alias', + until: '2.17.0', + url: 'https://emberjs.com/deprecations/v2.x/#toc_controller-content-alias' + }) }); diff --git a/packages/ember-runtime/lib/mixins/controller_content_model_alias_deprecation.js b/packages/ember-runtime/lib/mixins/controller_content_model_alias_deprecation.js deleted file mode 100644 index 4f59c8fa383..00000000000 --- a/packages/ember-runtime/lib/mixins/controller_content_model_alias_deprecation.js +++ /dev/null @@ -1,49 +0,0 @@ -import { Mixin } from 'ember-metal'; -import { deprecate } from 'ember-debug'; - -/* - The ControllerContentModelAliasDeprecation mixin is used to provide a useful - deprecation warning when specifying `content` directly on a `Ember.Controller` - (without also specifying `model`). - - Ember versions prior to 1.7 used `model` as an alias of `content`, but due to - much confusion this alias was reversed (so `content` is now an alias of `model). - - This change reduces many caveats with model/content, and also sets a - simple ground rule: Never set a controllers content, rather always set - its model and ember will do the right thing. - - Used internally by Ember in `Ember.Controller`. -*/ -export default Mixin.create({ - /** - @private - - Moves `content` to `model` at extend time if a `model` is not also specified. - - Note that this currently modifies the mixin themselves, which is technically - dubious but is practically of little consequence. This may change in the - future. - - @method willMergeMixin - @since 1.4.0 - */ - willMergeMixin(props) { - // Calling super is only OK here since we KNOW that - // there is another Mixin loaded first. - this._super(...arguments); - - let modelSpecified = !!props.model; - - if (props.content && !modelSpecified) { - props.model = props.content; - delete props['content']; - - deprecate( - 'Do not specify `content` on a Controller, use `model` instead.', - false, - { id: 'ember-runtime.will-merge-mixin', until: '3.0.0' } - ); - } - } -}); diff --git a/packages/ember-runtime/tests/controllers/controller_test.js b/packages/ember-runtime/tests/controllers/controller_test.js index 8ea64577180..75a26ef6d74 100644 --- a/packages/ember-runtime/tests/controllers/controller_test.js +++ b/packages/ember-runtime/tests/controllers/controller_test.js @@ -102,16 +102,18 @@ QUnit.module('Controller deprecations'); QUnit.module('Controller Content -> Model Alias'); -QUnit.test('`model` is aliased as `content`', function() { - expect(1); +QUnit.test('`content` is a deprecated alias of `model`', function() { + expect(2); let controller = Controller.extend({ model: 'foo-bar' }).create(); - equal(controller.get('content'), 'foo-bar', 'content is an alias of model'); + expectDeprecation(function () { + equal(controller.get('content'), 'foo-bar', 'content is an alias of model'); + }); }); -QUnit.test('`content` is moved to `model` when `model` is unset', function() { +QUnit.test('`content` is not moved to `model` when `model` is unset', function() { expect(2); let controller; @@ -121,18 +123,19 @@ QUnit.test('`content` is moved to `model` when `model` is unset', function() { }).create(); }); - equal(controller.get('model'), 'foo-bar', 'model is set properly'); - equal(controller.get('content'), 'foo-bar', 'content is set properly'); + notEqual(controller.get('model'), 'foo-bar', 'model is set properly'); + equal(controller.get('content'), 'foo-bar', 'content is not set properly'); }); -QUnit.test('specifying `content` (without `model` specified) results in deprecation', function() { - expect(1); +QUnit.test('specifying `content` (without `model` specified) does not result in deprecation', function() { + expect(2); + expectNoDeprecation(); - expectDeprecation(function() { - Controller.extend({ - content: 'foo-bar' - }).create(); - }, 'Do not specify `content` on a Controller, use `model` instead.'); + let controller = Controller.extend({ + content: 'foo-bar' + }).create(); + + equal(get(controller, 'content'), 'foo-bar'); }); QUnit.test('specifying `content` (with `model` specified) does not result in deprecation', function() {