Skip to content

Commit

Permalink
Rerun Apply Shim when mixins with consumers are redefined
Browse files Browse the repository at this point in the history
When element stamps, make its mixins are valid, and if not, run the
Apply Shim again.

Flag is stored on element prototype to make the "valid" check extremely
fast

Only new instances of consuming elements will be updated

Fixes #3802
  • Loading branch information
dfreedm committed Jul 26, 2016
1 parent 19d9f3c commit 498e23f
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 20 deletions.
3 changes: 2 additions & 1 deletion polymer.html
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
this._setupShady();
this._registerHost();
if (this._template) {
this._validateMixins();
// manage local dom
this._poolContent();
// host stack
Expand All @@ -88,7 +89,7 @@
// acquire instance behaviors
this._marshalBehaviors();
/*
TODO(sorvell): It's *slightly() more efficient to marshal attributes prior
TODO(sorvell): It's *slightly() more efficient to marshal attributes prior
to installing hostAttributes, but then hostAttributes must be separately
funneled to configure, which is cumbersome.
Since this delta seems hard to measure we will not bother atm.
Expand Down
33 changes: 23 additions & 10 deletions src/lib/apply-shim.html
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,15 @@
var MIXIN_VAR_SEP = '_-_';

// map of mixin to property names
// --foo: {border: 2px} -> (--foo, ['border'])
// --foo: {border: 2px} -> {properties: {(--foo, ['border'])}, dependants: {'element-name': proto}}
var mixinMap = {};

function mapSet(name, prop) {
function mapSet(name, props) {
name = name.trim();
mixinMap[name] = prop;
mixinMap[name] = {
properties: props,
dependants: {}
};
}

function mapGet(name) {
Expand Down Expand Up @@ -133,12 +136,17 @@
var mixinAsProperties = consumeCssProperties(valueMixin);
var prefix = matchText.slice(0, matchText.indexOf('--'));
var mixinValues = cssTextToMap(mixinAsProperties);
var oldProperties = mapGet(propertyName);
var combinedProps = mixinValues;
if (oldProperties) {
var mixinEntry = mapGet(propertyName);
if (mixinEntry) {
// NOTE: since we use mixin, the map of properties is updated here
// and this is what we want.
combinedProps = Polymer.Base.mixin(oldProperties, mixinValues);
combinedProps = Polymer.Base.mixin(mixinEntry.properties, mixinValues);
for (var elementName in mixinEntry.dependants) {
if (elementName !== ApplyShim.__currentElementProto.is) {
mixinEntry.dependants[elementName].__mixinsInvalid = true;
}
}
} else {
mapSet(propertyName, combinedProps);
}
Expand Down Expand Up @@ -170,10 +178,12 @@
function atApplyToCssProperties(mixinName, fallbacks) {
mixinName = mixinName.replace(APPLY_NAME_CLEAN, '');
var vars = [];
var mixinProperties = mapGet(mixinName);
if (mixinProperties) {
var mixinEntry = mapGet(mixinName);
if (mixinEntry) {
var currentProto = ApplyShim.__currentElementProto;
mixinEntry.dependants[currentProto.is] = currentProto;
var p, parts, f;
for (p in mixinProperties) {
for (p in mixinEntry.properties) {
f = fallbacks && fallbacks[p];
parts = [p, ': var(', mixinName, MIXIN_VAR_SEP, p];
if (f) {
Expand Down Expand Up @@ -214,8 +224,11 @@
var ApplyShim = {
_map: mixinMap,
_separator: MIXIN_VAR_SEP,
transform: function(styles) {
transform: function(styles, elementProto) {
this.__currentElementProto = elementProto;
styleUtil.forRulesInStyles(styles, this._boundTransformRule);
elementProto.__mixinsInvalid = false;
this.__currentElementProto = null;
},
transformRule: function(rule) {
rule.cssText = this.transformCssText(rule.parsedCssText);
Expand Down
8 changes: 5 additions & 3 deletions src/lib/custom-style.html
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@
// TODO(sorvell): we could only do this if and only if
// this.ownerDocument != document;
// however, if we do that, we also have to change the `attached`
// code to go at `_beforeAttached` time because this is when
// code to go at `_beforeAttached` time because this is when
// elements produce styles (otherwise this breaks @apply shim)
this._tryApply();
},
Expand All @@ -153,7 +153,7 @@
var e = this.__appliedElement;
// used applied element from HTMLImports polyfill or this
if (!settings.useNativeCSSProperties) {
// if default style properties exist when
// if default style properties exist when
// this element tries to apply styling then,
// it has been loaded async and needs to trigger a full updateStyles
// to guarantee properties it provides update correctly.
Expand Down Expand Up @@ -209,6 +209,7 @@
}
var styleRules = styleUtil.rulesForStyle(e);
if (!targetedBuild) {
applyShim.__currentElementProto = this.__proto__;
styleUtil.forEachRule(styleRules,
function(rule) {
// shim the selector for current runtime settings
Expand All @@ -219,6 +220,7 @@
}
}
);
applyShim.__currentElementProto = null;
}
// custom properties shimming
// (if we use native custom properties, no need to apply any property shimming)
Expand Down Expand Up @@ -256,7 +258,7 @@
_flushCustomProperties: function() {
// if this style has not yet applied at all and it was loaded asynchronously
// (detected by Polymer being ready when this element tried to apply), then
// do a full updateStyles to ensure that
// do a full updateStyles to ensure that
if (this.__needsUpdateStyles) {
this.__needsUpdateStyles = false;
updateDebouncer = debounce(updateDebouncer, this._updateStyles);
Expand Down
8 changes: 4 additions & 4 deletions src/standard/styling.html
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
// We can avoid *all* shimming if native properties are used
// and there is a shadow css build and we are using native shadow.
var hasTargetedCssBuild = styleUtil.isTargetedBuild(this.__cssBuild);
if (settings.useNativeCSSProperties && this.__cssBuild === 'shadow'
if (settings.useNativeCSSProperties && this.__cssBuild === 'shadow'
&& hasTargetedCssBuild) {
return;
}
Expand All @@ -72,12 +72,12 @@
// css is calculated.
// css build takes care of apply shim, so avoid doing this work.
if (settings.useNativeCSSProperties && !this.__cssBuild) {
applyShim.transform(this._styles);
applyShim.transform(this._styles, this);
}
// calculate element static styling (with a targeted build and native
// properties, there's only 1 style and no need to parse it!
var cssText = settings.useNativeCSSProperties && hasTargetedCssBuild ?
(this._styles.length && this._styles[0].textContent.trim()) :
var cssText = settings.useNativeCSSProperties && hasTargetedCssBuild ?
(this._styles.length && this._styles[0].textContent.trim()) :
styleTransformer.elementStyles(this);
// prepare to shim style properties.
this._prepStyleProperties();
Expand Down
25 changes: 23 additions & 2 deletions src/standard/x-styling.html
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,31 @@
this._ownStylePropertyNames.length);
},

_validateMixins: function() {
if (this.__mixinsInvalid) {
// rerun apply shim
Polymer.ApplyShim.transform(this._styles, this.__proto__);
var cssText = styleTransformer.elementStyles(this);
if (nativeShadow) {
// replace style in template
var templateStyle = this._template.content.querySelector('style');
if (templateStyle) {
templateStyle.textContent = cssText;
}
} else {
// replace scoped style
var shadyStyle = this._scopeStyle && this._scopeStyle.nextSibling;
if (shadyStyle) {
shadyStyle.textContent = cssText;
}
}
}
},

_beforeAttached: function() {
// note: do this once automatically,
// then requires calling `updateStyles`
if ((!this._scopeSelector || this.__stylePropertiesInvalid) &&
if ((!this._scopeSelector || this.__stylePropertiesInvalid) &&
this._needsStyleProperties()) {
this.__stylePropertiesInvalid = false;
this._updateStyleProperties();
Expand Down Expand Up @@ -273,7 +294,7 @@
} else {
this._styleProperties = null;
}
// if called when an element is not attached, invalidate
// if called when an element is not attached, invalidate
// styling by unsetting scopeSelector.
} else {
this.__stylePropertiesInvalid = true;
Expand Down
77 changes: 77 additions & 0 deletions test/unit/styling-cross-scope-apply.html
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,71 @@
</script>
</dom-module>

<dom-module id="x-consume-mixin">
<template>
<style>
:host {
@apply --above;
}
</style>
<span>X</span>
</template>
<script>
HTMLImports.whenReady(function() {
Polymer({
is: 'x-consume-mixin'
})
})
</script>
</dom-module>

<dom-module id="x-produce-mixin-1">
<template>
<style>
:host {
--above: {
color: rgb(255, 0, 0);
};
}
</style>
<x-consume-mixin id="child"></x-consume-mixin>
</template>
<script>
HTMLImports.whenReady(function() {
Polymer({
is: 'x-produce-mixin-1'
})
})
</script>
</dom-module>

<dom-module id="x-produce-mixin-2">
<template>
<style>
:host {
--above: {
background-color: rgb(0, 255, 0);
};
}
</style>
<x-consume-mixin id="child"></x-consume-mixin>
</template>
<script>
HTMLImports.whenReady(function() {
Polymer({
is: 'x-produce-mixin-2'
})
})
</script>
</dom-module>
<style is="custom-style">
:root {
--above: {
border: 8px solid blue;
}
}
</style>

<script>
suite('scoped-styling-apply', function() {
function assertComputed(element, value, property) {
Expand Down Expand Up @@ -616,6 +681,18 @@
assertComputed(div, '2px');
assertComputed(div, '0px', 'height');
});

test('Redefined mixins apply new properties for future elements', function() {
var parent1 = document.createElement('x-produce-mixin-1');
var parent2 = document.createElement('x-produce-mixin-2');
document.body.appendChild(parent1);
document.body.appendChild(parent2);
CustomElements.takeRecords();
assertComputed(parent1.$.child, 'rgb(255, 0, 0)', 'color');
assertComputed(parent2.$.child, 'rgb(0, 255, 0)', 'background-color');
assertComputed(parent1.$.child, '0px');
assertComputed(parent2.$.child, '0px');
});
});

</script>
Expand Down

0 comments on commit 498e23f

Please sign in to comment.