Skip to content

Commit

Permalink
[BUGFIX release] Ensure _actions specified to extend works.
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
rwjblue committed Aug 24, 2015
1 parent 3e15e67 commit dcc0dc3
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 3 deletions.
33 changes: 33 additions & 0 deletions packages/ember-routing/tests/system/route_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 33 additions & 3 deletions packages/ember-runtime/lib/mixins/action_handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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');
}
});
}

0 comments on commit dcc0dc3

Please sign in to comment.