Improve component CSS injection lifecycle

.changeset/fresh-styles-inject.md

@@ -0,0 +1,5 @@
+---
+"@salt-ds/styles": patch
+---
+
+Improved component CSS injection to avoid redundant duplicate style writes, respond to style injection and insertion point changes, and clean up injected style state more defensively.

packages/styles/src/__tests__/__e2e__/useStyleInject.cy.tsx

@@ -1,5 +1,8 @@
 import { Button } from "@salt-ds/core";
-import { InsertionPointProvider } from "@salt-ds/styles";
+import {
+  InsertionPointProvider,
+  StyleInjectionProvider,
+} from "@salt-ds/styles";
 import { useState } from "react";
 
 import TestComponent from "./TestComponent";
@@ -39,6 +42,79 @@ const RemovableTest = () => {
   );
 };
 
+const SameCssRefCountTest = () => {
+  const [isFirstVisible, setIsFirstVisible] = useState(true);
+  const [isSecondVisible, setIsSecondVisible] = useState(true);
+
+  return (
+    <div>
+      <Button onClick={() => setIsFirstVisible(false)}>Remove first</Button>
+      <Button onClick={() => setIsSecondVisible(false)}>Remove second</Button>
+      {isFirstVisible && (
+        <TestComponent
+          className="TestComponent1"
+          injectionId="test-component"
+          injectionCss={testComponentCss1}
+        >
+          First
+        </TestComponent>
+      )}
+      {isSecondVisible && (
+        <TestComponent
+          className="TestComponent1"
+          injectionId="test-component"
+          injectionCss={testComponentCss1}
+        >
+          Second
+        </TestComponent>
+      )}
+    </div>
+  );
+};
+
+const StyleInjectionToggleTest = () => {
+  const [isStyleInjectionEnabled, setIsStyleInjectionEnabled] = useState(false);
+
+  return (
+    <div>
+      <Button onClick={() => setIsStyleInjectionEnabled((old) => !old)}>
+        Toggle style injection
+      </Button>
+      <StyleInjectionProvider value={isStyleInjectionEnabled}>
+        <TestComponent
+          className="TestComponent1"
+          injectionId="test-component"
+          injectionCss={testComponentCss1}
+        />
+      </StyleInjectionProvider>
+    </div>
+  );
+};
+
+const InsertionPointToggleTest = () => {
+  const [useSecondMarker, setUseSecondMarker] = useState(false);
+  const insertionPoint = document.querySelector(
+    useSecondMarker
+      ? '[data-marker="dynamic-example-two"]'
+      : '[data-marker="dynamic-example-one"]',
+  );
+
+  return (
+    <div>
+      <Button onClick={() => setUseSecondMarker(true)}>
+        Move insertion point
+      </Button>
+      <InsertionPointProvider insertionPoint={insertionPoint}>
+        <TestComponent
+          className="TestComponent1"
+          injectionId="test-component"
+          injectionCss={testComponentCss1}
+        />
+      </InsertionPointProvider>
+    </div>
+  );
+};
+
 describe("Given two components with the same injection ID but different css", () => {
   it("SHOULD inject both sets of css", () => {
     cy.mount(
@@ -107,6 +183,51 @@ describe("Given two components with no insertion ID but different css", () => {
   });
 });
 
+describe("Given two components with the same css", () => {
+  it("SHOULD share one style element until all component instances are removed", () => {
+    const SELECTOR = '[data-salt-style="test-component"]';
+
+    cy.mount(<SameCssRefCountTest />);
+
+    cy.get(SELECTOR).then((injectedStyles) => {
+      cy.wrap(injectedStyles.length).should("equal", 1);
+      cy.wrap(injectedStyles[0].innerHTML).should("equal", testComponentCss1);
+    });
+
+    cy.findByRole("button", { name: "Remove first" }).realClick();
+
+    cy.get(SELECTOR).then((injectedStyles) => {
+      cy.wrap(injectedStyles.length).should("equal", 1);
+      cy.wrap(injectedStyles[0].innerHTML).should("equal", testComponentCss1);
+    });
+
+    cy.findByRole("button", { name: "Remove second" }).realClick();
+
+    cy.get(SELECTOR).should("not.exist");
+  });
+});
+
+describe("Given style injection is toggled", () => {
+  it("SHOULD inject and remove styles when the provider value changes", () => {
+    const SELECTOR = '[data-salt-style="test-component"]';
+
+    cy.mount(<StyleInjectionToggleTest />);
+
+    cy.get(SELECTOR).should("not.exist");
+
+    cy.findByRole("button", { name: "Toggle style injection" }).realClick();
+
+    cy.get(SELECTOR).then((injectedStyles) => {
+      cy.wrap(injectedStyles.length).should("equal", 1);
+      cy.wrap(injectedStyles[0].innerHTML).should("equal", testComponentCss1);
+    });
+
+    cy.findByRole("button", { name: "Toggle style injection" }).realClick();
+
+    cy.get(SELECTOR).should("not.exist");
+  });
+});
+
 describe("Given a removed component which has injected css", () => {
   it("SHOULD remove the injected style elements", () => {
     cy.mount(
@@ -131,6 +252,36 @@ describe("Given a removed component which has injected css", () => {
   });
 });
 
+describe("Given an injected style element is removed outside React", () => {
+  it("SHOULD clean up without error and allow the style to be injected again", () => {
+    cy.mount(
+      <div>
+        <RemovableTest />
+      </div>,
+    );
+
+    const SELECTOR = '[data-salt-style="test-component"]';
+
+    cy.findByRole("button").realClick();
+
+    cy.get(SELECTOR).then((injectedStyles) => {
+      injectedStyles[0].remove();
+    });
+    cy.get(SELECTOR).should("not.exist");
+
+    cy.findByRole("button").realClick();
+
+    cy.get(SELECTOR).should("not.exist");
+
+    cy.findByRole("button").realClick();
+
+    cy.get(SELECTOR).then((injectedStyles) => {
+      cy.wrap(injectedStyles.length).should("equal", 1);
+      cy.wrap(injectedStyles[0].innerHTML).should("equal", testComponentCss1);
+    });
+  });
+});
+
 describe("Given an insertion point", () => {
   // insert a marker in the head that the InsertionPointProvider will use
   before(() => {
@@ -167,3 +318,53 @@ describe("Given an insertion point", () => {
     });
   });
 });
+
+describe("Given the insertion point changes", () => {
+  before(() => {
+    cy.get("html").then(() => {
+      document
+        .querySelectorAll('[data-marker^="dynamic-example"]')
+        .forEach((marker) => {
+          marker.remove();
+        });
+
+      const firstStyleMarker = document.createElement("meta");
+      firstStyleMarker.dataset.marker = "dynamic-example-one";
+      document.head.append(firstStyleMarker);
+
+      const secondStyleMarker = document.createElement("meta");
+      secondStyleMarker.dataset.marker = "dynamic-example-two";
+      document.head.append(secondStyleMarker);
+    });
+  });
+
+  it("SHOULD move the injected styles to the updated insertion point", () => {
+    cy.mount(<InsertionPointToggleTest />);
+
+    cy.get('[data-salt-style="test-component"]').then((injectedStyle) => {
+      cy.get('[data-marker="dynamic-example-one"]').then((marker) => {
+        cy.wrap(
+          injectedStyle[0].compareDocumentPosition(marker[0]) &
+            Node.DOCUMENT_POSITION_FOLLOWING,
+        ).should("equal", Node.DOCUMENT_POSITION_FOLLOWING);
+      });
+    });
+
+    cy.findByRole("button", { name: "Move insertion point" }).realClick();
+
+    cy.get('[data-salt-style="test-component"]').then((injectedStyle) => {
+      cy.get('[data-marker="dynamic-example-one"]').then((marker) => {
+        cy.wrap(
+          injectedStyle[0].compareDocumentPosition(marker[0]) &
+            Node.DOCUMENT_POSITION_PRECEDING,
+        ).should("equal", Node.DOCUMENT_POSITION_PRECEDING);
+      });
+      cy.get('[data-marker="dynamic-example-two"]').then((marker) => {
+        cy.wrap(
+          injectedStyle[0].compareDocumentPosition(marker[0]) &
+            Node.DOCUMENT_POSITION_FOLLOWING,
+        ).should("equal", Node.DOCUMENT_POSITION_FOLLOWING);
+      });
+    });
+  });
+});

packages/styles/src/use-style-injection/useStyleInjection.ts

@@ -56,7 +56,8 @@ export function useComponentCssInjection({
         insertionPoint || targetWindow.document.head.firstChild,
       );
     } else {
-      styleMap.styleElement.textContent = css;
+      // The map is keyed by the CSS string, so an existing style element
+      // already contains this CSS and only the reference count needs updating.
       styleMap.count++;
     }
     sheetsMap.set(css, styleMap);
@@ -68,11 +69,14 @@ export function useComponentCssInjection({
       if (styleMap?.styleElement) {
         styleMap.count--;
         if (styleMap.count < 1) {
-          targetWindow.document.head.removeChild(styleMap.styleElement);
+          styleMap.styleElement.remove();
           styleMap.styleElement = null;
           sheetsMap?.delete(css);
+          if (sheetsMap?.size === 0) {
+            windowSheetsMap.delete(targetWindow);
+          }
         }
       }
     };
-  }, [testId, css, targetWindow]);
+  }, [testId, css, targetWindow, styleInjectionEnabled, insertionPoint]);
 }