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.

Sign up for GitHub

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

Jump to bottom

Bare minimum for custom effects (take 2) #3573

Closed
wants to merge 4 commits into from
+307 −84
Closed

Bare minimum for custom effects (take 2) #3573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
59b5f24
Eliminate creating accessors for parent props; use Bind API directly.
kevinpschaaf Nov 23, 2015
35a533a
Do not pre-store effects in the accessors.
kaste Apr 9, 2016
523ef03
Expose bare minimum to add effects dynamically.
kaste Mar 9, 2016
f6b107d
Use functionPath-effects in templatizer.
kaste Apr 10, 2016
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
14 changes: 9 additions & 5 deletions src/lib/bind/accessors.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@

_effectEffects: function(property, value, effects, old, fromAbove) {
for (var i=0, l=effects.length, fx; (i<l) && (fx=effects[i]); i++) {
fx.fn.call(this, property, value, fx.effect, old, fromAbove);
if (!fx.disabled) {
fx.fn.call(this, property, value, fx.effect, old, fromAbove);
}
}
},

Expand Down Expand Up @@ -111,7 +113,8 @@
var propEffect = {
kind: kind,
effect: effect,
fn: Polymer.Bind['_' + kind + 'Effect']
fn: Polymer.Bind['_' + kind + 'Effect'],
trigger: property
};
fx.push(propEffect);
return propEffect;
Expand All @@ -129,7 +132,7 @@
// effects have priority
fx.sort(this._sortPropertyEffects);
// create accessors
this._createAccessors(model, n, fx);
this._createAccessors(model, n);
}
}
//console.groupEnd();
Expand Down Expand Up @@ -157,14 +160,15 @@

// create accessors that implement effects

_createAccessors: function(model, property, effects) {
_createAccessors: function(model, property) {
var defun = {
get: function() {
// TODO(sjmiles): elide delegation for performance, good ROI?
return this.__data__[property];
}
};
var setter = function(value) {
var effects = this._propertyEffects && this._propertyEffects[property];
this._propertySetter(property, value, effects);
};
// ReadOnly properties have a private setter only
Expand Down Expand Up @@ -224,7 +228,7 @@
// queued events during configuration can theoretically lead to
// divergence of the passed value from the current value, but we
// really need to track down a specific case where this happens.
value = target[property];
value = target.__data__ ? target.__data__[property] : target[property];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this reads a little clearer.

value = (target.__data__ || target)[property]


if (negated) {
value = !value;
Expand Down
144 changes: 71 additions & 73 deletions src/lib/template/templatizer.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
archetype._prepBindings();

// boilerplate code
archetype._notifyPathUp = this._notifyPathUpImpl;
archetype._notifyPathUp = function() {};
archetype._scopeElementClass = this._scopeElementClassImpl;
archetype.listen = this._listenImpl;
archetype._showHideChildren = this._showHideChildrenImpl;
Expand Down Expand Up @@ -193,7 +193,7 @@
}
for (prop in this._instanceProps) {
archetype._addPropertyEffect(prop, 'function',
this._createInstancePropEffector(prop));
this._createInstancePropEffector());
}
},

Expand All @@ -215,101 +215,124 @@
archetype._parentProps = c._parentProps;
},

