Skip to content

Shady Parts: improved support for mutations, plus small bugfixes #357

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

Merged
merged 10 commits into from
Jun 24, 2020
2 changes: 2 additions & 0 deletions packages/shadycss/externs/shadycss-externs.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
* flushCustomStyles: function(),
* getComputedStyleValue: function(!Element, string): string,
* onInsertBefore: function(!HTMLElement, !HTMLElement, ?HTMLElement): void,
* onSetAttribute: function(!HTMLElement, !string, !string): void,
* onRemoveAttribute: function(!HTMLElement, !string): void,
* ScopingShim: (Object|undefined),
* ApplyShim: (Object|undefined),
* CustomStyleInterface: (Object|undefined),
Expand Down
7 changes: 7 additions & 0 deletions packages/shadycss/src/common-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN

import { MIXIN_MATCH, VAR_ASSIGN } from './common-regex.js';

/**
* The prefix used by ShadyDOM for the native version of patched methods
* (e.g. `__shady_native_setAttribute`).
* @type {!string}
*/
export const NATIVE_PREFIX = '__shady_native_';

/**
* @param {Element} element
* @param {Object=} properties
Expand Down
40 changes: 40 additions & 0 deletions packages/shadycss/src/scoping-shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,46 @@ export default class ScopingShim {
}
}

/* eslint-disable no-unused-vars */

/**
* Hook for performing ShadyCSS updates when a relevant attribute is set.
* Note ShadyDOM will only call this function for "part" and "exportparts"
* attributes.
*
* @param {!HTMLElement} element Element
* @param {!string} name Attribute name.
* @param {!string} value Attribute value.
* @return {void}
*/
onSetAttribute(element, name, value) {
if (!disableShadowParts) {
if (name === 'part') {
shadowParts.onSetPartAttribute(element);
}
}
}

/* eslint-enable no-unused-vars */

/**
* Hook for performing ShadyCSS updates when a relevant attribute is removed.
* Note ShadyDOM will only call this function for "part" and "exportparts"
* attributes.
*
* @param {!HTMLElement} element Element
* @param {!string} name Attribute name ("part" or "exportparts")
* @param {!string} value Attribute value.
* @return {void}
*/
onRemoveAttribute(element, name, value) {
if (!disableShadowParts) {
if (name === 'part') {
shadowParts.onRemovePartAttribute(element);
}
}
}

/**
* Apply styles for the given element
*
Expand Down
121 changes: 96 additions & 25 deletions packages/shadycss/src/shadow-parts.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,30 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
import {StyleNode} from './css-parse.js';
import StyleInfo from './style-info.js';
import StyleProperties from './style-properties.js';
import * as StyleUtil from './style-util.js';
import {nativeCssVariables} from './style-settings.js';
import {NATIVE_PREFIX} from './common-utils.js';

/**
* Set the "shady-part" attribute using the native method.
*
* @param {!HTMLElement} element
* @param {!string} value
* @return {void}
*/
function setShadyPartAttribute(element, value) {
element[NATIVE_PREFIX + 'setAttribute']('shady-part', value);
}

/**
* Remove the "shady-part" attribute using the native method.
*
* @param {!HTMLElement} element
* @return {void}
*/
function removeShadyPartAttribute(element) {
element[NATIVE_PREFIX + 'removeAttribute']('shady-part');
}

/**
* Parse a CSS Shadow Parts "part" attribute into an array of part names.
Expand Down Expand Up @@ -71,9 +94,9 @@ export function parseExportPartsAttribute(attr) {
/**
* Regular expression to de-compose a ::part rule into interesting pieces. See
* parsePartSelector for description of pieces.
* [0 ][1 ][2 ] [3 ] [4 ]
* [0 ][1 ][2 ] [3 ] [4 ]
*/
const PART_REGEX = /(.*?)([a-z]+-\w+)([^\s]*?)::part\((.*)?\)(::?.*)?/;
const PART_REGEX = /(.*?)([a-z]+(?:-\w+)+)([^\s]*?)::part\((.*)?\)(::?.*)?/;

/**
* De-compose a ::part rule into interesting pieces.
Expand Down Expand Up @@ -299,6 +322,36 @@ export function findExportedPartMappings(host) {
return result;
}

/**
* Update the "shady-part" attribute when the "part" attribute is set.
*
* @param {!HTMLElement} element The element.
* @return {void}
*/
export function onSetPartAttribute(element) {
if (!element.getRootNode) {
// TODO(aomarks) We're in noPatch mode. Wrap where needed and add tests.
// https://github.com/webcomponents/polyfills/issues/343
return;
}
const root = element.getRootNode();
if (root === document || root === element) {
// Nowhere to receive part styles from.
return;
}
addShadyPartAttributes(root.host, [element]);
}

/**
* Remove the "shady-part" attribute when the "part" attribute is removed.
*
* @param {!HTMLElement} element The element.
* @return {void}
*/
export function onRemovePartAttribute(element) {
removeShadyPartAttribute(element);
}

/* eslint-disable no-unused-vars */
/**
* Add "shady-part" attributes to new nodes on insertion.
Expand All @@ -313,8 +366,8 @@ export function findExportedPartMappings(host) {
*/
export function onInsertBefore(parentNode, newNode, referenceNode) {
/* eslint-enable no-unused-vars */
if (newNode instanceof Text) {
// No parts in text.
if (newNode instanceof Text || newNode instanceof Comment) {
// No parts in text or comments.
return;
}
if (!parentNode.getRootNode) {
Expand Down Expand Up @@ -345,6 +398,18 @@ export function onInsertBefore(parentNode, newNode, referenceNode) {
if (parts.length === 0) {
return;
}
addShadyPartAttributes(host, parts);
}

/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

This does more than just add the attributes. Suggest adding explanation. Also find the name parts confusing for describing an array of elements.

Copy link
Contributor Author

@aomarks aomarks Jun 10, 2020

Choose a reason for hiding this comment

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

This does more than just add the attributes. Suggest adding explanation.

Done (styleShadowParts with new jsdoc description of exactly what it does).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also find the name parts confusing for describing an array of elements.

Done, renamed to partNodes. I also updated some other spots to try and use the variables partName and partNode consistently instead of part.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, I think that improves the readability a lot.

* Add "shady-part" attributes to the given part nodes. All part nodes are
* assumed to be in the same given host.
*
* @param {!HTMLElement} host The host element of the given parts.
* @param {!Array<!HTMLElement>} parts The new or changed part elements.
* @return {void}
*/
export function addShadyPartAttributes(host, parts) {
const hostScope = host.localName;
const superRoot = host.getRootNode();
const parentScope =
Expand All @@ -360,6 +425,9 @@ export function onInsertBefore(parentNode, newNode, referenceNode) {
const shadyParts = [];
const partNames = splitPartString(node.getAttribute('part'));
if (partNames.length === 0) {
// Remove the "shady-part" attribute to support the case of "part" getting set to
// the empty string or whitespace.
removeShadyPartAttribute(node);
continue;
}

Expand Down Expand Up @@ -389,7 +457,7 @@ export function onInsertBefore(parentNode, newNode, referenceNode) {
}
}
}
node.setAttribute('shady-part', shadyParts.join(' '));
setShadyPartAttribute(node, shadyParts.join(' '));

if (!nativeCssVariables) {
// Find the part rules that match this node and that consume custom
Expand Down Expand Up @@ -546,39 +614,42 @@ export function analyzeTemplatePartRules(providerScope, styleAst) {
return;
}
for (const styleNode of styleAst.rules) {
const selector = styleNode['selector'];
if (!selector || selector.indexOf('::part') === -1) {
const selectorList = styleNode['selector'];
if (!selectorList || selectorList.indexOf('::part') === -1) {
// TODO(aomarks) We do the `indexOf` check here to make the case where no
// `::part` rules are used is as fast as possible, but we could get away
// with just the `parsePartSelector` call we're doing next if the
// difference is negligible. Needs benchmarking.
continue;
}
const parsed = parsePartSelector(selector);
if (parsed === null) {
continue;
}
const consumedProperties = findConsumedCustomProperties(
styleNode['cssText']
);
if (consumedProperties.length === 0) {
continue;
}
const {elementName: receiverScope, parts} = parsed;
if (parts.length === 0) {
continue;
}
const key = [providerScope, receiverScope].join(':');
let entries = partRulesMap.get(key);
if (entries === undefined) {
entries = [];
partRulesMap.set(key, entries);
const selectors = StyleUtil.splitSelectorList(selectorList);
for (const selector of selectors) {
const parsed = parsePartSelector(selector);
if (parsed === null) {
continue;
}
const {elementName: receiverScope, parts} = parsed;
if (parts.length === 0) {
continue;
}
const key = [providerScope, receiverScope].join(':');
let entries = partRulesMap.get(key);
if (entries === undefined) {
entries = [];
partRulesMap.set(key, entries);
}
entries.push({
styleNode,
consumedProperties,
partNames: splitPartString(parts),
});
}
entries.push({
styleNode,
consumedProperties,
partNames: splitPartString(parts),
});
}
}

Expand Down
13 changes: 13 additions & 0 deletions packages/shadydom/src/patches/Element.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {scopeClassAttribute} from '../style-scoping.js';
import {shadyDataForNode} from '../shady-data.js';
import {attachShadow, ownerShadyRootForNode} from '../attach-shadow.js';
import {eventPropertyNamesForElement, wrappedDescriptorForEventProperty} from '../patch-events.js';
import {getScopingShim} from '../style-scoping.js';

const doc = window.document;

Expand Down Expand Up @@ -102,6 +103,12 @@ export const ElementPatches = utils.getOwnPropertyDescriptors({
} else if (!scopeClassAttribute(this, attr, value)) {
this[utils.NATIVE_PREFIX + 'setAttribute'](attr, value);
distributeAttributeChange(this, attr);
if (!utils.disableShadowParts && attr === 'part') {
const shim = getScopingShim();
if (shim) {
shim['onSetAttribute'](this, attr, value);
}
}
}
},

Expand All @@ -115,6 +122,12 @@ export const ElementPatches = utils.getOwnPropertyDescriptors({
} else if (!scopeClassAttribute(this, attr, '')) {
this[utils.NATIVE_PREFIX + 'removeAttribute'](attr);
distributeAttributeChange(this, attr);
if (!utils.disableShadowParts && attr === 'part') {
const shim = getScopingShim();
if (shim) {
shim['onRemoveAttribute'](this, attr);
}
}
} else if (this.getAttribute(attr) === '') {
// ensure that "class" attribute is fully removed if ShadyCSS does not keep scoping
this[utils.NATIVE_PREFIX + 'removeAttribute'](attr);
Expand Down
Loading