From a44d019e62be1c71c50edb7f2d166d039ebc30b5 Mon Sep 17 00:00:00 2001 From: Josh <12938082+joshwooding@users.noreply.github.com> Date: Wed, 15 Jul 2026 14:47:48 +0100 Subject: [PATCH 1/2] Improve component CSS injection lifecycle --- .../__tests__/__e2e__/useStyleInject.cy.tsx | 203 +++++++++++++++++- .../use-style-injection/useStyleInjection.ts | 10 +- 2 files changed, 209 insertions(+), 4 deletions(-) diff --git a/packages/styles/src/__tests__/__e2e__/useStyleInject.cy.tsx b/packages/styles/src/__tests__/__e2e__/useStyleInject.cy.tsx index d7774f80de1..446c7f5c63b 100644 --- a/packages/styles/src/__tests__/__e2e__/useStyleInject.cy.tsx +++ b/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 ( +
+ + + {isFirstVisible && ( + + First + + )} + {isSecondVisible && ( + + Second + + )} +
+ ); +}; + +const StyleInjectionToggleTest = () => { + const [isStyleInjectionEnabled, setIsStyleInjectionEnabled] = useState(false); + + return ( +
+ + + + +
+ ); +}; + +const InsertionPointToggleTest = () => { + const [useSecondMarker, setUseSecondMarker] = useState(false); + const insertionPoint = document.querySelector( + useSecondMarker + ? '[data-marker="dynamic-example-two"]' + : '[data-marker="dynamic-example-one"]', + ); + + return ( +
+ + + + +
+ ); +}; + 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(); + + 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(); + + 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( +
+ +
, + ); + + 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(); + + 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); + }); + }); + }); +}); diff --git a/packages/styles/src/use-style-injection/useStyleInjection.ts b/packages/styles/src/use-style-injection/useStyleInjection.ts index 72eabd3fc7b..94b94953ed6 100644 --- a/packages/styles/src/use-style-injection/useStyleInjection.ts +++ b/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]); } From 431b3602bb08a5bff30636c2ab25d182863579ac Mon Sep 17 00:00:00 2001 From: Josh <12938082+joshwooding@users.noreply.github.com> Date: Wed, 15 Jul 2026 14:51:13 +0100 Subject: [PATCH 2/2] Add styles changeset --- .changeset/fresh-styles-inject.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fresh-styles-inject.md diff --git a/.changeset/fresh-styles-inject.md b/.changeset/fresh-styles-inject.md new file mode 100644 index 00000000000..f1ae4ef552a --- /dev/null +++ b/.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.