Skip to content

Commit

Permalink
[FEAT] Adds more assertions to native decorators, and parenless invoc…
Browse files Browse the repository at this point in the history
…ation

This upstreams a couple of helpful assertions from ember-decorators,
and also adds the ability to call `@service`, `@controller`, and
`@computed` without parens.
  • Loading branch information
Chris Garrett committed Feb 11, 2019
1 parent 1336873 commit 671231a
Show file tree
Hide file tree
Showing 9 changed files with 162 additions and 21 deletions.
38 changes: 33 additions & 5 deletions packages/@ember/-internals/metal/lib/computed.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { Meta, meta as metaFor, peekMeta } from '@ember/-internals/meta';
import { inspect, toString } from '@ember/-internals/utils';
import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features';
import {
EMBER_METAL_TRACKED_PROPERTIES,
EMBER_NATIVE_DECORATOR_SUPPORT,
} from '@ember/canary-features';
import { assert, deprecate, warn } from '@ember/debug';
import EmberError from '@ember/error';
import {
Expand All @@ -14,10 +17,12 @@ import {
addDependentKeys,
ComputedDescriptor,
Decorator,
ElementDescriptor,
isElementDescriptor,
makeComputedDecorator,
removeDependentKeys,
} from './decorator';
import { descriptorForDecorator } from './descriptor_map';
import { descriptorForDecorator, isComputedDecorator } from './descriptor_map';
import expandProperties from './expand_properties';
import { defineProperty } from './properties';
import { notifyPropertyChange } from './property_events';
Expand Down Expand Up @@ -173,6 +178,11 @@ export class ComputedProperty extends ComputedDescriptor {
let config = args.pop();

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)
);

