From 498e23f77344053c4361429d16dff81c119f31d9 Mon Sep 17 00:00:00 2001 From: Daniel Freedman Date: Mon, 25 Jul 2016 17:33:40 -0700 Subject: [PATCH] Rerun Apply Shim when mixins with consumers are redefined 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 --- polymer.html | 3 +- src/lib/apply-shim.html | 33 +++++++--- src/lib/custom-style.html | 8 ++- src/standard/styling.html | 8 +-- src/standard/x-styling.html | 25 +++++++- test/unit/styling-cross-scope-apply.html | 77 ++++++++++++++++++++++++ 6 files changed, 134 insertions(+), 20 deletions(-) diff --git a/polymer.html b/polymer.html index 1ab47eac4a..5f4cbbc008 100644 --- a/polymer.html +++ b/polymer.html @@ -72,6 +72,7 @@ this._setupShady(); this._registerHost(); if (this._template) { + this._validateMixins(); // manage local dom this._poolContent(); // host stack @@ -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. diff --git a/src/lib/apply-shim.html b/src/lib/apply-shim.html index 19803c8d27..09b8f25a18 100644 --- a/src/lib/apply-shim.html +++ b/src/lib/apply-shim.html @@ -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) { @@ -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); } @@ -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) { @@ -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); diff --git a/src/lib/custom-style.html b/src/lib/custom-style.html index bad8dd0746..bb33fe6f68 100644 --- a/src/lib/custom-style.html +++ b/src/lib/custom-style.html @@ -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(); }, @@ -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. @@ -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 @@ -219,6 +220,7 @@ } } ); + applyShim.__currentElementProto = null; } // custom properties shimming // (if we use native custom properties, no need to apply any property shimming) @@ -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); diff --git a/src/standard/styling.html b/src/standard/styling.html index ffaa5cb8aa..fb2c9e20f4 100644 --- a/src/standard/styling.html +++ b/src/standard/styling.html @@ -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; } @@ -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(); diff --git a/src/standard/x-styling.html b/src/standard/x-styling.html index c9b84edf92..d4e5e4378d 100644 --- a/src/standard/x-styling.html +++ b/src/standard/x-styling.html @@ -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(); @@ -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; diff --git a/test/unit/styling-cross-scope-apply.html b/test/unit/styling-cross-scope-apply.html index 3aea37523a..6bd6547e90 100644 --- a/test/unit/styling-cross-scope-apply.html +++ b/test/unit/styling-cross-scope-apply.html @@ -447,6 +447,71 @@ + + + + + + + + + + + + + + + +