Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEAT] Makes the @action decorator work in classic classes #17827

Merged
merged 1 commit into from
Apr 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/@ember/-internals/metal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ export {
PROPERTY_DID_CHANGE,
} from './lib/property_events';
export { defineProperty } from './lib/properties';
export { nativeDescDecorator } from './lib/decorator';
export { isElementDescriptor, nativeDescDecorator } from './lib/decorator';
export {
descriptorForProperty,
isComputedDecorator,
setComputedDecorator,
isClassicDecorator,
setClassicDecorator,
} from './lib/descriptor_map';
export { watchKey, unwatchKey } from './lib/watch_key';
export { ChainNode, finishChains, removeChainWatcher } from './lib/chains';
Expand Down
4 changes: 2 additions & 2 deletions packages/@ember/-internals/metal/lib/computed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
makeComputedDecorator,
removeDependentKeys,
} from './decorator';
import { descriptorForDecorator, isComputedDecorator } from './descriptor_map';
import { descriptorForDecorator, isClassicDecorator } from './descriptor_map';
import expandProperties from './expand_properties';
import { defineProperty } from './properties';
import { notifyPropertyChange } from './property_events';
Expand Down Expand Up @@ -180,7 +180,7 @@ export class ComputedProperty extends ComputedDescriptor {
if (typeof config === 'function') {
assert(
`You attempted to pass a computed property instance to computed(). Computed property instances are decorator functions, and cannot be passed to computed() because they cannot be turned into decorators twice`,
!isComputedDecorator(config)
!isClassicDecorator(config)
);

this._getter = config as ComputedPropertyGetter;
Expand Down
6 changes: 3 additions & 3 deletions packages/@ember/-internals/metal/lib/decorator.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Meta, meta as metaFor } from '@ember/-internals/meta';
import { EMBER_NATIVE_DECORATOR_SUPPORT } from '@ember/canary-features';
import { assert } from '@ember/debug';
import { setComputedDecorator } from './descriptor_map';
import { setClassicDecorator } from './descriptor_map';
import { unwatch, watch } from './watching';

export type DecoratorPropertyDescriptor = PropertyDescriptor & { initializer?: any } | undefined;
Expand Down Expand Up @@ -90,7 +90,7 @@ export function nativeDescDecorator(propertyDesc: PropertyDescriptor) {
return propertyDesc;
};

setComputedDecorator(decorator);
setClassicDecorator(decorator);

return decorator;
}
Expand Down Expand Up @@ -170,7 +170,7 @@ export function makeComputedDecorator(
};
};

setComputedDecorator(decorator, desc);
setClassicDecorator(decorator, desc);

Object.setPrototypeOf(decorator, DecoratorClass.prototype);