this._getter = config as ComputedPropertyGetter;
} else {
const objectConfig = config as ComputedPropertyGetterAndSetter;
Expand Down Expand Up @@ -700,12 +710,30 @@ class ComputedDecoratorImpl extends Function {
@static
@param {String} [dependentKeys*] Optional dependent keys that trigger this computed property.
@param {Function} func The computed property function.
@return {ComputedDecorator} property descriptor instance
@return {ComputedDecorator} property decorator instance
@public
*/
export function computed(...args: (string | ComputedPropertyConfig)[]): ComputedDecorator {
export function computed(elementDesc: ElementDescriptor): ElementDescriptor;
export function computed(...args: (string | ComputedPropertyConfig)[]): ComputedDecorator;
export function computed(
...args: (string | ComputedPropertyConfig | ElementDescriptor)[]
): ComputedDecorator | ElementDescriptor {
if (isElementDescriptor(args[0])) {
assert(
'Native decorators are not enabled without the EMBER_NATIVE_DECORATOR_SUPPORT flag. If you are using computed in a classic class, add parenthesis to it: computed()',
Boolean(EMBER_NATIVE_DECORATOR_SUPPORT)
);

let decorator = makeComputedDecorator(
new ComputedProperty([]),
ComputedDecoratorImpl
) as ComputedDecorator;

return decorator(args[0] as ElementDescriptor);
}

return makeComputedDecorator(
new ComputedProperty(args),
new ComputedProperty(args as (string | ComputedPropertyConfig)[]),
ComputedDecoratorImpl
) as ComputedDecorator;
}
Expand Down
15 changes: 15 additions & 0 deletions packages/@ember/-internals/metal/lib/decorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ export type Decorator = (
isClassicDecorator?: boolean
) => ElementDescriptor;

export function isElementDescriptor(maybeDesc: any) {
return (
maybeDesc !== undefined &&
typeof maybeDesc.toString === 'function' &&
maybeDesc.toString() === '[object Descriptor]'
);
}

// ..........................................................
// DEPENDENT KEYS
//
Expand Down Expand Up @@ -135,6 +143,13 @@ export function makeComputedDecorator(
EMBER_NATIVE_DECORATOR_SUPPORT || isClassicDecorator
);

assert(
`Only one computed property decorator can be applied to a class field or accessor, but '${key}' was decorated twice. You may have added the decorator to both a getter and setter, which is unecessary.`,
isClassicDecorator ||
!propertyDesc.get ||
!propertyDesc.get.toString().includes('CPGETTER_FUNCTION')
);

elementDesc.kind = 'method';
elementDesc.descriptor = {
enumerable: desc.enumerable,
Expand Down
2 changes: 1 addition & 1 deletion packages/@ember/-internals/metal/lib/descriptor_map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function descriptorForDecorator(dec: import('./decorator').Decorator) {
@return {boolean}
@private
*/
export function isComputedDecorator(dec: import('./decorator').Decorator | null | undefined) {
export function isComputedDecorator(dec: any) {
return dec !== null && dec !== undefined && DECORATOR_DESCRIPTOR_MAP.has(dec);
}

Expand Down
29 changes: 24 additions & 5 deletions packages/@ember/-internals/metal/lib/injected_property.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { getOwner } from '@ember/-internals/owner';
import { EMBER_MODULE_UNIFICATION } from '@ember/canary-features';
import { EMBER_MODULE_UNIFICATION, EMBER_NATIVE_DECORATOR_SUPPORT } from '@ember/canary-features';
import { assert } from '@ember/debug';
import { DEBUG } from '@glimmer/env';
import { computed } from './computed';
import { ElementDescriptor, isElementDescriptor } from './decorator';
import { defineProperty } from './properties';

export let DEBUG_INJECTION_FUNCTIONS: WeakMap<Function, any>;
Expand All @@ -27,12 +28,19 @@ export interface InjectedPropertyOptions {
@namespace Ember
@constructor
@param {String} type The container type the property will lookup
@param {String} name (optional) The name the property will lookup, defaults
@param {String} nameOrDesc (optional) The name the property will lookup, defaults
to the property's name
@private
*/
export default function inject(type: string, name?: string, options?: InjectedPropertyOptions) {
export default function inject(
type: string,
nameOrDesc: string | ElementDescriptor,
options?: InjectedPropertyOptions
) {
assert('a string type must be provided to inject', typeof type === 'string');

let source: string | undefined, namespace: string | undefined;
let name = typeof nameOrDesc === 'string' ? nameOrDesc : undefined;

if (EMBER_MODULE_UNIFICATION) {
source = options ? options.source : undefined;
Expand All @@ -48,7 +56,7 @@ export default function inject(type: string, name?: string, options?: InjectedPr
}
}

let getInjection = function getInjection(this: any, propertyName: string) {
let getInjection = function(this: any, propertyName: string) {
let owner = getOwner(this) || this.container; // fallback to `container` for backwards compat

assert(
Expand All @@ -68,11 +76,22 @@ export default function inject(type: string, name?: string, options?: InjectedPr
});
}

return computed({
let decorator = computed({
get: getInjection,

set(this: any, keyName: string, value: any) {
defineProperty(this, keyName, null, value);
},
});

if (isElementDescriptor(nameOrDesc)) {
assert(
'Native decorators are not enabled without the EMBER_NATIVE_DECORATOR_SUPPORT flag. If you are using inject in a classic class, add parenthesis to it: inject()',
Boolean(EMBER_NATIVE_DECORATOR_SUPPORT)
);

return decorator(nameOrDesc as ElementDescriptor);
} else {
return decorator;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,26 @@ if (EMBER_NATIVE_DECORATOR_SUPPORT) {
new Foo();
}, /Attempted to apply a computed property that already has a getter\/setter to a foo, but it is a method or an accessor./);
}

['@test it throws if a CP is passed to it']() {
expectAssertion(() => {
class Foo {
bar;

@computed(
'bar',
computed({
get() {
return this._foo;
},
})
)
foo;
}

new Foo();
}, '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');
}
}
);

Expand All @@ -154,7 +174,7 @@ if (EMBER_NATIVE_DECORATOR_SUPPORT) {

['@test computed property works with a getter'](assert) {
class TestObj {
@computed()
@computed
get someGetter() {
return true;
}
Expand Down Expand Up @@ -272,6 +292,19 @@ if (EMBER_NATIVE_DECORATOR_SUPPORT) {
assert.equal(set(obj, 'foo', 'bar'), 'bar', 'should return set value with set()');
assert.equal(count, 0, 'should not have invoked getter');
}

['@test throws if a value is decorated twice']() {
expectAssertion(() => {
class Obj {
@computed
@computed
get foo() {
return this._foo;
}
}
new Obj();
}, "Only one computed property decorator can be applied to a class field or accessor, but 'foo' was decorated twice. You may have added the decorator to both a getter and setter, which is unecessary.");
}
}
);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ moduleFor(
'inject',
class extends AbstractTestCase {
['@test injected properties should be descriptors'](assert) {
assert.ok(isComputedDecorator(inject()));
assert.ok(isComputedDecorator(inject('type')));
}

['@test injected properties should be overridable'](assert) {
let obj = {};
defineProperty(obj, 'foo', inject());
defineProperty(obj, 'foo', inject('type'));

set(obj, 'foo', 'bar');

Expand Down
46 changes: 46 additions & 0 deletions packages/@ember/-internals/runtime/tests/inject_test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { inject } from '@ember/-internals/metal';
import { EMBER_NATIVE_DECORATOR_SUPPORT } from '@ember/canary-features';
import { DEBUG } from '@glimmer/env';
import EmberObject from '../lib/system/object';
import { buildOwner } from 'internal-test-helpers';
Expand Down Expand Up @@ -49,3 +50,48 @@ moduleFor(
}
}
);

if (EMBER_NATIVE_DECORATOR_SUPPORT) {
moduleFor(
'inject - decorator',
class extends AbstractTestCase {
['@test works with native decorators'](assert) {
let owner = buildOwner();

class Service extends EmberObject {}

class Foo extends EmberObject {
@inject('service', 'main') main;
}

owner.register('service:main', Service);
owner.register('foo:main', Foo);

let foo = owner.lookup('foo:main');

assert.ok(foo.main instanceof Service, 'service injected correctly');
}

['@test uses the decorated property key if not provided'](assert) {
let owner = buildOwner();

function service() {
return inject('service', ...arguments);
}

class Service extends EmberObject {}

class Foo extends EmberObject {
@service main;
}

owner.register('service:main', Service);
owner.register('foo:main', Foo);

let foo = owner.lookup('foo:main');

assert.ok(foo.main instanceof Service, 'service injected correctly');
}
}
);
}
8 changes: 4 additions & 4 deletions packages/@ember/controller/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Object as EmberObject } from '@ember/-internals/runtime';
import ControllerMixin from './lib/controller_mixin';
import { inject as metalInject } from '@ember/-internals/metal';
import ControllerMixin from './lib/controller_mixin';

/**
@module @ember/controller
Expand Down Expand Up @@ -40,11 +40,11 @@ const Controller = EmberObject.extend(ControllerMixin);
@since 1.10.0
@param {String} name (optional) name of the controller to inject, defaults
to the property's name
@return {Ember.InjectedProperty} injection descriptor instance
@return {ComputedDecorator} injection decorator instance
@public
*/
export function inject(name, options) {
return metalInject('controller', name, options);
export function inject(nameOrDesc, options) {
return metalInject('controller', nameOrDesc, options);
}

export default Controller;
6 changes: 3 additions & 3 deletions packages/@ember/service/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ import { inject as metalInject } from '@ember/-internals/metal';
@for @ember/service
@param {String} name (optional) name of the service to inject, defaults to
the property's name
@return {Ember.InjectedProperty} injection descriptor instance
@return {ComputedDecorator} injection decorator instance
@public
*/
export function inject(name, options) {
return metalInject('service', name, options);
export function inject(nameOrDesc, options) {
return metalInject('service', nameOrDesc, options);
}

/**
Expand Down

0 comments on commit 671231a

Please sign in to comment.