From 5b5631d4ee983ccd68ab3038539b45d9ce37217f Mon Sep 17 00:00:00 2001 From: Justin Fagnani Date: Wed, 1 Jun 2022 08:16:25 -0700 Subject: [PATCH 1/3] Remove css-tag stylesheet caching by css text --- .changeset/loud-dragons-hang.md | 7 ++ packages/reactive-element/src/css-tag.ts | 19 +++--- .../reactive-element/src/test/css-tag_test.ts | 65 +++++++++---------- 3 files changed, 47 insertions(+), 44 deletions(-) create mode 100644 .changeset/loud-dragons-hang.md diff --git a/.changeset/loud-dragons-hang.md b/.changeset/loud-dragons-hang.md new file mode 100644 index 0000000000..c8cf65172d --- /dev/null +++ b/.changeset/loud-dragons-hang.md @@ -0,0 +1,7 @@ +--- +'@lit/reactive-element': patch +'lit': patch +'lit-element': patch +--- + +Changed the caching behavior of the css`` template literal tag so that same-text styles do not share a CSSStyleSheet. Note that this may be a breaking change in some very unusual scenarios on Chromium browsers only. diff --git a/packages/reactive-element/src/css-tag.ts b/packages/reactive-element/src/css-tag.ts index 47a63fbdbb..9aacc3975f 100644 --- a/packages/reactive-element/src/css-tag.ts +++ b/packages/reactive-element/src/css-tag.ts @@ -31,8 +31,6 @@ export type CSSResultGroup = CSSResultOrNative | CSSResultArray; const constructionToken = Symbol(); -const styleSheetCache = new Map(); - /** * A container for a string of CSS text, that may be used to create a CSSStyleSheet. * @@ -44,6 +42,7 @@ export class CSSResult { // This property needs to remain unminified. ['_$cssResult$'] = true; readonly cssText: string; + private _styleSheet?: CSSStyleSheet; private constructor(cssText: string, safeToken: symbol) { if (safeToken !== constructionToken) { @@ -54,17 +53,15 @@ export class CSSResult { this.cssText = cssText; } - // Note, this is a getter so that it's lazy. In practice, this means - // stylesheets are not created until the first element instance is made. + // This is a getter so that it's lazy. In practice, this means stylesheets + // are not created until the first element instance is made. get styleSheet(): CSSStyleSheet | undefined { - // Note, if `supportsAdoptingStyleSheets` is true then we assume - // CSSStyleSheet is constructable. - let styleSheet = styleSheetCache.get(this.cssText); - if (supportsAdoptingStyleSheets && styleSheet === undefined) { - styleSheetCache.set(this.cssText, (styleSheet = new CSSStyleSheet())); - styleSheet.replaceSync(this.cssText); + // If `supportsAdoptingStyleSheets` is true then we assume CSSStyleSheet is + // constructable. + if (supportsAdoptingStyleSheets && this._styleSheet === undefined) { + (this._styleSheet = new CSSStyleSheet()).replaceSync(this.cssText); } - return styleSheet; + return this._styleSheet; } toString(): string { diff --git a/packages/reactive-element/src/test/css-tag_test.ts b/packages/reactive-element/src/test/css-tag_test.ts index 7d2737b33a..79539decea 100644 --- a/packages/reactive-element/src/test/css-tag_test.ts +++ b/packages/reactive-element/src/test/css-tag_test.ts @@ -4,7 +4,12 @@ * SPDX-License-Identifier: BSD-3-Clause */ -import {css, CSSResult, unsafeCSS} from '../css-tag.js'; +import { + css, + CSSResult, + unsafeCSS, + supportsAdoptingStyleSheets, +} from '../css-tag.js'; import {assert} from '@esm-bundle/chai'; suite('Styling', () => { @@ -14,46 +19,43 @@ suite('Styling', () => { const cssValue = css; const makeStyle = () => cssValue`foo`; const style1 = makeStyle(); - assert.equal( - (style1 as CSSResult).styleSheet, - (style1 as CSSResult).styleSheet - ); - const style2 = makeStyle(); - assert.equal( - (style1 as CSSResult).styleSheet, - (style2 as CSSResult).styleSheet - ); + if (supportsAdoptingStyleSheets) { + assert.isDefined(style1.styleSheet); + assert.strictEqual(style1.styleSheet, style1.styleSheet); + const style2 = makeStyle(); + assert.notStrictEqual(style1.styleSheet, style2.styleSheet); + } else { + assert.isUndefined(style1.styleSheet); + } }); - test('css with same values always produce the same stylesheet', () => { + test('css with same values produce unique stylesheets', () => { // Alias avoids syntax highlighting issues in editors const cssValue = css; const makeStyle = () => cssValue`background: ${cssValue`blue`}`; const style1 = makeStyle(); - assert.equal( - (style1 as CSSResult).styleSheet, - (style1 as CSSResult).styleSheet - ); - const style2 = makeStyle(); - assert.equal( - (style1 as CSSResult).styleSheet, - (style2 as CSSResult).styleSheet - ); + if (supportsAdoptingStyleSheets) { + assert.isDefined(style1.styleSheet); + assert.strictEqual(style1.styleSheet, style1.styleSheet); + const style2 = makeStyle(); + assert.notStrictEqual(style1.styleSheet, style2.styleSheet); + } else { + assert.isUndefined(style1.styleSheet); + } }); test('unsafeCSS() CSSResults always produce the same stylesheet', () => { // Alias avoids syntax highlighting issues in editors const makeStyle = () => unsafeCSS(`foo`); const style1 = makeStyle(); - assert.equal( - (style1 as CSSResult).styleSheet, - (style1 as CSSResult).styleSheet - ); - const style2 = makeStyle(); - assert.equal( - (style1 as CSSResult).styleSheet, - (style2 as CSSResult).styleSheet - ); + if (supportsAdoptingStyleSheets) { + assert.isDefined(style1.styleSheet); + assert.strictEqual(style1.styleSheet, style1.styleSheet); + const style2 = makeStyle(); + assert.notStrictEqual(style1.styleSheet, style2.styleSheet); + } else { + assert.isUndefined(style1.styleSheet); + } }); test('`css` get styles throws when unsafe values are used', async () => { @@ -75,10 +77,7 @@ suite('Styling', () => { margin: ${spacer * 2}px; } `; - assert.equal( - (result as CSSResult).cssText.replace(/\s/g, ''), - 'div{margin:4px;}' - ); + assert.equal(result.cssText.replace(/\s/g, ''), 'div{margin:4px;}'); }); test('`CSSResult` cannot be constructed', async () => { From b243b75d1041805ef5a17b7c7958105b006fcfae Mon Sep 17 00:00:00 2001 From: Justin Fagnani Date: Fri, 17 Jun 2022 14:17:14 -0700 Subject: [PATCH 2/3] Cache by TemplateStringsArray --- packages/reactive-element/src/css-tag.ts | 42 ++++++++++++++++--- .../reactive-element/src/test/css-tag_test.ts | 10 ++--- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/packages/reactive-element/src/css-tag.ts b/packages/reactive-element/src/css-tag.ts index 9aacc3975f..7b763d7b2a 100644 --- a/packages/reactive-element/src/css-tag.ts +++ b/packages/reactive-element/src/css-tag.ts @@ -31,6 +31,8 @@ export type CSSResultGroup = CSSResultOrNative | CSSResultArray; const constructionToken = Symbol(); +const cssTagCache = new WeakMap(); + /** * A container for a string of CSS text, that may be used to create a CSSStyleSheet. * @@ -43,14 +45,20 @@ export class CSSResult { ['_$cssResult$'] = true; readonly cssText: string; private _styleSheet?: CSSStyleSheet; + private _strings: TemplateStringsArray | undefined; - private constructor(cssText: string, safeToken: symbol) { + private constructor( + cssText: string, + strings: TemplateStringsArray | undefined, + safeToken: symbol + ) { if (safeToken !== constructionToken) { throw new Error( 'CSSResult is not constructable. Use `unsafeCSS` or `css` instead.' ); } this.cssText = cssText; + this._strings = strings; } // This is a getter so that it's lazy. In practice, this means stylesheets @@ -58,10 +66,23 @@ export class CSSResult { get styleSheet(): CSSStyleSheet | undefined { // If `supportsAdoptingStyleSheets` is true then we assume CSSStyleSheet is // constructable. - if (supportsAdoptingStyleSheets && this._styleSheet === undefined) { - (this._styleSheet = new CSSStyleSheet()).replaceSync(this.cssText); + let styleSheet = this._styleSheet; + const strings = this._strings; + if (supportsAdoptingStyleSheets && styleSheet === undefined) { + const cacheable = strings !== undefined && strings.length === 1; + if (cacheable) { + styleSheet = cssTagCache.get(strings); + } + if (styleSheet === undefined) { + (this._styleSheet = styleSheet = new CSSStyleSheet()).replaceSync( + this.cssText + ); + if (cacheable) { + cssTagCache.set(strings, styleSheet); + } + } } - return this._styleSheet; + return styleSheet; } toString(): string { @@ -70,7 +91,11 @@ export class CSSResult { } type ConstructableCSSResult = CSSResult & { - new (cssText: string, safeToken: symbol): CSSResult; + new ( + cssText: string, + strings: TemplateStringsArray | undefined, + safeToken: symbol + ): CSSResult; }; const textFromCSSResult = (value: CSSResultGroup | number) => { @@ -98,6 +123,7 @@ const textFromCSSResult = (value: CSSResultGroup | number) => { export const unsafeCSS = (value: unknown) => new (CSSResult as ConstructableCSSResult)( typeof value === 'string' ? value : String(value), + undefined, constructionToken ); @@ -120,7 +146,11 @@ export const css = ( (acc, v, idx) => acc + textFromCSSResult(v) + strings[idx + 1], strings[0] ); - return new (CSSResult as ConstructableCSSResult)(cssText, constructionToken); + return new (CSSResult as ConstructableCSSResult)( + cssText, + strings, + constructionToken + ); }; /** diff --git a/packages/reactive-element/src/test/css-tag_test.ts b/packages/reactive-element/src/test/css-tag_test.ts index 79539decea..2dd24e15cd 100644 --- a/packages/reactive-element/src/test/css-tag_test.ts +++ b/packages/reactive-element/src/test/css-tag_test.ts @@ -14,7 +14,7 @@ import {assert} from '@esm-bundle/chai'; suite('Styling', () => { suite('css tag', () => { - test('CSSResults always produce the same stylesheet', () => { + test('stylesheet from same template literal without expressions are cached', () => { // Alias avoids syntax highlighting issues in editors const cssValue = css; const makeStyle = () => cssValue`foo`; @@ -23,13 +23,14 @@ suite('Styling', () => { assert.isDefined(style1.styleSheet); assert.strictEqual(style1.styleSheet, style1.styleSheet); const style2 = makeStyle(); - assert.notStrictEqual(style1.styleSheet, style2.styleSheet); + // Equal because we cache stylesheets based on TemplateStringArrays + assert.strictEqual(style1.styleSheet, style2.styleSheet); } else { assert.isUndefined(style1.styleSheet); } }); - test('css with same values produce unique stylesheets', () => { + test('stylesheet from same template literal with expressions are not cached', () => { // Alias avoids syntax highlighting issues in editors const cssValue = css; const makeStyle = () => cssValue`background: ${cssValue`blue`}`; @@ -44,8 +45,7 @@ suite('Styling', () => { } }); - test('unsafeCSS() CSSResults always produce the same stylesheet', () => { - // Alias avoids syntax highlighting issues in editors + test('unsafeCSS() always produces a new stylesheet', () => { const makeStyle = () => unsafeCSS(`foo`); const style1 = makeStyle(); if (supportsAdoptingStyleSheets) { From da13ee2bbebca434a0f5ad63f755069e1cf0d4d8 Mon Sep 17 00:00:00 2001 From: Justin Fagnani Date: Fri, 17 Jun 2022 14:20:04 -0700 Subject: [PATCH 3/3] pdate change description --- .changeset/loud-dragons-hang.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/loud-dragons-hang.md b/.changeset/loud-dragons-hang.md index c8cf65172d..daca17cac5 100644 --- a/.changeset/loud-dragons-hang.md +++ b/.changeset/loud-dragons-hang.md @@ -4,4 +4,4 @@ 'lit-element': patch --- -Changed the caching behavior of the css`` template literal tag so that same-text styles do not share a CSSStyleSheet. Note that this may be a breaking change in some very unusual scenarios on Chromium browsers only. +Changed the caching behavior of the css`` template literal tag so that same-text styles do not share a CSSStyleSheet. Note that this may be a breaking change in some very unusual scenarios on Chromium and Firefox > 101 only.