From 822b15fa0ba404ad017c3f600676a4aed85b7595 Mon Sep 17 00:00:00 2001 From: Russell Bicknell Date: Mon, 13 May 2019 15:20:16 -0700 Subject: [PATCH 1/9] Scope styles even if the rendered value is not a TemplateResult. --- package.json | 2 +- src/env.d.ts | 4 ++ src/lib/shady-render.ts | 14 +++---- .../lib/shady-render-scoping-shim_test.ts | 41 +++++++++++++++++++ src/test/test-utils/shadow-root.ts | 3 +- test/index.html | 3 ++ test/shady-scoping-shim.html | 21 ++++++++++ 7 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 src/test/lib/shady-render-scoping-shim_test.ts create mode 100644 test/shady-scoping-shim.html diff --git a/package.json b/package.json index 28071fa8bb..fd9438d0a6 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,7 @@ "devDependencies": { "@types/chai": "^4.1.0", "@types/mocha": "^5.2.0", - "@webcomponents/shadycss": "^1.5.2", + "@webcomponents/shadycss": "^1.8.0", "@webcomponents/webcomponentsjs": "^2.0.4", "chai": "^4.1.2", "clang-format": "^1.2.4", diff --git a/src/env.d.ts b/src/env.d.ts index da0c99a6bc..9e693d9221 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -5,6 +5,10 @@ interface ShadyCSS { prepareTemplateDom(template: Element, elementName: string): void; prepareTemplateStyles( template: Element, elementName: string, typeExtension?: string): void; + ScopingShim: undefined|{ + prepareAdoptedCssText( + cssTextArray: Array, elementName: string): void; + }; } interface ShadyDOM { diff --git a/src/lib/shady-render.ts b/src/lib/shady-render.ts index 9bd3ee373a..4935fc4977 100644 --- a/src/lib/shady-render.ts +++ b/src/lib/shady-render.ts @@ -24,6 +24,7 @@ * Do not remove this comment; it keeps typedoc from misplacing the module * docs. */ +import {html} from '../lit-html.js'; import {removeNodes} from './dom.js'; import {insertNodeIntoTemplate, removeNodesFromTemplate} from './modify-template.js'; import {RenderOptions} from './render-options.js'; @@ -248,7 +249,7 @@ export const render = const hasRendered = parts.has(container); const needsScoping = compatibleShadyCSSVersion && container.nodeType === 11 /* Node.DOCUMENT_FRAGMENT_NODE */ && - !!(container as ShadowRoot).host && result instanceof TemplateResult; + !!(container as ShadowRoot).host; // Handle first render to a scope specially... const firstScopeRender = needsScoping && !shadyRenderSet.has(scopeName); // On first scope render, render into a fragment; this cannot be a single @@ -272,12 +273,11 @@ export const render = if (firstScopeRender) { const part = parts.get(renderContainer)!; parts.delete(renderContainer); - if (part.value instanceof TemplateInstance) { - prepareTemplateStyles( - renderContainer as DocumentFragment, - part.value.template, - scopeName); - } + const template = part.value instanceof TemplateInstance ? + part.value.template : + new Template(html``, document.createElement('template')); + prepareTemplateStyles( + renderContainer as DocumentFragment, template, scopeName); removeNodes(container, container.firstChild); container.appendChild(renderContainer); parts.set(container, part); diff --git a/src/test/lib/shady-render-scoping-shim_test.ts b/src/test/lib/shady-render-scoping-shim_test.ts new file mode 100644 index 0000000000..4a4c2d8109 --- /dev/null +++ b/src/test/lib/shady-render-scoping-shim_test.ts @@ -0,0 +1,41 @@ +/** + * @license + * Copyright (c) 2017 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 + */ + +import {renderShadowRoot} from '../test-utils/shadow-root.js'; + +const assert = chai.assert; + +suite('shady-render scoping shim', () => { + test('scoped styles are applied for non-TemplateResult values', function() { + if (typeof window.ShadyDOM === 'undefined' || !window.ShadyDOM.inUse) { + this.skip(); + return; + } + if (typeof window.ShadyCSS === 'undefined' || + window.ShadyCSS.nativeShadow || + window.ShadyCSS.ScopingShim === undefined) { + this.skip(); + return; + } + const container = document.createElement('scope-1'); + window.ShadyCSS.ScopingShim.prepareAdoptedCssText( + [':host { border-top: 2px solid black; }'], 'scope-1'); + document.body.appendChild(container); + renderShadowRoot(undefined, container); + assert.equal( + getComputedStyle(container).getPropertyValue('border-top-width').trim(), + '2px'); + document.body.removeChild(container); + }); +}); diff --git a/src/test/test-utils/shadow-root.ts b/src/test/test-utils/shadow-root.ts index 35fe86d805..4aebd137ef 100644 --- a/src/test/test-utils/shadow-root.ts +++ b/src/test/test-utils/shadow-root.ts @@ -12,12 +12,11 @@ * http://polymer.github.io/PATENTS.txt */ import {render} from '../../lib/shady-render.js'; -import {TemplateResult} from '../../lit-html.js'; /** * A helper for creating a shadowRoot on an element. */ -export const renderShadowRoot = (result: TemplateResult, element: Element) => { +export const renderShadowRoot = (result: unknown, element: Element) => { if (!element.shadowRoot) { element.attachShadow({mode: 'open'}); } diff --git a/test/index.html b/test/index.html index c6cf0c51a0..18f1488259 100644 --- a/test/index.html +++ b/test/index.html @@ -32,6 +32,9 @@ 'shady-apply.html', 'shady-apply.html?shadydom=true', 'shady-apply.html?shadydom=true&shimcssproperties=true', + 'shady-scoping-shim.html', + 'shady-scoping-shim.html?shadydom=true', + 'shady-scoping-shim.html?shadydom=true&shimcssproperties=true', 'no-template.html', ]); diff --git a/test/shady-scoping-shim.html b/test/shady-scoping-shim.html new file mode 100644 index 0000000000..07d9ca0713 --- /dev/null +++ b/test/shady-scoping-shim.html @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + From 434aa5f97f7d1eab7a1d179b4727cfca041ddc81 Mon Sep 17 00:00:00 2001 From: Russell Bicknell Date: Mon, 13 May 2019 17:07:01 -0700 Subject: [PATCH 2/9] Add a comment about why an empty Template is passed to `prepareTemplateStyles`. --- src/lib/shady-render.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/lib/shady-render.ts b/src/lib/shady-render.ts index 4935fc4977..9e7a888060 100644 --- a/src/lib/shady-render.ts +++ b/src/lib/shady-render.ts @@ -273,6 +273,13 @@ export const render = if (firstScopeRender) { const part = parts.get(renderContainer)!; parts.delete(renderContainer); + // ShadyCSS might have style sheets (e.g. from `prepareAdoptedCssText`) + // that should apply to `renderContainer` even if the rendered value is + // not a TemplateInstance. However, it will only insert scoped styles + // into the document if `prepareTemplateStyles` has already been called + // for the given scope name. `prepareTemplateStyles` requires a + // template element (and the local wrapper requires a Template), so we + // create an empty one here to satisfy it. const template = part.value instanceof TemplateInstance ? part.value.template : new Template(html``, document.createElement('template')); @@ -286,7 +293,7 @@ export const render = // initial render to this container. // This is needed whenever dynamic changes are made so it would be // safest to do every render; however, this would regress performance - // so we leave it up to the user to call `ShadyCSSS.styleElement` + // so we leave it up to the user to call `ShadyCSS.styleElement` // for dynamic changes. if (!hasRendered && needsScoping) { window.ShadyCSS!.styleElement((container as ShadowRoot).host); From 6e254e1fe55732bba7d832e35c64516b450736a3 Mon Sep 17 00:00:00 2001 From: Russell Bicknell Date: Tue, 14 May 2019 17:50:24 -0700 Subject: [PATCH 3/9] Test that adopted styles still apply after later renders if the first rendered value was not a TemplateInstance. --- .../lib/shady-render-scoping-shim_test.ts | 39 +++++++++++++++---- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/src/test/lib/shady-render-scoping-shim_test.ts b/src/test/lib/shady-render-scoping-shim_test.ts index 4a4c2d8109..5814499494 100644 --- a/src/test/lib/shady-render-scoping-shim_test.ts +++ b/src/test/lib/shady-render-scoping-shim_test.ts @@ -12,24 +12,25 @@ * http://polymer.github.io/PATENTS.txt */ +import {html} from '../../lib/shady-render.js'; import {renderShadowRoot} from '../test-utils/shadow-root.js'; const assert = chai.assert; suite('shady-render scoping shim', () => { - test('scoped styles are applied for non-TemplateResult values', function() { - if (typeof window.ShadyDOM === 'undefined' || !window.ShadyDOM.inUse) { - this.skip(); - return; - } - if (typeof window.ShadyCSS === 'undefined' || + setup(function() { + if (typeof window.ShadyDOM === 'undefined' || !window.ShadyDOM.inUse || + typeof window.ShadyCSS === 'undefined' || window.ShadyCSS.nativeShadow || window.ShadyCSS.ScopingShim === undefined) { this.skip(); return; } + }); + + test('scoped styles are applied for non-TemplateResult values', function() { const container = document.createElement('scope-1'); - window.ShadyCSS.ScopingShim.prepareAdoptedCssText( + window.ShadyCSS!.ScopingShim!.prepareAdoptedCssText( [':host { border-top: 2px solid black; }'], 'scope-1'); document.body.appendChild(container); renderShadowRoot(undefined, container); @@ -38,4 +39,28 @@ suite('shady-render scoping shim', () => { '2px'); document.body.removeChild(container); }); + + const testName = 'adopted CSS remains when rendering a TemplateResult ' + + 'after an initial non-TemplateResult'; + test(testName, function() { + const container = document.createElement('scope-2'); + window.ShadyCSS!.ScopingShim!.prepareAdoptedCssText( + [':host { border-top: 2px solid black; } button { font-size: 7px; } '], + 'scope-2'); + document.body.appendChild(container); + renderShadowRoot(undefined, container); + assert.equal( + getComputedStyle(container).getPropertyValue('border-top-width').trim(), + '2px'); + renderShadowRoot(html``, container); + assert.equal( + getComputedStyle(container).getPropertyValue('border-top-width').trim(), + '2px'); + assert.equal( + getComputedStyle(container.shadowRoot!.querySelector('button')!) + .getPropertyValue('font-size') + .trim(), + '7px'); + document.body.removeChild(container); + }); }); From 755fdc5701a28fb301b2c908a2f1fc93ec1e8152 Mon Sep 17 00:00:00 2001 From: Russell Bicknell Date: Wed, 15 May 2019 12:39:06 -0700 Subject: [PATCH 4/9] shady-render: `prepareTemplateStyles` no longer requires a Template. (...) - Adds a test that ShadyCSS mixins are processed when the first render is of a non-TemplateInstance value. - Adds a test that ShadyCSS scoping is applied when the first render is of a non-TemplateInstance value. --- src/lib/shady-render.ts | 26 ++++++++++------ src/test/lib/shady-render-apply_test.ts | 25 ++++++++++++++++ .../lib/shady-render-scoping-shim_test.ts | 30 +++++++++++++++++-- 3 files changed, 69 insertions(+), 12 deletions(-) diff --git a/src/lib/shady-render.ts b/src/lib/shady-render.ts index 9e7a888060..e2d552e96e 100644 --- a/src/lib/shady-render.ts +++ b/src/lib/shady-render.ts @@ -24,7 +24,6 @@ * Do not remove this comment; it keeps typedoc from misplacing the module * docs. */ -import {html} from '../lit-html.js'; import {removeNodes} from './dom.js'; import {insertNodeIntoTemplate, removeNodesFromTemplate} from './modify-template.js'; import {RenderOptions} from './render-options.js'; @@ -126,8 +125,13 @@ const shadyRenderSet = new Set(); * output. */ const prepareTemplateStyles = - (renderedDOM: DocumentFragment, template: Template, scopeName: string) => { + (scopeName: string, renderedDOM: DocumentFragment, template?: Template) => { shadyRenderSet.add(scopeName); + // If `renderedDOM` is stamped from a Template, then we need to edit that + // Template's underlying template element. Otherwise, we create one here + // to give to ShadyCSS, which still requires one while scoping. + const templateElement = + template ? template.element : document.createElement('template'); // Move styles out of rendered DOM and store. const styles = renderedDOM.querySelectorAll('style'); const {length} = styles; @@ -136,7 +140,7 @@ const prepareTemplateStyles = // Ensure prepareTemplateStyles is called to support adding // styles via `prepareAdoptedCssText` since that requires that // `prepareTemplateStyles` is called. - window.ShadyCSS!.prepareTemplateStyles(template.element, scopeName); + window.ShadyCSS!.prepareTemplateStyles(templateElement, scopeName); return; } const condensedStyle = document.createElement('style'); @@ -154,18 +158,22 @@ const prepareTemplateStyles = removeStylesFromLitTemplates(scopeName); // And then put the condensed style into the "root" template passed in as // `template`. - const content = template.element.content; - insertNodeIntoTemplate(template, condensedStyle, content.firstChild); + const content = templateElement.content; + if (template) { + insertNodeIntoTemplate(template, condensedStyle, content.firstChild); + } else { + content.insertBefore(condensedStyle, content.firstChild); + } // Note, it's important that ShadyCSS gets the template that `lit-html` // will actually render so that it can update the style inside when // needed (e.g. @apply native Shadow DOM case). - window.ShadyCSS!.prepareTemplateStyles(template.element, scopeName); + window.ShadyCSS!.prepareTemplateStyles(templateElement, scopeName); const style = content.querySelector('style'); if (window.ShadyCSS!.nativeShadow && style !== null) { // When in native Shadow DOM, ensure the style created by ShadyCSS is // included in initially rendered output (`renderedDOM`). renderedDOM.insertBefore(style.cloneNode(true), renderedDOM.firstChild); - } else { + } else if (template) { // When no style is left in the template, parts will be broken as a // result. To fix this, we put back the style node ShadyCSS removed // and then tell lit to remove that node from the template. @@ -282,9 +290,9 @@ export const render = // create an empty one here to satisfy it. const template = part.value instanceof TemplateInstance ? part.value.template : - new Template(html``, document.createElement('template')); + undefined; prepareTemplateStyles( - renderContainer as DocumentFragment, template, scopeName); + scopeName, renderContainer as DocumentFragment, template); removeNodes(container, container.firstChild); container.appendChild(renderContainer); parts.set(container, part); diff --git a/src/test/lib/shady-render-apply_test.ts b/src/test/lib/shady-render-apply_test.ts index 366c5d0e74..e2c45f0cbf 100644 --- a/src/test/lib/shady-render-apply_test.ts +++ b/src/test/lib/shady-render-apply_test.ts @@ -46,6 +46,31 @@ suite('shady-render @apply', () => { document.body.removeChild(container); }); + test('styles with mixins that are not in a TemplateInstance', function() { + const container = document.createElement('scope-6'); + document.body.appendChild(container); + const style = document.createElement('style'); + style.innerHTML = ` + :host { + --batch: { + border: 3px solid orange; + padding: 4px; + }; + } + div { + @apply --batch; + } + `; + const result = [style, htmlWithApply`
Testing...
`]; + renderShadowRoot(result, container); + const div = (container.shadowRoot!).querySelector('div'); + const computedStyle = getComputedStyle(div!); + assert.equal( + computedStyle.getPropertyValue('border-top-width').trim(), '3px'); + assert.equal(computedStyle.getPropertyValue('padding-top').trim(), '4px'); + document.body.removeChild(container); + }); + test( 'styles with css custom properties using @apply render in different contexts', async () => { diff --git a/src/test/lib/shady-render-scoping-shim_test.ts b/src/test/lib/shady-render-scoping-shim_test.ts index 5814499494..c180f6e808 100644 --- a/src/test/lib/shady-render-scoping-shim_test.ts +++ b/src/test/lib/shady-render-scoping-shim_test.ts @@ -28,7 +28,10 @@ suite('shady-render scoping shim', () => { } }); - test('scoped styles are applied for non-TemplateResult values', function() { + let testName; + + testName = 'scoped styles are applied for non-TemplateResult values'; + test(testName, function() { const container = document.createElement('scope-1'); window.ShadyCSS!.ScopingShim!.prepareAdoptedCssText( [':host { border-top: 2px solid black; }'], 'scope-1'); @@ -40,8 +43,8 @@ suite('shady-render scoping shim', () => { document.body.removeChild(container); }); - const testName = 'adopted CSS remains when rendering a TemplateResult ' + - 'after an initial non-TemplateResult'; + testName = 'adopted CSS remains when rendering a TemplateResult after an ' + + 'initial non-TemplateResult'; test(testName, function() { const container = document.createElement('scope-2'); window.ShadyCSS!.ScopingShim!.prepareAdoptedCssText( @@ -63,4 +66,25 @@ suite('shady-render scoping shim', () => { '7px'); document.body.removeChild(container); }); + + testName = 'Styles inserted in the initial render through NodeParts are ' + + 'scoped.'; + test(testName, function() { + const style = document.createElement('style'); + style.innerHTML = + ':host { border-top: 2px solid black; } button { font-size: 7px; }'; + const container = document.createElement('scope-3'); + document.body.appendChild(container); + renderShadowRoot( + html`${style}`, container); + assert.equal( + getComputedStyle(container).getPropertyValue('border-top-width').trim(), + '2px'); + assert.equal( + getComputedStyle(container.shadowRoot!.querySelector('button')!) + .getPropertyValue('font-size') + .trim(), + '7px'); + document.body.removeChild(container); + }); }); From 72c11e81a0e60f69b94a5eb8bf8abd20dfb1dec2 Mon Sep 17 00:00:00 2001 From: Russell Bicknell Date: Fri, 17 May 2019 18:19:09 -0700 Subject: [PATCH 5/9] Revert "Revert "Allow `render` to accept any type. (#904)" (#913)" This reverts commit 86412caeada2cdede6bda62fa195dd298319f664. --- src/lib/parts.ts | 7 +++--- src/lib/render.ts | 9 ++++--- src/lib/shady-render.ts | 2 +- src/test/lib/parts_test.ts | 47 +++++++++++++++++++++++++++++++++++++ src/test/lib/render_test.ts | 39 ++++++++++++++++++++++++++++++ 5 files changed, 95 insertions(+), 9 deletions(-) diff --git a/src/lib/parts.ts b/src/lib/parts.ts index 09874acaeb..d49ee9c838 100644 --- a/src/lib/parts.ts +++ b/src/lib/parts.ts @@ -245,15 +245,16 @@ export class NodePart implements Part { private __commitText(value: unknown): void { const node = this.startNode.nextSibling!; value = value == null ? '' : value; + const valueAsString: string = + typeof value === 'string' ? value : String(value); if (node === this.endNode.previousSibling && node.nodeType === 3 /* Node.TEXT_NODE */) { // If we only have a single text node between the markers, we can just // set its value, rather than replacing it. // TODO(justinfagnani): Can we just check if this.value is primitive? - (node as Text).data = value as string; + (node as Text).data = valueAsString; } else { - this.__commitNode(document.createTextNode( - typeof value === 'string' ? value : String(value))); + this.__commitNode(document.createTextNode(valueAsString)); } this.value = value; } diff --git a/src/lib/render.ts b/src/lib/render.ts index 03d23d277f..109a8918e0 100644 --- a/src/lib/render.ts +++ b/src/lib/render.ts @@ -20,18 +20,17 @@ import {removeNodes} from './dom.js'; import {NodePart} from './parts.js'; import {RenderOptions} from './render-options.js'; import {templateFactory} from './template-factory.js'; -import {TemplateResult} from './template-result.js'; export const parts = new WeakMap(); /** - * Renders a template to a container. + * Renders a template result or other value to a container. * * To update a container with new values, reevaluate the template literal and * call `render` with the new result. * - * @param result a TemplateResult created by evaluating a template tag like - * `html` or `svg`. + * @param result Any value renderable by NodePart - typically a TemplateResult + * created by evaluating a template tag like `html` or `svg`. * @param container A DOM parent to render to. The entire contents are either * replaced, or efficiently updated if the same result type was previous * rendered there. @@ -40,7 +39,7 @@ export const parts = new WeakMap(); * container, as those changes will not effect previously rendered DOM. */ export const render = - (result: TemplateResult, + (result: unknown, container: Element|DocumentFragment, options?: Partial) => { let part = parts.get(container); diff --git a/src/lib/shady-render.ts b/src/lib/shady-render.ts index 64e6267070..9bd3ee373a 100644 --- a/src/lib/shady-render.ts +++ b/src/lib/shady-render.ts @@ -241,7 +241,7 @@ export interface ShadyRenderOptions extends Partial { * supported. */ export const render = - (result: TemplateResult, + (result: unknown, container: Element|DocumentFragment|ShadowRoot, options: ShadyRenderOptions) => { const scopeName = options.scopeName; diff --git a/src/test/lib/parts_test.ts b/src/test/lib/parts_test.ts index 5b2f4d91ff..e3d887975e 100644 --- a/src/test/lib/parts_test.ts +++ b/src/test/lib/parts_test.ts @@ -124,6 +124,53 @@ suite('Parts', () => { assert.equal(stripExpressionMarkers(container.innerHTML), ''); }); + test('accepts a symbol', () => { + const sym = Symbol(); + part.setValue(sym); + part.commit(); + assert.equal(stripExpressionMarkers(container.innerHTML), String(sym)); + }); + + test('accepts a symbol with a description', () => { + const sym = Symbol('description!'); + part.setValue(sym); + part.commit(); + assert.equal(stripExpressionMarkers(container.innerHTML), String(sym)); + }); + + test('accepts a symbol on subsequent renders', () => { + const sym1 = Symbol(); + part.setValue(sym1); + part.commit(); + assert.equal(stripExpressionMarkers(container.innerHTML), String(sym1)); + + // If the previously rendered value caused a single text node to be + // created, then subsequent renders will try to update the existing text + // node by setting `.data`. If the new value is a symbol and it isn't + // explicitly converted with `String`, then this would throw. + const sym2 = Symbol('description!'); + part.setValue(sym2); + part.commit(); + assert.equal(stripExpressionMarkers(container.innerHTML), String(sym2)); + }); + + test('accepts an object', () => { + part.setValue({}); + part.commit(); + assert.equal( + stripExpressionMarkers(container.innerHTML), '[object Object]'); + }); + + test('accepts an object with a `toString` method', () => { + part.setValue({ + toString() { + return 'toString!'; + } + }); + part.commit(); + assert.equal(stripExpressionMarkers(container.innerHTML), 'toString!'); + }); + test('accepts a function', () => { const f = () => { throw new Error(); diff --git a/src/test/lib/render_test.ts b/src/test/lib/render_test.ts index fb31d19516..3341a769c6 100644 --- a/src/test/lib/render_test.ts +++ b/src/test/lib/render_test.ts @@ -1406,4 +1406,43 @@ suite('render()', () => { assert(container.innerHTML, '
'); }); }); + + // `render` directly passes the given value to `NodePart#setValue`, so these + // tests are really just a sanity check that they are accepted by `render`. + // Tests about rendering behavior for specific values should generally be + // grouped with those of `NodePart#setValue` and `#commit`. + suite('accepts types other than TemplateResult', () => { + test('accepts undefined', () => { + render(undefined, container); + assert.equal(stripExpressionMarkers(container.innerHTML), ''); + }); + + test('accepts a string', () => { + render('test', container); + assert.equal(stripExpressionMarkers(container.innerHTML), 'test'); + }); + + test('accepts an object', () => { + render({}, container); + assert.equal( + stripExpressionMarkers(container.innerHTML), '[object Object]'); + }); + + test('accepts an object with `toString`', () => { + render( + { + toString() { + return 'toString!'; + } + }, + container); + assert.equal(stripExpressionMarkers(container.innerHTML), 'toString!'); + }); + + test('accepts an symbol', () => { + const sym = Symbol('description!'); + render(sym, container); + assert.equal(stripExpressionMarkers(container.innerHTML), String(sym)); + }); + }); }); From 4b8d1509af344f5cf5fed52ea93b8fa3864c87ad Mon Sep 17 00:00:00 2001 From: Russell Bicknell Date: Wed, 5 Jun 2019 15:34:39 -0700 Subject: [PATCH 6/9] Add / update comments. --- src/lib/parts.ts | 2 ++ src/lib/shady-render.ts | 4 +--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/parts.ts b/src/lib/parts.ts index d49ee9c838..fbb4ad6d9c 100644 --- a/src/lib/parts.ts +++ b/src/lib/parts.ts @@ -245,6 +245,8 @@ export class NodePart implements Part { private __commitText(value: unknown): void { const node = this.startNode.nextSibling!; value = value == null ? '' : value; + // If `value` isn't already a string, we explicitly convert it here in case + // it can't be implicitly converted - i.e. it's a symbol. const valueAsString: string = typeof value === 'string' ? value : String(value); if (node === this.endNode.previousSibling && diff --git a/src/lib/shady-render.ts b/src/lib/shady-render.ts index 466cee8ed2..9bc179b87d 100644 --- a/src/lib/shady-render.ts +++ b/src/lib/shady-render.ts @@ -288,9 +288,7 @@ export const render = // that should apply to `renderContainer` even if the rendered value is // not a TemplateInstance. However, it will only insert scoped styles // into the document if `prepareTemplateStyles` has already been called - // for the given scope name. `prepareTemplateStyles` requires a - // template element (and the local wrapper requires a Template), so we - // create an empty one here to satisfy it. + // for the given scope name. const template = part.value instanceof TemplateInstance ? part.value.template : undefined; From 5d71c2394eefb7b0b358ff35e0df10334976367a Mon Sep 17 00:00:00 2001 From: Russell Bicknell Date: Wed, 5 Jun 2019 15:34:53 -0700 Subject: [PATCH 7/9] Test that `render` accepts a node. --- src/test/lib/render_test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/lib/render_test.ts b/src/test/lib/render_test.ts index 4f076feb19..a2e2904bb8 100644 --- a/src/test/lib/render_test.ts +++ b/src/test/lib/render_test.ts @@ -1447,5 +1447,14 @@ suite('render()', () => { render(sym, container); assert.equal(stripExpressionMarkers(container.innerHTML), String(sym)); }); + + test('accepts a node', () => { + const div = document.createElement('div'); + div.appendChild(document.createTextNode('text in the div')); + render(div, container); + assert.equal( + stripExpressionMarkers(container.innerHTML), + '
text in the div
'); + }); }); }); From bf1373b1e1e18c47c41a8b3c04cd2218b7ec3e60 Mon Sep 17 00:00:00 2001 From: Russell Bicknell Date: Wed, 5 Jun 2019 17:58:10 -0700 Subject: [PATCH 8/9] Add a comment about interaction with @apply. --- src/lib/shady-render.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/lib/shady-render.ts b/src/lib/shady-render.ts index 9bc179b87d..c83dd79700 100644 --- a/src/lib/shady-render.ts +++ b/src/lib/shady-render.ts @@ -140,6 +140,13 @@ const prepareTemplateStyles = // Ensure prepareTemplateStyles is called to support adding // styles via `prepareAdoptedCssText` since that requires that // `prepareTemplateStyles` is called. + // + // ShadyCSS will only update styles containing @apply in the template + // given to `prepareTemplateStyles`. If no lit Template was given, + // ShadyCSS will not be able to update uses of @apply in any relevant + // template. However, this is not a problem because we only create the + // template for the purpose of supporting `prepareAdoptedCssText`, + // which doesn't support @apply at all. window.ShadyCSS!.prepareTemplateStyles(templateElement, scopeName); return; } From 572c10377d3e1142af0ea0d5b5cf9b841870950f Mon Sep 17 00:00:00 2001 From: Russell Bicknell Date: Fri, 7 Jun 2019 13:26:11 -0700 Subject: [PATCH 9/9] Explicitly convert to boolean in conditions. --- src/lib/shady-render.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/shady-render.ts b/src/lib/shady-render.ts index c83dd79700..ddde69a24d 100644 --- a/src/lib/shady-render.ts +++ b/src/lib/shady-render.ts @@ -131,7 +131,7 @@ const prepareTemplateStyles = // Template's underlying template element. Otherwise, we create one here // to give to ShadyCSS, which still requires one while scoping. const templateElement = - template ? template.element : document.createElement('template'); + !!template ? template.element : document.createElement('template'); // Move styles out of rendered DOM and store. const styles = renderedDOM.querySelectorAll('style'); const {length} = styles; @@ -166,7 +166,7 @@ const prepareTemplateStyles = // And then put the condensed style into the "root" template passed in as // `template`. const content = templateElement.content; - if (template) { + if (!!template) { insertNodeIntoTemplate(template, condensedStyle, content.firstChild); } else { content.insertBefore(condensedStyle, content.firstChild); @@ -180,7 +180,7 @@ const prepareTemplateStyles = // When in native Shadow DOM, ensure the style created by ShadyCSS is // included in initially rendered output (`renderedDOM`). renderedDOM.insertBefore(style.cloneNode(true), renderedDOM.firstChild); - } else if (template) { + } else if (!!template) { // When no style is left in the template, parts will be broken as a // result. To fix this, we put back the style node ShadyCSS removed // and then tell lit to remove that node from the template.