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.

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

Expose bare minimum to add effects dynamically. #3460

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 5 additions & 2 deletions src/lib/bind/accessors.html
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 Down
2 changes: 2 additions & 0 deletions src/lib/template/templatizer.html
Original file line number Diff line number Diff line change
Expand Up @@ -282,12 +282,14 @@
_createHostPropEffector: function(prop) {
var prefix = this._parentPropPrefix;
return function(source, value) {
if (source !== prop) return;
this.dataHost._templatized[prefix + prop] = value;
};
},

_createInstancePropEffector: function(prop) {
return function(source, value, old, fromAbove) {
if (source !== prop) return;
if (!fromAbove) {
this.dataHost._forwardInstanceProp(this, prop, value);
}
Expand Down
65 changes: 65 additions & 0 deletions src/standard/effectBuilder.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,71 @@
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);

// Because the property descriptor encloses the effects, we recreate
// it. Just for the record: In case this would be the first effect of
// the property, we had to create the underlying trigger machinery
// anyway.
var effects = Polymer.Bind.ensurePropertyEffects(this, property);
if (!effects._installed) {
// The current value will be masked by the descriptor, read it ...
var val = this[property];
Polymer.Bind._createAccessors(this, property, effects);
// closure ^^^^^^^ alert
effects._installed = true;
// ... 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, the newly added effect will not be executed
// before the next notification we have to propagate. This is intended
// and a bonus point for adding all custom effects at the end.
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 change the effects- array
// immediately, otherwise the for-loop would break. On the other side
// removing an effect must be immediate too. So we use a flag here, and
// actually remove the effect in a microtask.
fx.disabled = true;

this.async(function() {
var property = fx.trigger;
var effects = property && this._propertyEffects &&
this._propertyEffects[property];
if (effects) {
var index = effects.indexOf(fx);
if (index !== -1) {
effects.splice(index, 1);
}
}
});
},

// prototyping
Expand Down
14 changes: 10 additions & 4 deletions src/standard/notify-path.html
Original file line number Diff line number Diff line change
Expand Up @@ -274,10 +274,12 @@
var fx$ = this._propertyEffects && this._propertyEffects[model];
if (fx$) {
for (var i=0, fx; (i<fx$.length) && (fx=fx$[i]); i++) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to self: Must prestore length just like in accessors!

// use memoized path functions
var fxFn = fx.pathFn;
if (fxFn) {
fxFn.call(this, path, value, fx.effect);
if (!fx.disabled) {
// use memoized path functions
var fxFn = fx.pathFn;
if (fxFn) {
fxFn.call(this, path, value, fx.effect);
}
}
}
}
Expand Down Expand Up @@ -320,6 +322,10 @@
}
},

_functionPathEffect: function(path, value, effect) {
Polymer.Bind._functionEffect.call(this, path, value, effect);
},

_pathMatchesEffect: function(path, effect) {
var effectArg = effect.trigger.name;
return (effectArg == path) ||
Expand Down
4 changes: 4 additions & 0 deletions test/unit/bind-elements.html
Original file line number Diff line number Diff line change
Expand Up @@ -713,3 +713,7 @@
});
</script>
</dom-module>

<script>
Polymer({is: 'x-custom-effect'});
</script>
143 changes: 143 additions & 0 deletions test/unit/bind.html
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,149 @@
});
});

suite('custom user effects', function() {

test('Add custom effect', function() {
var el = document.createElement('x-custom-effect');

var called = 0;
el.addPropertyEffect('foo', function(path, value, old) {
called += 1;
assert.equal(path, 'foo');
assert.equal(value, 'bar');
assert.equal(old, undefined);
});

el.foo = 'bar';
assert.equal(called, 1);
});

test('Remove custom effect', function() {
var el = document.createElement('x-custom-effect');

var called = 0;
var fx = el.addPropertyEffect('foo', function() {
called += 1;
});

el.removePropertyEffect(fx);

el.foo = 'bar';
assert.equal(called, 0);
});

test('Ensure old values are sent', function() {
var el = document.createElement('x-custom-effect');
el.foo = 'bar';

var called = 0;
el.addPropertyEffect('foo', function(path, value, old) {
called += 1;
assert.equal(old, 'bar');
});

el.foo = 'quux';
assert.equal(called, 1);
});

test('Ensure path effects can be seen', function() {
var el = document.createElement('x-custom-effect');
el.foo = {bar: 'quux'};

var called = 0;
el.addPropertyEffect('foo', function(path, value, old) {
called += 1;
assert.equal(path, 'foo.bar');
assert.equal(value, 'quod');
assert.equal(old, undefined); // always undefined for structured paths!
});

el.set('foo.bar', 'quod');
assert.equal(called, 1);
});

test('Ensure independence', function() {
var el1 = document.createElement('x-custom-effect');
var called = 0;
el1.addPropertyEffect('foo', function() {
called += 1;
});

var el2 = document.createElement('x-custom-effect');

el2.foo = 'bar';
assert.equal(called, 0);
});

test('Ensure independence (property effects already created on prototype)',
function() {
Polymer({
is: 'x-custom-effect2',
properties: {
foo: {
observer: '_foo'
}
},
_foo: function(){}
});
var el1 = document.createElement('x-custom-effect2');

var called = 0;
el1.addPropertyEffect('foo', function() {
called += 1;
});
el1.foo = 'bar';
assert.equal(called, 1);

var el2 = document.createElement('x-custom-effect2');
el2.foo = 'bar';
assert.equal(called, 1);
});

test('Removing an effect as a side-effect does not break the running ' +
'effect loop', function() {
var el = document.createElement('x-custom-effect');

var called = 0;
var fx = el.addPropertyEffect('foo', function() {
el.removePropertyEffect(fx);
});
el.addPropertyEffect('foo', function() {
called += 1;
});

el.foo = 'bar';
assert.equal(called, 1);
});

test('Adding an effect as a side-effect does not invoke it immediately',
function() {
var el = document.createElement('x-custom-effect');

var called = 0;
var firstRun = true;
el.addPropertyEffect('foo', function() {
if (firstRun) {
el.addPropertyEffect('foo', function() {
called += 1;
});
firstRun = false;
}
});

el.foo = 'bar';
assert.equal(firstRun, false);
assert.equal(called, 0);

el.foo = 'quod';
assert.equal(called, 1);


});


});

</script>

</body>
Expand Down