ES6 @keyResponder decorator fails with parameters
toovy opened this issue · 2 comments
Hi,
I'm having issues with a fresh 3.20.0 project. Using the @keyResponder decorator without parameters works, e.g.
@keyResponder
export default class Test extends Component {
Making a simple change like adding the parenthesis, or event adding the priority option leads to an error:
@keyResponder()
export default class Test extends Component {
TypeError: Class extends value undefined is not a constructor or null
There seems to be a difference on how the class decorator is called. When no parenthesis or options are added the call to the decorator declared in https://github.com/adopted-ember-addons/ember-keyboard/blob/master/addon/decorators/key-responder.js#L21 sets the first parameter to the DecoratedClass.
But when adding parenthesis or event options like { priority: 1 } the first parameter is not the decorated class any more, but the options, thus https://github.com/adopted-ember-addons/ember-keyboard/blob/master/addon/decorators/key-responder.js#L30 fails as DecoratedClass is the object { priority: 1 }.
Maybe the way babel transpiles the decorators has changed. This might be the reason why older projects don't have an issue.
I experimented a little bit with the code and came up with a solution that might work for both scenarios. I'm doubting this is backwards compatible. Having little experience with decorators I would like to discuss this change before creating a pull request.
export default function keyResponder(opts = {}) {
const createClass = function(DecoratedClass) {
if (opts.priority === undefined) {
opts.priority = 0;
}
if (opts.activated === undefined) {
opts.activated = true;
}
return class ClassAsKeyResponder extends DecoratedClass {
static name = `${DecoratedClass.name}WithKeyResponder`;
@service keyboard;
get keyboardPriority() {
if (super.keyboardPriority === undefined) {
return opts.priority;
}
return super.keyboardPriority;
}
set keyboardPriority(val) {
super.keyboardPriority = val;
}
get keyboardActivated() {
if (super.keyboardActivated === undefined) {
return opts.activated;
}
return super.keyboardActivated;
}
set keyboardActivated(val) {
super.keyboardActivated = val;
}
constructor() {
super(...arguments);
populateKeyboardHandlers(this);
this.keyboard.register(this);
}
willDestroy() {
this.keyboard.unregister(this);
super.willDestroy(...arguments);
}
}
}
if (typeof opts === "function") {
return createClass(opts)
} else {
return function(DecoratedClass) {
return createClass(DecoratedClass)
}
}
}
Any thoughts?
BR Tobias
Also found this bug today, would you mind transforming that fix into a PR? Thank in advance!
I'd be happy to review a PR and fast-track a patch release. We will need to make sure that a) we have tests for both variations, and b) tests are running in CI under Ember 3.20.x.