Skip to content
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

Remove css-tag stylesheet caching by css text #2978

Merged
merged 4 commits into from Jun 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .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 and Firefox > 101 only.
51 changes: 39 additions & 12 deletions packages/reactive-element/src/css-tag.ts
Expand Up @@ -31,7 +31,7 @@ export type CSSResultGroup = CSSResultOrNative | CSSResultArray;

const constructionToken = Symbol();

const styleSheetCache = new Map<string, CSSStyleSheet>();
const cssTagCache = new WeakMap<TemplateStringsArray, CSSStyleSheet>();

/**
* A container for a string of CSS text, that may be used to create a CSSStyleSheet.
Expand All @@ -44,25 +44,43 @@ export class CSSResult {
// This property needs to remain unminified.
['_$cssResult$'] = true;
readonly cssText: string;

private constructor(cssText: string, safeToken: symbol) {
private _styleSheet?: CSSStyleSheet;
private _strings: TemplateStringsArray | undefined;

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;
}

// 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` is true then we assume CSSStyleSheet is
// constructable.
let styleSheet = this._styleSheet;
const strings = this._strings;
if (supportsAdoptingStyleSheets && styleSheet === undefined) {
styleSheetCache.set(this.cssText, (styleSheet = new CSSStyleSheet()));
styleSheet.replaceSync(this.cssText);
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 styleSheet;
}
Expand All @@ -73,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) => {
Expand Down Expand Up @@ -101,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
);

Expand All @@ -123,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
);
};

/**
Expand Down
71 changes: 35 additions & 36 deletions packages/reactive-element/src/test/css-tag_test.ts
Expand Up @@ -4,56 +4,58 @@
* 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', () => {
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`;
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();
// Equal because we cache stylesheets based on TemplateStringArrays
assert.strictEqual(style1.styleSheet, style2.styleSheet);
} else {
assert.isUndefined(style1.styleSheet);
}
});

test('css with same values always produce the same stylesheet', () => {
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`}`;
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
test('unsafeCSS() always produces a new stylesheet', () => {
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 () => {
Expand All @@ -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 () => {
Expand Down