From dcc0dc3d8a3b9546c6da4cc8b2cb6086a3c98a00 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Sun, 23 Aug 2015 20:51:06 -0400 Subject: [PATCH] [BUGFIX release] Ensure `_actions` specified to extend works. The initial deprecation of `_actions` and moving it to `actions` was a bit too naive. The `deprecateProperty` helper was used, which meant that when `_actions` was set, it would call `set(this, 'actions', newValue)`. This meant that passing `_actions` to `Ember.Route.extend` (or `.reopen`) resulted in the `_actions` completely clobbering `actions` (and loosing any previously defined actions there). The fix was to: * Replace the generic `deprecateProperty` usage with a custom `Object.defineProperty` call, that defines a setter with an assertion instead of clobbering. * Implementing a `willMergeMixin` that mutates the `props` at extend time (to ensure that `_actions` is moved to `actions` with a deprecation). --- .../ember-routing/tests/system/route_test.js | 33 +++++++++++++++++ .../lib/mixins/action_handler.js | 36 +++++++++++++++++-- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/packages/ember-routing/tests/system/route_test.js b/packages/ember-routing/tests/system/route_test.js index d94b4494a23..7735178dc55 100644 --- a/packages/ember-routing/tests/system/route_test.js +++ b/packages/ember-routing/tests/system/route_test.js @@ -213,6 +213,39 @@ QUnit.test('can access `actions` hash via `_actions` [DEPRECATED]', function() { }, 'Usage of `_actions` is deprecated, use `actions` instead.'); }); +QUnit.test('actions in both `_actions` and `actions` results in an assertion', function() { + expectAssertion(function() { + EmberRoute.extend({ + _actions: { }, + actions: { } + }).create(); + }, 'Specifying `_actions` and `actions` in the same mixin is not supported.'); +}); + +QUnit.test('actions added via `_actions` can be used [DEPRECATED]', function() { + expect(3); + + let route; + expectDeprecation(function() { + route = EmberRoute.extend({ + _actions: { + bar: function() { + ok(true, 'called bar action'); + } + } + }, { + actions: { + foo: function() { + ok(true, 'called foo action'); + } + } + }).create(); + }, 'Specifying actions in `_actions` is deprecated, please use `actions` instead.'); + + route.send('foo'); + route.send('bar'); +}); + QUnit.module('Ember.Route serialize', { setup: setup, teardown: teardown diff --git a/packages/ember-runtime/lib/mixins/action_handler.js b/packages/ember-runtime/lib/mixins/action_handler.js index 4dbe931263f..901d4318893 100644 --- a/packages/ember-runtime/lib/mixins/action_handler.js +++ b/packages/ember-runtime/lib/mixins/action_handler.js @@ -5,7 +5,6 @@ import Ember from 'ember-metal/core'; import { Mixin } from 'ember-metal/mixin'; import { get } from 'ember-metal/property_get'; -import { deprecateProperty } from 'ember-metal/deprecate_property'; /** `Ember.ActionHandler` is available on some familiar classes including @@ -185,13 +184,44 @@ var ActionHandler = Mixin.create({ ') does not have a `send` method', typeof target.send === 'function'); target.send(...arguments); } + }, + + willMergeMixin(props) { + Ember.assert('Specifying `_actions` and `actions` in the same mixin is not supported.', !props.actions || !props._actions); + + if (props._actions) { + Ember.deprecate( + 'Specifying actions in `_actions` is deprecated, please use `actions` instead.', + false, + { id: 'ember-runtime.action-handler-_actions', until: '3.0.0' } + ); + + props.actions = props._actions; + delete props._actions; + } } }); export default ActionHandler; export function deprecateUnderscoreActions(factory) { - deprecateProperty(factory.prototype, '_actions', 'actions', { - id: 'ember-runtime.action-handler-_actions', until: '3.0.0' + function deprecate() { + Ember.deprecate( + `Usage of \`_actions\` is deprecated, use \`actions\` instead.`, + false, + { id: 'ember-runtime.action-handler-_actions', until: '3.0.0' } + ); + } + + Object.defineProperty(factory.prototype, '_actions', { + configurable: true, + enumerable: false, + set(value) { + Ember.assert(`You cannot set \`_actions\` on ${this}, please use \`actions\` instead.`); + }, + get() { + deprecate(); + return get(this, 'actions'); + } }); }