// Sets up accessors on the template to call abstract _forwardParentProp
// API that should be implemented by Templatizer users to get parent
// properties to their template instances. These accessors are memoized
// on the archetype and copied to instances.
// Sets up accessors on the template to call abstract
// _forwardParentProp/Path API that should be implemented by Templatizer
// users to get parent properties to their template instances. These
// accessors are memoized on the archetype and copied to instances.
_prepParentProperties: function(archetype, template) {
var parentProps = this._parentProps = archetype._parentProps;
if (this._forwardParentProp && parentProps) {
if ((this._forwardParentProp || this._forwardParentPath) &&
parentProps) {
// Prototype setup (memoized on archetype)
var proto = archetype._parentPropProto;
var prop;
if (!proto) {
for (prop in this._instanceProps) {
delete parentProps[prop];
}
proto = archetype._parentPropProto = Object.create(null);
if (template != this) {
// Assumption: if `this` isn't the template being templatized,
// assume that the template is not a Poylmer.Base, so prep it
// for binding
Polymer.Bind.prepareModel(proto);
Polymer.Base.prepareModelNotifyPath(proto);
}
// Create accessors for each parent prop that forward the property
// to template instances through abstract _forwardParentProp API
// that should be implemented by Templatizer users
// Create accessors for each parent prop that forward the property to
// template instances through abstract _forwardParentProp/Path API
// that should be implemented by Templatizer users.
var propertyEffects = this.mixin({}, this._propertyEffects);
var propertyInfo = this.mixin({}, this._propertyInfo);
var prefixedParentProps = {};
for (prop in parentProps) {
var parentProp = this._parentPropPrefix + prop;
// TODO(sorvell): remove reference Bind library functions here.
// Needed for effect optimization.
var effects = [{
kind: 'function',
effect: this._createForwardPropEffector(prop),
fn: Polymer.Bind._functionEffect
fn: Polymer.Bind._functionEffect,
pathFn: this._functionPathEffect
}, {
kind: 'notify',
fn: Polymer.Bind._notifyEffect,
effect: {event:
Polymer.CaseMap.camelToDashCase(parentProp) + '-changed'}
}];
Polymer.Bind._createAccessors(proto, parentProp, effects);
propertyEffects[parentProp] = effects;
propertyInfo[parentProp] = {};
prefixedParentProps[parentProp] = true;
}
proto = archetype._parentPropProto = {
_propertyEffects: propertyEffects,
_propertyInfo: propertyInfo,
prefixedParentProps: prefixedParentProps
};
}
// capture this reference for use below
var self = this;
// Instance setup
if (template != this) {
Polymer.Bind.prepareModel(template);
Polymer.Base.prepareModelNotifyPath(template);
Polymer.Bind.prepareInstance(template);
template._forwardParentProp = function(source, value) {
self._forwardParentProp(source, value);
if (self._forwardParentProp) {
template._forwardParentProp = function(prop, value) {
self._forwardParentProp(prop, value);
}
}
if (self._forwardParentPath) {
template._forwardParentPath = function(path, value) {
self._forwardParentPath(path, value);
}
}
}
this._extendTemplate(template, proto);
template._pathEffector = function(path, value, fromAbove) {
return self._pathEffectorImpl(path, value, fromAbove);
}
}
},


_createForwardPropEffector: function(prop) {
return function(source, value) {
this._forwardParentProp(prop, value);
var prefixLength = this._parentPropPrefix.length;
return function(path, value) {
if (path.indexOf('.') !== -1) {
if (this._forwardParentPath) {
var newPath = path.slice(prefixLength);
this._forwardParentPath(newPath, value);
}
} else if (this._forwardParentProp) {
this._forwardParentProp(prop, value);
}
};
},

_createHostPropEffector: function(prop) {
var prefix = this._parentPropPrefix;
return function(source, value) {
this.dataHost._templatized[prefix + prop] = value;
var prefixedProp = prefix + prop;
return function(path, value, old, fromAbove) {
if (!fromAbove) {
var dataHost = this.dataHost;
if (path.indexOf('.') !== -1) {
dataHost._templatized._notifyPath(prefix + path, value, false);
} else {
dataHost._templatized.__setProperty(prefixedProp, value, false);
}
}
};
},

_createInstancePropEffector: function(prop) {
return function(source, value, old, fromAbove) {
_createInstancePropEffector: function() {
return function(path, value, old, fromAbove) {
if (!fromAbove) {
this.dataHost._forwardInstanceProp(this, prop, value);
if (path.indexOf('.') !== -1) {
this.dataHost._forwardInstancePath(this, path, value);
} else {
this.dataHost._forwardInstanceProp(this, path, value);
}
}
};
},

// Similar to Polymer.Base.extend, but retains any previously set instance
// values (_propertySetter back on instance once accessor is installed)
// Extends template with parent property info & effects and seed pre-bound data
_extendTemplate: function(template, proto) {
var n$ = Object.getOwnPropertyNames(proto);
if (proto._propertySetter) {
// _propertySetter API may need to be copied onto the template,
// and it needs to come first to allow the property swizzle below
template._propertySetter = proto._propertySetter;
}
for (var i=0, n; (i<n$.length) && (n=n$[i]); i++) {
var val = template[n];
var pd = Object.getOwnPropertyDescriptor(proto, n);
Object.defineProperty(template, n, pd);
if (val !== undefined) {
template._propertySetter(n, val);
}
template._propertyEffects = proto._propertyEffects;
template._propertyInfo = proto._propertyInfo;
var prefixedParentProps = proto.prefixedParentProps;
for (var p in prefixedParentProps) {
var val = template[p];
template.__data__[p] = val;
}
},

Expand All @@ -323,31 +346,6 @@
// _forwardParentProp: function(prop, value) { },
/* eslint-enable no-unused-vars */

_notifyPathUpImpl: function(path, value) {
var dataHost = this.dataHost;
var dot = path.indexOf('.');
var root = dot < 0 ? path : path.slice(0, dot);
// Call extension point for Templatizer sub-classes
dataHost._forwardInstancePath.call(dataHost, this, path, value);
if (root in dataHost._parentProps) {
dataHost._templatized._notifyPath(dataHost._parentPropPrefix + path, value);
}
},

// Overrides Base notify-path module
_pathEffectorImpl: function(path, value, fromAbove) {
if (this._forwardParentPath) {
if (path.indexOf(this._parentPropPrefix) === 0) {
var subPath = path.substring(this._parentPropPrefix.length);
var model = this._modelForPath(subPath);
if (model in this._parentProps) {
this._forwardParentPath(subPath, value);
}
}
}
Polymer.Base._pathEffector.call(this._templatized, path, value, fromAbove);
},

_constructorImpl: function(model, host) {
this._rootDataHost = host._getRootDataHost();
this._setupConfigure(model);
Expand Down Expand Up @@ -425,7 +423,7 @@
var templatized = this._templatized;
for (var prop in this._parentProps) {
if (model[prop] === undefined) {
model[prop] = templatized[this._parentPropPrefix + prop];
model[prop] = templatized.__data__[this._parentPropPrefix + prop];
}
}
}
Expand Down
67 changes: 67 additions & 0 deletions src/standard/effectBuilder.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,73 @@
var prop = Polymer.Bind.addPropertyEffect(this, property, kind, effect);
// memoize path function for faster lookup.
prop.pathFn = this['_' + prop.kind + 'PathEffect'];
return prop;
},

_ensureOwnEffects: function(property) {
// Move the property effects from the prototype to the instance, so
// we can mutate them. We use prototypal inheritance here to go cheap.
var pE = this._propertyEffects;
if (pE) {
if (!this.hasOwnProperty('_propertyEffects')) {
pE = this._propertyEffects = Polymer.Base.chainObject({}, pE);
}

// Shallow-copy the effects array for this property, to the instance.
if (pE[property] && !pE.hasOwnProperty(property)) {
pE[property] = pE[property].slice();
}
}
},

addPropertyEffect: function(property, fn) {
this._ensureOwnEffects(property);

// We have to create the underlying trigger machinery here, if this is
// the first effect of the property.
var effects = Polymer.Bind.ensurePropertyEffects(this, property);
if (effects.length === 0) {
// The current value will be masked by the descriptor, read it ...
var val = this[property];
Polymer.Bind._createAccessors(this, property);
// ... and put it in our data store
this.__data__[property] = val;
}

// Theoretically the effects must be sorted again, but `function`-effects
// are executed last anyway, so we can skip this for now. NOTE that, if
// you add a custom effect inside another effect, t.i. the notification-
// loop is already running, mutating the effects in place like we do here
// would be usually catastrophic. This 'just' works b/c we prestore the
// loop-length in our for-loops (see `effectEffects`). Note as well that
// the newly added effect will not be executed before the next
// notification we have to propagate. This is intended and a bonus point
// for appending only.
return this._addPropertyEffect(property, 'function', fn);
},

removePropertyEffect: function(fx) {
// Since effects can be removed as a side-effect, t.i. within a running
// effects-loop, we must ensure not to shorten the effects-array in
// place, otherwise the for-loop would break. On the other side removing
// an effect must be immediate, so we use a flag here to indicate that
// this effect should be skipped.
fx.disabled = true;

// We then slice and splice and reassign the effects-array. This new
// array will be used not before the next notification we have to
// propagate.
var property = fx.trigger;
var effects = property && this._propertyEffects &&
this._propertyEffects[property];
if (effects) {
var index = effects.indexOf(fx);
if (index !== -1) {
effects = effects.slice();
effects.splice(index, 1);
this._propertyEffects[property] = effects;
}
}
},

// prototyping
Expand Down
Loading