jayphelps / core-decorators

Library of stage-0 JavaScript decorators (aka ES2016/ES7 decorators but not accurate) inspired by languages that come with built-ins like @​override, @​deprecate, @​autobind, @​mixin and more. Popular with React/Angular, but is framework agnostic.
MIT License
4.51k stars 263 forks source link

@property decorator #44

Open devlato opened 8 years ago

devlato commented 8 years ago

Hi!

It would be great to have a @property decorator which can be used as shorthand for getter/setter declaration:

import {property} from 'core-decorators';

class Item {
    @property
    name;

    _getName() {}  // getter

    _setName() {}  // setter
}

So the decorator could autobind instance methods _get${propertyNameInUpperCamelCase}() and _set${propertyNameInUpperCamelCase}() as getter and setter, i.e. in the example above it will be _getName() and _setName() methods.

If getter and setter are not exist it can be autogenerated. Also some customization for method names can be added, and it can add some __private field which can be used as private store for properties for autogenerated values.

jayphelps commented 8 years ago

I'm probably missing something, but why use Java-style getters/setters instead of native ones?

const __bar__ = Symbol('__bar__');

class Foo {
  get bar() {
    return this[__bar__];
  }

  set bar(value) {
    this[__bar__] = value;
  }
}

Is it just personal preference of not liking the voodoo?

jayphelps commented 8 years ago

Can someone provide a real-world example where these getters/setters being generated benefits you in some way?

jayphelps commented 8 years ago

@silkentrance ah, I definitely see that particular issue but I think we can easy resolve it in a way that would be cleaner and still keep native getter/setters.

What if the @override decorator (or another, new one) could be used to signal that this getter or setter should be merged into an existing descriptor?

class Base {
  get name() {}
  set name(v) {}
}

class Derived {
  @override
  set name(v) {}
}

Under the hood, pseudo code:

function override(target, key, descriptor) {
  if ('get' in descriptor || 'set' in descriptor) {
    const superKlass = Object.getPrototypeOf(target);
    const superDescriptor = Object.getOwnPropertyDescriptor(superKlass, key);
    // Merge the new descriptor in with the old one, which will
    // keep any existing get or set that we aren't overriding
    descriptor = { ...superDescriptor, ...descriptor };
  }

  // the rest of the normal code
}

I'm leaning towards actually making this a new decorator instead currently @override does not mutate the descriptor at all, just tests it overrides. I'm not sure what we'd call it though as it would only be useful for the get/set inheritance use case. @mergeDescriptors , @mergeExistingDescriptor ? :hankey:

jayphelps commented 8 years ago

@silkentrance It seems so. I don't believe OP had your issue in mind, but I could be wrong. If OP can confirm, we can split into a different ticket. I think that descriptor merge issue is indeed a valid use case for a decorator.

silkentrance commented 8 years ago

@jayphelps already did ...

silkentrance commented 8 years ago

CONS: the problem with the original proposal is that the @property decorated property is defined only when instantiating Item. So, the _get and _set methods will still be available in the prototype.