Expand Down
8 changes: 4 additions & 4 deletions packages/@ember/-internals/metal/lib/descriptor_map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,22 @@ export function descriptorForDecorator(dec: import('./decorator').Decorator) {
/**
Check whether a value is a decorator

@method isComputedDecorator
@method isClassicDecorator
@param {any} possibleDesc the value to check
@return {boolean}
@private
*/
export function isComputedDecorator(dec: any) {
export function isClassicDecorator(dec: any) {
return dec !== null && dec !== undefined && DECORATOR_DESCRIPTOR_MAP.has(dec);
}

/**
Set a value as a decorator

@method setComputedDecorator
@method setClassicDecorator
@param {function} decorator the value to mark as a decorator
@private
*/
export function setComputedDecorator(dec: import('./decorator').Decorator, value: any = true) {
export function setClassicDecorator(dec: import('./decorator').Decorator, value: any = true) {
DECORATOR_DESCRIPTOR_MAP.set(dec, value);
}
4 changes: 2 additions & 2 deletions packages/@ember/-internals/metal/lib/mixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { makeComputedDecorator, nativeDescDecorator } from './decorator';
import {
descriptorForDecorator,
descriptorForProperty,
isComputedDecorator,
isClassicDecorator,
} from './descriptor_map';
import { addListener, removeListener } from './events';
import expandProperties from './expand_properties';
Expand Down Expand Up @@ -277,7 +277,7 @@ function addNormalizedProperty(
concats?: string[],
mergings?: string[]
): void {
if (isComputedDecorator(value)) {
if (isClassicDecorator(value)) {
// Wrap descriptor function to implement _super() if needed
descs[key] = giveDecoratorSuper(meta, key, value, values, descs, base);
values[key] = undefined;
Expand Down
4 changes: 2 additions & 2 deletions packages/@ember/-internals/metal/lib/properties.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Meta, meta as metaFor, peekMeta, UNDEFINED } from '@ember/-internals/me
import { assert } from '@ember/debug';
import { DEBUG } from '@glimmer/env';
import { Decorator } from './decorator';
import { descriptorForProperty, isComputedDecorator } from './descriptor_map';
import { descriptorForProperty, isClassicDecorator } from './descriptor_map';
import { overrideChains } from './property_events';

export type MandatorySetterFunction = ((this: object, value: any) => void) & {
Expand Down Expand Up @@ -149,7 +149,7 @@ export function defineProperty(
}

let value;
if (isComputedDecorator(desc)) {
if (isClassicDecorator(desc)) {
let propertyDesc;

if (DEBUG) {
Expand Down
6 changes: 3 additions & 3 deletions packages/@ember/-internals/metal/lib/tracked.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { assert } from '@ember/debug';
import { DEBUG } from '@glimmer/env';
import { combine, CONSTANT_TAG, Tag } from '@glimmer/reference';
import { Decorator, DecoratorPropertyDescriptor, isElementDescriptor } from './decorator';
import { setComputedDecorator } from './descriptor_map';
import { setClassicDecorator } from './descriptor_map';
import { dirty, ensureRunloop, tagFor, tagForProperty } from './tags';

type Option<T> = T | null;
Expand Down Expand Up @@ -164,7 +164,7 @@ export function tracked(...args: any[]): Decorator | DecoratorPropertyDescriptor
return descriptorForField([target, key, fieldDesc]);
};

setComputedDecorator(decorator);
setClassicDecorator(decorator);

return decorator;
}
Expand All @@ -180,7 +180,7 @@ export function tracked(...args: any[]): Decorator | DecoratorPropertyDescriptor
if (DEBUG) {
// Normally this isn't a classic decorator, but we want to throw a helpful
// error in development so we need it to treat it like one
setComputedDecorator(tracked);
setClassicDecorator(tracked);
}

function descriptorForField([_target, key, desc]: [
Expand Down
4 changes: 2 additions & 2 deletions packages/@ember/-internals/metal/lib/watch_key.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Meta, meta as metaFor, peekMeta, UNDEFINED } from '@ember/-internals/meta';
import { lookupDescriptor } from '@ember/-internals/utils';
import { DEBUG } from '@glimmer/env';
import { descriptorForProperty, isComputedDecorator } from './descriptor_map';
import { descriptorForProperty, isClassicDecorator } from './descriptor_map';
import {
DEFAULT_GETTER_FUNCTION,
INHERITING_GETTER_FUNCTION,
Expand Down Expand Up @@ -56,7 +56,7 @@ if (DEBUG) {
let descriptor = lookupDescriptor(obj, keyName);
let hasDescriptor = descriptor !== null;
let possibleDesc = hasDescriptor && descriptor!.value;
if (isComputedDecorator(possibleDesc)) {
if (isClassicDecorator(possibleDesc)) {
return;
}
let configurable = hasDescriptor ? descriptor!.configurable : true;
Expand Down
4 changes: 2 additions & 2 deletions packages/@ember/-internals/metal/tests/computed_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
computed,
getCachedValueFor,
defineProperty,
isComputedDecorator,
isClassicDecorator,
get,
set,
isWatching,
Expand All @@ -18,7 +18,7 @@ moduleFor(
'computed',
class extends AbstractTestCase {
['@test computed property should be an instance of descriptor'](assert) {
assert.ok(isComputedDecorator(computed(function() {})));
assert.ok(isClassicDecorator(computed(function() {})));
}

['@test computed properties assert the presence of a getter or setter function']() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { setOwner } from '@ember/-internals/owner';
import { defineProperty, get, isComputedDecorator, set, inject } from '..';
import { defineProperty, get, isClassicDecorator, set, inject } from '..';
import { moduleFor, AbstractTestCase } from 'internal-test-helpers';

moduleFor(
'inject',
class extends AbstractTestCase {
['@test injected properties should be descriptors'](assert) {
assert.ok(isComputedDecorator(inject('type')));
assert.ok(isClassicDecorator(inject('type')));
}

['@test injected properties should be overridable'](assert) {
Expand Down
4 changes: 2 additions & 2 deletions packages/@ember/-internals/runtime/lib/system/core_object.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
defineProperty,
descriptorForProperty,
classToString,
isComputedDecorator,
isClassicDecorator,
DEBUG_INJECTION_FUNCTIONS,
} from '@ember/-internals/metal';
import ActionHandler from '../mixins/action_handler';
Expand Down Expand Up @@ -78,7 +78,7 @@ function initialize(obj, properties) {
'EmberObject.create no longer supports defining computed ' +
'properties. Define computed properties using extend() or reopen() ' +
'before calling create().',
!isComputedDecorator(value)
!isClassicDecorator(value)
);
assert(
'EmberObject.create no longer supports defining methods that call _super.',
Expand Down
44 changes: 39 additions & 5 deletions packages/@ember/object/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { EMBER_NATIVE_DECORATOR_SUPPORT } from '@ember/canary-features';
import { assert } from '@ember/debug';
import { assign } from '@ember/polyfills';
import { isElementDescriptor, setClassicDecorator } from '@ember/-internals/metal';

/**
Decorator that turns the target function into an Action
Expand Down Expand Up @@ -41,11 +42,7 @@ export let action;
if (EMBER_NATIVE_DECORATOR_SUPPORT) {
let BINDINGS_MAP = new WeakMap();

action = function action(target, key, desc) {
assert('The @action decorator must be applied to methods', typeof desc.value === 'function');

let actionFn = desc.value;

let setupAction = function(target, key, actionFn) {
if (target.constructor !== undefined && typeof target.constructor.proto === 'function') {
target.constructor.proto();
}
Expand Down Expand Up @@ -78,4 +75,41 @@ if (EMBER_NATIVE_DECORATOR_SUPPORT) {
},
};
};

action = function action(target, key, desc) {
let actionFn;

if (!isElementDescriptor([target, key, desc])) {
actionFn = target;

let decorator = function(target, key, desc, meta, isClassicDecorator) {
assert(
'The @action decorator may only be passed a method when used in classic classes. You should decorate methods directly in native classes',
isClassicDecorator
);

assert(
'The action() decorator must be passed a method when used in classic classes',
typeof actionFn === 'function'
);

return setupAction(target, key, actionFn);
};

setClassicDecorator(decorator);

return decorator;
}

actionFn = desc.value;

assert(
'The @action decorator must be applied to methods when used in native classes',
typeof actionFn === 'function'
);

return setupAction(target, key, actionFn);
};

setClassicDecorator(action);
}
44 changes: 44 additions & 0 deletions packages/@ember/object/tests/action_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,50 @@ if (EMBER_NATIVE_DECORATOR_SUPPORT) {
new TestObject();
}, /The @action decorator must be applied to methods/);
}

'@test action decorator throws an error if passed a function in native classes'() {
expectAssertion(() => {
class TestObject extends EmberObject {
@action(function() {}) foo = 'bar';
}

new TestObject();
}, /The @action decorator may only be passed a method when used in classic classes/);
}

'@test action decorator can be used as a classic decorator with strings'(assert) {
let FooComponent = Component.extend({
foo: action(function() {
assert.ok(true, 'called!');
}),
});

this.registerComponent('foo-bar', {
ComponentClass: FooComponent,
template: "<button {{action 'foo'}}>Click Me!</button>",
});

this.render('{{foo-bar}}');

this.$('button').click();
}

'@test action decorator can be used as a classic decorator directly'(assert) {
let FooComponent = Component.extend({
foo: action(function() {
assert.ok(true, 'called!');
}),
});

this.registerComponent('foo-bar', {
ComponentClass: FooComponent,
template: '<button onclick={{this.foo}}>Click Me!</button>',
});

this.render('{{foo-bar}}');

this.$('button').click();
}
}
);
}
2 changes: 1 addition & 1 deletion packages/ember/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ Ember._tracked = metal.tracked;
computed.alias = metal.alias;
Ember.cacheFor = metal.getCachedValueFor;
Ember.ComputedProperty = metal.ComputedProperty;
Ember._setComputedDecorator = metal.setComputedDecorator;
Ember._setClassicDecorator = metal.setClassicDecorator;
Ember.meta = meta;
Ember.get = metal.get;
Ember.getWithDefault = metal.getWithDefault;
Expand Down
2 changes: 1 addition & 1 deletion packages/ember/tests/reexports_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ let allExports = [
['_tracked', '@ember/-internals/metal', 'tracked'],
['computed.alias', '@ember/-internals/metal', 'alias'],
['ComputedProperty', '@ember/-internals/metal'],
['_setComputedDecorator', '@ember/-internals/metal', 'setComputedDecorator'],
['_setClassicDecorator', '@ember/-internals/metal', 'setClassicDecorator'],
['cacheFor', '@ember/-internals/metal', 'getCachedValueFor'],
['merge', '@ember/polyfills'],
['instrument', '@ember/instrumentation'],
Expand Down
4 changes: 2 additions & 2 deletions tests/docs/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ module.exports = {
'isArray',
'isBlank',
'isBrowser',
'isComputedDecorator',
'isClassicDecorator',
'isDestroyed',
'isDestroying',
'isEmpty',
Expand Down Expand Up @@ -501,7 +501,7 @@ module.exports = {
'serializeQueryParam',
'serializeQueryParamKey',
'set',
'setComputedDecorator',
'setClassicDecorator',
'setDiff',
'setEach',
'setEngineParent',
Expand Down