refactor: set adoptedStyleSheets only once per component connection (#9660)

Author: vursen

packages/vaadin-themable-mixin/src/css-property-observer.js

@@ -14,14 +14,36 @@ export class CSSPropertyObserver {
   #name;
   #callback;
   #properties = new Set();
+  #styleSheet;
+  #isConnected = false;
 
   constructor(root, name, callback) {
     this.#root = root;
     this.#name = name;
     this.#callback = callback;
+  }
+
+  #handleTransitionEvent(event) {
+    const { propertyName } = event;
+    if (this.#properties.has(propertyName)) {
+      this.#callback(propertyName);
+    }
+  }
+
+  observe(property) {
+    this.connect();
+
+    this.#properties.add(property);
+    this.#rootHost.style.setProperty(`--${this.#name}-props`, [...this.#properties].join(', '));
+  }
 
-    const styleSheet = new CSSStyleSheet();
-    styleSheet.replaceSync(`
+  connect() {
+    if (this.#isConnected) {
+      return;
+    }
+
+    this.#styleSheet = new CSSStyleSheet();
+    this.#styleSheet.replaceSync(`
       :is(:root, :host)::before {
         content: '' !important;
         position: absolute !important;
@@ -32,22 +54,24 @@ export class CSSPropertyObserver {
         transition-property: var(--${this.#name}-props) !important;
       }
     `);
-    this.#root.adoptedStyleSheets.unshift(styleSheet);
+    this.#root.adoptedStyleSheets.unshift(this.#styleSheet);
 
     this.#rootHost.addEventListener('transitionstart', (event) => this.#handleTransitionEvent(event));
     this.#rootHost.addEventListener('transitionend', (event) => this.#handleTransitionEvent(event));
-  }
 
-  #handleTransitionEvent(event) {
-    const { propertyName } = event;
-    if (this.#properties.has(propertyName)) {
-      this.#callback(propertyName);
-    }
+    this.#isConnected = true;
   }
 
-  observe(property) {
-    this.#properties.add(property);
-    this.#rootHost.style.setProperty(`--${this.#name}-props`, [...this.#properties].join(', '));
+  disconnect() {
+    this.#properties.clear();
+
+    this.#root.adoptedStyleSheets = this.#root.adoptedStyleSheets.filter((s) => s !== this.#styleSheet);
+
+    this.#rootHost.removeEventListener('transitionstart', this.#handleTransitionEvent);
+    this.#rootHost.removeEventListener('transitionend', this.#handleTransitionEvent);
+    this.#rootHost.style.removeProperty(`--${this.#name}-props`);
+
+    this.#isConnected = false;
   }
 
   get #rootHost() {

packages/vaadin-themable-mixin/src/lumo-injector.js

@@ -84,10 +84,16 @@ export class LumoInjector {
     this.#root = root;
     this.#cssPropertyObserver = new CSSPropertyObserver(this.#root, 'vaadin-lumo-injector', (propertyName) => {
       const tagName = propertyName.slice(2).replace('-lumo-inject', '');
-      this.#updateComponentStyleSheet(tagName);
+      this.#updateStyleSheet(tagName);
     });
   }
 
+  disconnect() {
+    this.#cssPropertyObserver.disconnect();
+    this.#styleSheetsByTag.clear();
+    this.#componentsByTag.values().forEach((components) => components.forEach(removeLumoStyleSheet));
+  }
+
   /**
    * Adds a component to the list of elements monitored for style injection.
    * If the styles have already been detected, they are injected into the
@@ -103,8 +109,15 @@ export class LumoInjector {
     this.#componentsByTag.set(tagName, this.#componentsByTag.get(tagName) ?? new Set());
     this.#componentsByTag.get(tagName).add(component);
 
-    this.#updateComponentStyleSheet(tagName);
+    const stylesheet = this.#styleSheetsByTag.get(tagName);
+    if (stylesheet) {
+      if (stylesheet.cssRules.length > 0) {
+        injectLumoStyleSheet(component, stylesheet);
+      }
+      return;
+    }
 
+    this.#initStyleSheet(tagName);
     this.#cssPropertyObserver.observe(lumoInjectPropName);
   }
 
@@ -121,17 +134,21 @@ export class LumoInjector {
     removeLumoStyleSheet(component);
   }
 
-  #updateComponentStyleSheet(tagName) {
+  #initStyleSheet(tagName) {
+    this.#styleSheetsByTag.set(tagName, new CSSStyleSheet());
+    this.#updateStyleSheet(tagName);
+  }
+
+  #updateStyleSheet(tagName) {
     const { tags, modules } = parseStyleSheets(this.#rootStyleSheets);
 
     const cssText = (tags.get(tagName) ?? [])
       .flatMap((moduleName) => modules.get(moduleName) ?? [])
       .map((rule) => rule.cssText)
       .join('\n');
 
-    const stylesheet = this.#styleSheetsByTag.get(tagName) ?? new CSSStyleSheet();
+    const stylesheet = this.#styleSheetsByTag.get(tagName);
     stylesheet.replaceSync(cssText);
-    this.#styleSheetsByTag.set(tagName, stylesheet);
 
     this.#componentsByTag.get(tagName)?.forEach((component) => {
       if (cssText) {

packages/vaadin-themable-mixin/test/lumo-injection-mixin.test.js

@@ -117,6 +117,11 @@ describe('Lumo injection', () => {
   }
 
   describe('in global scope', () => {
+    afterEach(() => {
+      document.__lumoInjector?.disconnect();
+      document.__lumoInjector = undefined;
+    });
+
     describe('styles added after element is connected', () => {
       beforeEach(async () => {
         element = fixtureSync('<test-foo></test-foo>');
@@ -237,6 +242,11 @@ describe('Lumo injection', () => {
       element = document.createElement('test-foo');
     });
 
+    afterEach(() => {
+      host.__lumoInjector?.disconnect();
+      host.__lumoInjector = undefined;
+    });
+
     describe('styles added after element is connected', () => {
       beforeEach(async () => {
         host.shadowRoot.appendChild(element);
@@ -413,6 +423,11 @@ describe('Lumo injection', () => {
       wrapper = document.createElement('test-bar');
     });
 
+    afterEach(() => {
+      host.__lumoInjector?.disconnect();
+      host.__lumoInjector = undefined;
+    });
+
     it('should inject matching styles added to parent shadow host', async () => {
       host.shadowRoot.appendChild(wrapper);
       await nextRender();
@@ -447,6 +462,11 @@ describe('Lumo injection', () => {
       content = element.shadowRoot.querySelector('[part="content"]');
     });
 
+    afterEach(() => {
+      document.__lumoInjector?.disconnect();
+      document.__lumoInjector = undefined;
+    });
+
     it('should inject matching styles for the extending component', async () => {
       const style = document.createElement('style');
       style.textContent = TEST_FOO_STYLES.replaceAll('foo', 'baz');
@@ -483,6 +503,8 @@ describe('Lumo injection', () => {
 
     afterEach(() => {
       style.remove();
+      document.__lumoInjector?.disconnect();
+      document.__lumoInjector = undefined;
     });
 
     it('should not remove styles from injected stylesheets when calling registerStyles()', () => {