Is .ATTRS really necessary? #80
Comments
|
A suggestion for it would be using ES7 Class Properties which are available on Babel 5. |
|
Also everything (at least everything I can think of) could be accomplished by a combo of Class Properties and Decorators. |
|
@yuchi, could you give an example how we can reach similar attribute definition with properties/decorators? I would love to get rid of "ATTRS". Thank you. |
class Attribute {}
class Component extends Attribute {
// Simple property with default value
valueAttribute = 42;
// Simple property with default `valueFn`
valueFnAttribute = this.valueFn();
// Purely dynamic value
get purelyDynamicValue() {
return this._storedValue;
}
set purelyDynamicValue(v) {
this._storedValue;
}
// Lazy valueFn behaviour
@memoize
get lazyValue() {
console.log('Fires once');
return 32;
}
// Write once
@writeOnce
get stickyValue() {
return this._stickyValue;
}
set stickyValue(v) {
this._stickyValue = v;
}
// Methods
valueFn() {
return 42 * 2;
}
}
const c = new Component();
// Will log "Fires once"… well… once!
assert(c.lazyValue === 32);
assert(c.lazyValue === 32);
assert(c.lazyValue === 32);
// Will log "Writes once"… well, you got it.
c.stickyValue = 42;
c.stickyValue = 32;
assert(c.stickyValue === 42);
// Decorators
function writeOnce(host, name, descriptor) {
const { get, set, enumerable, configurable } = descriptor;
const result = { ...descriptor };
const run = Symbol(name + '-has-run');
if (set) {
result.set = function writeOnce(...args) {
if (this[ run ] === true) return;
this[ run ] = true;
set.apply(this, args);
};
}
return result;
}
function memoize(host, name, descriptor) {
const { get, set, enumerable, configurable } = descriptor;
const result = { ...descriptor };
if (get) {
result.get = function memoizedGet(...args) {
const value = get.apply(this, args);
Object.defineProperty(this, name, { value, enumerable, configurable });
return value;
}
}
return result;
}
function assert(a) {
if (!a) throw new Error("Assertion failed");
} |
|
(Very off the envelope examples, took exactly 9 minutes to write that down) |
|
By the way the An (untested) implementation could be the following one: const MY_ENUM = [ 'simple', 'complex', 'idiotic' ];
class Component {
@attribute
set myAttribute(newValue, oldValue) { return MY_ENUM.indexOf(newValue); }
get myAttribute(value) { return MY_ENUM[ value ] || 'idiotic' }
}
function attribute(host, name, descriptor) {
const { get, set, enumerable, configurable } = descriptor;
const result = { ...descriptor };
const storage = Symbol( name + '-storage' );
result.get = function attributeGet() {
return get.call(this, this[ storage ]);
};
result.set = function attributeSet(v) {
this[ storage ] = set.call(this, v, this[ storage ]);
};
return result;
} |
|
Tested it, setter can have only one parameter (why?!) so: const MY_ENUM = [ 'simple', 'complex', 'idiotic' ];
class Component {
@attribute
set myAttribute({ newValue, oldValue }) { return MY_ENUM.indexOf(newValue); }
get myAttribute(value) { return MY_ENUM[ value ] || 'idiotic' }
}
function attribute(host, name, descriptor) {
const { get, set, enumerable, configurable } = descriptor;
const result = { ...descriptor };
const storage = Symbol( name + '-storage' );
result.get = function attributeGet() {
return get.call(this, this[ storage ]);
};
result.set = function attributeSet(v) {
this[ storage ] = set.call(this, { newValue: v, oldValue: this[ storage ] });
};
return result;
}
var c = new Component();
console.log(c.myAttribute);
console.log(c.myAttribute = 'lol');
console.log(c.myAttribute);
console.log(c.myAttribute = 'complex');
console.log(c.myAttribute); |
|
I think my only concerns with decorators (which I do think would be able to solve most of the use cases) would be around sharing those functions, or using them as a reference from either a static method on the Attribute class, or as imported functions. Overall, though, I think it could work to use them. It seems like you should be able to use any arbitrary reference as a decorator, but do you know for sure? |
|
Decorators is just sugar over property descriptor manipulation. The ideas I showed are almost all get/set oriented. Decorators become great when used in class properties though, so we can have |
|
Also, small note to future self: import validator from 'attribute';
class Component {
@validator.string // currently works perfectly on babel
myAwesomeStringOnlyProp = '';
} |
|
Hey @yuchi, Could you please explain how exactly you was going to implement the validator above? Also, did you consider the case when validator's function might be part of the class's instance? For example: import validator from 'attribute';
class Component {
validateMyAwesomeStringOnlyProp(val) {
return typeof val === 'string';
}
@validator // how can you use validateMyAwesomeStringOnlyProp as validator?
myAwesomeStringOnlyProp = '';
} |
|
Probably pseudo code: // attribute.js
export default function validator(fn) {
return (target, name, { value, initializer, enumerable, configurable }) => {
return {
enumerable, configurable,
get() {
return retrieve(this, name, initializer, value);
},
set(newValue) {
if (fn(newValue)) store(this, name, newValue);
}
};
};
}
validator.string = validator(newValue => typeof newValue === 'string');
validator.number = validator(newValue => typeof newValue === 'number');
// etc…
function store(instance, name, value) {
instance[ '_' + name ] = value;
}
function retrieve(instance, name, initializer, defValue) {
const key = '_' + name;
if (key in instance) return instance[ key ];
else if (initializer) return initializer();
else return defValue;
}
// example.js
import validator from 'validator';
class Example {
@validator(value => value % 2 === 0)
prop = 42;
}
const example = new Example();
console.log(example.prop); // logs 42
example.prop = 44;
console.log(example.prop); // logs 44
example.prop = 41;
console.log(example.prop); // logs 44 |
|
Okay, thanks! Yesterday I played with this and I couldn't imagine a way to achieve validator without an internal property. As I see, you did the same, so we are on the same page. Just FYI - we are trying to find a way to share code among modules. Class annotations are one option. If we can combine this with getting rid of ATTRS, it would be awesome, wouldn't it? |
|
Okay, so I have a working prototype of these. It seems the are at least two ways to achieve this, so the question is, which one would you guys prefer? Please vote for: Option 1) import Attribute from 'attribute';
import attr from 'attribute/attr';
class Test extends Attribute {
@attr({
validator: core.isString,
writeOnce: true
})
prop = 42;
}or Option 2) import Attribute from 'attribute';
import validator from 'attribute/Validator';
import writeOnce from 'attribute/WriteOnce';
class Test extends Attribute {
@validator(core.isString)
@writeOnce
prop = 42;
} |
|
I’m in favor of Option 2, for the following reasons:
|
|
Can you share your version of |
|
This is top secret - if I tell you, I will have to kill you. (will commit soon, wait a bit) |
|
Me and @ipeychev were talking today and we realized that we also need some kind of a @attr annotation just for cases where no other attribute annotation will be added but we want to mark the variable as an attribute. For example, if we want an attribute that doesn't have validator or writeOnce. It could look something like this: class Test extends Attribute {
@attr
prop = 42;
}But if any attribute annotation is already used, we don't need the @attr annotation to be required. So the previous example would still work. What do you guys think? |
|
We shooting to different stars here. While I still advocate for annotations to define pieces of the So the annotations would just be a standard library for JS instead of configuration sugar for this specific framework. |
|
On theory, we could go in this direction. However, this will be a lot of work, we should rewrite the Attribute class, to change the EventEmitter and so on. That's why we thought that we can create some kind of sugar on top of what we already have. |
|
We'll keep this notation for now (which is I'll close this for now, but feel free to reopen if you think we should still discuss it. (just make sure to reopen on metal/metal instead, we've moved there. |
I know we're based in YUI's attribute model but I wonder if having this static variable really adds any value.
The text was updated successfully, but these errors were encountered: