-
Notifications
You must be signed in to change notification settings - Fork 168
Primitive shadow parts support #329
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
Changes from 5 commits
3922e3e
4e29618
441c0de
64b9e6b
50f934a
afae643
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{ | ||
"singleQuote": true, | ||
"bracketSpacing": false | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,9 +10,10 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN | |
|
||
'use strict'; | ||
|
||
import {StyleNode} from './css-parse.js'; // eslint-disable-line no-unused-vars | ||
import {StyleNode, parse} from './css-parse.js'; // eslint-disable-line no-unused-vars | ||
import * as StyleUtil from './style-util.js'; | ||
import {nativeShadow} from './style-settings.js'; | ||
import {nativeShadow, disableShadowParts} from './style-settings.js'; | ||
import {parsePartSelector, formatShadyPartSelector} from './shadow-parts.js'; | ||
|
||
/* Transforms ShadowDOM styling into ShadyDOM styling | ||
|
||
|
@@ -315,6 +316,12 @@ class StyleTransformer { | |
NTH, (m, type, inner) => `:${type}(${inner.replace(/\s/g, '')})`); | ||
selector = this._twiddleNthPlus(selector); | ||
} | ||
if (!disableShadowParts && PART.test(selector)) { | ||
// Hacky transform "::part(foo bar)" to "::part(foo,bar)" so that | ||
// SIMPLE_SELECTOR_SEP isn't confused by the spaces. | ||
// TODO(aomarks) Can we make SIMPLE_SELECTOR_SEP smarter instead? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice to work this out, but acknowledge that it may be non-trivial. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged. Leaving for now with TODO. |
||
selector = selector.replace(PART, (m) => m.replace(' ', ',')); | ||
} | ||
// Preserve selectors like `:-webkit-any` so that SIMPLE_SELECTOR_SEP does | ||
// not get confused by spaces inside the pseudo selector | ||
const isMatches = MATCHES.test(selector); | ||
|
@@ -350,6 +357,8 @@ class StyleTransformer { | |
let slottedIndex = selector.indexOf(SLOTTED); | ||
if (selector.indexOf(HOST) >= 0) { | ||
selector = this._transformHostSelector(selector, hostScope); | ||
} else if (!disableShadowParts && selector.match(PART)) { | ||
selector = this._transformPartSelector(selector, hostScope); | ||
// replace other selectors with scoping class | ||
} else if (slottedIndex !== 0) { | ||
selector = scope ? this._transformSimpleSelector(selector, scope) : | ||
|
@@ -396,6 +405,32 @@ class StyleTransformer { | |
return output.join(''); | ||
} | ||
|
||
/** | ||
* Transform a `::part` selector into a `shady-part` attribute selector. | ||
* | ||
* Example: | ||
* Given: 'parent > x-b.fancy::part(foo):hover' andd scope 'x-a' | ||
* Returns: 'parent > x-b.fancy [shady-part~="x-a:x-b:foo"]:hover' | ||
* | ||
* @param {!string} selector The `::part` selector. | ||
* @param {!string} scope Lowercase custom-element name of the scope that | ||
* defined this `::part` rule. | ||
* @return {!string} Transformed `shady-part` attribute selector. | ||
*/ | ||
_transformPartSelector(selector, scope) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doc with an example transformation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
const parsed = parsePartSelector(selector); | ||
if (parsed === null) { | ||
return selector; | ||
} | ||
const {combinators, elementName, selectors, parts, pseudos} = parsed; | ||
// Earlier we did a hacky transform from "part(foo bar)" to "part(foo,bar)" | ||
// so that the SIMPLE_SELECTOR regex didn't get confused by spaces. | ||
const partSelector = | ||
formatShadyPartSelector(scope, elementName, parts.replace(',', ' ')); | ||
return (scope === 'document' ? '' : scope + ' ') + | ||
`${combinators}${elementName}${selectors} ${partSelector}${pseudos}`; | ||
} | ||
|
||
// :host(...) -> scopeName... | ||
_transformHostSelector(selector, hostScope) { | ||
let m = selector.match(HOST_PAREN); | ||
|
@@ -457,6 +492,8 @@ class StyleTransformer { | |
return ''; | ||
} else if (selector.match(SLOTTED)) { | ||
return this._transformComplexSelector(selector, SCOPE_DOC_SELECTOR); | ||
} else if (!disableShadowParts && selector.match(PART)) { | ||
return this._transformPartSelector(selector, 'document'); | ||
} else { | ||
return this._transformSimpleSelector(selector.trim(), SCOPE_DOC_SELECTOR); | ||
} | ||
|
@@ -470,6 +507,7 @@ const SIMPLE_SELECTOR_SEP = /(^|[\s>+~]+)((?:\[.+?\]|[^\s>+~=[])+)/g; | |
const SIMPLE_SELECTOR_PREFIX = /[[.:#*]/; | ||
const HOST = ':host'; | ||
const ROOT = ':root'; | ||
const PART = /::part\([^)]*\)/; | ||
const SLOTTED = '::slotted'; | ||
const SLOTTED_START = new RegExp(`^(${SLOTTED})`); | ||
// NOTE: this supports 1 nested () pair for things like | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -134,4 +134,4 @@ export function treeVisitor(node, visitorFn) { | |
treeVisitor(n, visitorFn); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,10 @@ import {shadyDataForNode} from './shady-data.js'; | |
/** @type {!Object} */ | ||
export const settings = window['ShadyDOM'] || {}; | ||
|
||
/** @type {boolean} */ | ||
export const disableShadowParts = | ||
Boolean(window['ShadyCSS'] && window['ShadyCSS']['disableShadowParts']); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so, should it? If there's no |
||
|
||
settings.hasNativeShadowDOM = Boolean(Element.prototype.attachShadow && Node.prototype.getRootNode); | ||
|
||
// The user might need to pass the custom elements polyfill a flag by setting an | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<!DOCTYPE html> | ||
<!-- | ||
@license | ||
Copyright (c) 2020 The Polymer Project Authors. All rights reserved. | ||
This code may only be used under the BSD style license found at http://polymer.github.io/LICENSE.txt | ||
The complete set of authors may be found at http://polymer.github.io/AUTHORS.txt | ||
The complete set of contributors may be found at http://polymer.github.io/CONTRIBUTORS.txt | ||
Code distributed by Google as part of the polymer project is also | ||
subject to an additional IP rights grant found at http://polymer.github.io/PATENTS.txt | ||
--> | ||
|
||
<title>shadycss/shadow-parts/all-native</title> | ||
|
||
<script src="../../node_modules/wct-browser-legacy/browser.js"></script> | ||
|
||
<!-- This suite is just for more convenient local test execution. --> | ||
<script type="module"> | ||
import {suites} from './suites'; | ||
|
||
WCT.loadSuites(suites); | ||
</script> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
<!DOCTYPE html> | ||
<!-- | ||
@license | ||
Copyright (c) 2020 The Polymer Project Authors. All rights reserved. | ||
This code may only be used under the BSD style license found at http://polymer.github.io/LICENSE.txt | ||
The complete set of authors may be found at http://polymer.github.io/AUTHORS.txt | ||
The complete set of contributors may be found at http://polymer.github.io/CONTRIBUTORS.txt | ||
Code distributed by Google as part of the polymer project is also | ||
subject to an additional IP rights grant found at http://polymer.github.io/PATENTS.txt | ||
--> | ||
|
||
<title>shadycss/shadow-parts/all-polyfilled</title> | ||
|
||
<script src="../../node_modules/wct-browser-legacy/browser.js"></script> | ||
|
||
<!-- This suite is just for more convenient local test execution. --> | ||
<script type="module"> | ||
import {suites} from './suites'; | ||
|
||
WCT.loadSuites( | ||
suites.map( | ||
(url) => | ||
url + "?wc-register=true&wc-shadydom=true&wc-shimcssproperties=true" | ||
) | ||
); | ||
</script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you consider whether shadow parts should be a part of the scoping shim or given separate opt-in support in ShadyCSS, like @apply or custom style interface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good thought, hadn't considered that. What are the main advantages you are thinking of?
The main one I can think of is that there would be more options for shipping smaller bundles to users who don't need shadow parts. We could create more permutations of the bundles, and update the loader to check the
disableShadowParts
flag, and only load shadow parts support when not true.Does this suggestion imply you think shadow parts should be opt-in rather than opt-out? I've been thinking we should do opt-out, because 1) shadow parts are so widely supported now that it feels like they are part of the core set of web components APIs, and 2) I think the performance cost to applications that don't end up using shadow parts is very small (since we will do almost nothing until a
::part
style is first detected in at least one template).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you've evaluated it correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed #339, will think about this a little more and refactor in a separate PR.