From 528d6716befec92bd41617b3fd1a0d58ae114354 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Sat, 30 Mar 2019 19:55:37 -0700 Subject: [PATCH] [FEAT] Makes the @action decorator work in classic classes Currently, the new `@action` decorator is the only decorator in Ember proper that doesn't work in classic classes currently. Since users could define actions on the actions hash in classic classes, we didn't originally see a reason for it to work in classic classes. However, with the fact that the decorator now _binds context_, it may actually become useful for users, especially if they decide to convert away from the `{{action}}` modifier/helper. ```js const FooComponent = Component.extend({ myAction: action(function() { // do a thing }), }); ``` Additionally, this makes them match up with our learning story for the rest of decorators - they're _always_ decorators, they just use different syntax in native classes and classic classes. --- packages/@ember/-internals/metal/index.ts | 6 +-- .../@ember/-internals/metal/lib/computed.ts | 4 +- .../@ember/-internals/metal/lib/decorator.ts | 6 +-- .../-internals/metal/lib/descriptor_map.ts | 8 ++-- packages/@ember/-internals/metal/lib/mixin.ts | 4 +- .../@ember/-internals/metal/lib/properties.ts | 4 +- .../@ember/-internals/metal/lib/tracked.ts | 6 +-- .../@ember/-internals/metal/lib/watch_key.ts | 4 +- .../-internals/metal/tests/computed_test.js | 4 +- .../metal/tests/injected_property_test.js | 4 +- .../runtime/lib/system/core_object.js | 4 +- packages/@ember/object/index.js | 44 ++++++++++++++++--- packages/@ember/object/tests/action_test.js | 44 +++++++++++++++++++ packages/ember/index.js | 2 +- packages/ember/tests/reexports_test.js | 2 +- tests/docs/expected.js | 4 +- 16 files changed, 114 insertions(+), 36 deletions(-) diff --git a/packages/@ember/-internals/metal/index.ts b/packages/@ember/-internals/metal/index.ts index 357244e49f2..a5cc6f31277 100644 --- a/packages/@ember/-internals/metal/index.ts +++ b/packages/@ember/-internals/metal/index.ts @@ -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'; diff --git a/packages/@ember/-internals/metal/lib/computed.ts b/packages/@ember/-internals/metal/lib/computed.ts index 750c45d57c3..b705fea6404 100644 --- a/packages/@ember/-internals/metal/lib/computed.ts +++ b/packages/@ember/-internals/metal/lib/computed.ts @@ -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'; @@ -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; diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index b0489fa1947..8b5d3513b6a 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -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; @@ -90,7 +90,7 @@ export function nativeDescDecorator(propertyDesc: PropertyDescriptor) { return propertyDesc; }; - setComputedDecorator(decorator); + setClassicDecorator(decorator); return decorator; } @@ -170,7 +170,7 @@ export function makeComputedDecorator( }; }; - setComputedDecorator(decorator, desc); + setClassicDecorator(decorator, desc); Object.setPrototypeOf(decorator, DecoratorClass.prototype); diff --git a/packages/@ember/-internals/metal/lib/descriptor_map.ts b/packages/@ember/-internals/metal/lib/descriptor_map.ts index b228e0646a6..dcc5fd5796e 100644 --- a/packages/@ember/-internals/metal/lib/descriptor_map.ts +++ b/packages/@ember/-internals/metal/lib/descriptor_map.ts @@ -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); } diff --git a/packages/@ember/-internals/metal/lib/mixin.ts b/packages/@ember/-internals/metal/lib/mixin.ts index d59c74865e3..4c3e25d59af 100644 --- a/packages/@ember/-internals/metal/lib/mixin.ts +++ b/packages/@ember/-internals/metal/lib/mixin.ts @@ -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'; @@ -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; diff --git a/packages/@ember/-internals/metal/lib/properties.ts b/packages/@ember/-internals/metal/lib/properties.ts index 35689551441..0c468490a6f 100644 --- a/packages/@ember/-internals/metal/lib/properties.ts +++ b/packages/@ember/-internals/metal/lib/properties.ts @@ -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) & { @@ -149,7 +149,7 @@ export function defineProperty( } let value; - if (isComputedDecorator(desc)) { + if (isClassicDecorator(desc)) { let propertyDesc; if (DEBUG) { diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts index 8b0e9feeef7..75abe6c3768 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/packages/@ember/-internals/metal/lib/tracked.ts @@ -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 | null; @@ -164,7 +164,7 @@ export function tracked(...args: any[]): Decorator | DecoratorPropertyDescriptor return descriptorForField([target, key, fieldDesc]); }; - setComputedDecorator(decorator); + setClassicDecorator(decorator); return decorator; } @@ -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]: [ diff --git a/packages/@ember/-internals/metal/lib/watch_key.ts b/packages/@ember/-internals/metal/lib/watch_key.ts index 062681e9c99..d087258bf27 100644 --- a/packages/@ember/-internals/metal/lib/watch_key.ts +++ b/packages/@ember/-internals/metal/lib/watch_key.ts @@ -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, @@ -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; diff --git a/packages/@ember/-internals/metal/tests/computed_test.js b/packages/@ember/-internals/metal/tests/computed_test.js index 52e3df08718..4027c8136a7 100644 --- a/packages/@ember/-internals/metal/tests/computed_test.js +++ b/packages/@ember/-internals/metal/tests/computed_test.js @@ -3,7 +3,7 @@ import { computed, getCachedValueFor, defineProperty, - isComputedDecorator, + isClassicDecorator, get, set, isWatching, @@ -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']() { diff --git a/packages/@ember/-internals/metal/tests/injected_property_test.js b/packages/@ember/-internals/metal/tests/injected_property_test.js index 06434e74920..734ce2ed2cb 100644 --- a/packages/@ember/-internals/metal/tests/injected_property_test.js +++ b/packages/@ember/-internals/metal/tests/injected_property_test.js @@ -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) { diff --git a/packages/@ember/-internals/runtime/lib/system/core_object.js b/packages/@ember/-internals/runtime/lib/system/core_object.js index 87bc7487303..9c220cc6299 100644 --- a/packages/@ember/-internals/runtime/lib/system/core_object.js +++ b/packages/@ember/-internals/runtime/lib/system/core_object.js @@ -23,7 +23,7 @@ import { defineProperty, descriptorForProperty, classToString, - isComputedDecorator, + isClassicDecorator, DEBUG_INJECTION_FUNCTIONS, } from '@ember/-internals/metal'; import ActionHandler from '../mixins/action_handler'; @@ -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.', diff --git a/packages/@ember/object/index.js b/packages/@ember/object/index.js index e28e8c452aa..cc02ee6567f 100644 --- a/packages/@ember/object/index.js +++ b/packages/@ember/object/index.js @@ -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 @@ -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(); } @@ -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); } diff --git a/packages/@ember/object/tests/action_test.js b/packages/@ember/object/tests/action_test.js index 23dfc317b27..d1ccc8c0bcb 100644 --- a/packages/@ember/object/tests/action_test.js +++ b/packages/@ember/object/tests/action_test.js @@ -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: "", + }); + + 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: '', + }); + + this.render('{{foo-bar}}'); + + this.$('button').click(); + } } ); } diff --git a/packages/ember/index.js b/packages/ember/index.js index fdda6f97ecc..8c415902eaa 100644 --- a/packages/ember/index.js +++ b/packages/ember/index.js @@ -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; diff --git a/packages/ember/tests/reexports_test.js b/packages/ember/tests/reexports_test.js index 62d310aa4be..a3ada5b56c3 100644 --- a/packages/ember/tests/reexports_test.js +++ b/packages/ember/tests/reexports_test.js @@ -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'], diff --git a/tests/docs/expected.js b/tests/docs/expected.js index 72ceeb1822d..58d6371f883 100644 --- a/tests/docs/expected.js +++ b/tests/docs/expected.js @@ -298,7 +298,7 @@ module.exports = { 'isArray', 'isBlank', 'isBrowser', - 'isComputedDecorator', + 'isClassicDecorator', 'isDestroyed', 'isDestroying', 'isEmpty', @@ -501,7 +501,7 @@ module.exports = { 'serializeQueryParam', 'serializeQueryParamKey', 'set', - 'setComputedDecorator', + 'setClassicDecorator', 'setDiff', 'setEach', 'setEngineParent',