Improving a web component, one step at a time

Earlier this month, Stefan Judis published a small web component that makes your text sparkle.

In the spirit of so-called HTML web components which apparently often comes with some sort of aversion for the shadow DOM, the element directly manipulates the light DOM. As a developer of web apps with heavy DOM manipulations, and lover of the platform, this feels weird to me as it could possibly break so many things: other code that manipulates the DOM and now sees new elements and could also change them, handling of disconnection and reconnection of the element (as most such elements modify their children in the connectedCallback without checking whether it had already been done), MutationObserver, etc.

The first thing that came to my mind was that shadow DOM, for all its drawbacks and bugs, was the perfect fit for such an element, and I wanted to update Stefan's element to use the shadow DOM instead. Then a couple days ago, Zach Leatherman published a similar element that makes it snow on its content, and I was pleased to see he used shadow DOM to encapsulate (hide) the snowflakes. That was the trigger for me to actually take the time to revisit Stefan's <sparkle-text> element, so here's a step by step of various improvements (in my opinion) I made.

Disclaimer before I begin: this not in any way a criticism of Stefan's work! On the contrary actually, it wouldn't have been possible without this prior work. I just want to show things that I think could be improved, and this is all very much subjective.

I'll link to commits in my fork without any (intermediate) demo, as all those changes don't have much impact on the element's behavior, as seen by a reader of the web page (if you're interested in what it changes when looked at through the DevTools, then clone the repository, run npm install, npm run start, then checkout each commit in turn), except in some specific situations. The final state is available here if you want to play with it in your DevTools.

Using shadow DOM

@@ -66,16 +62,21 @@ class SparklyText extends HTMLElement {
 `;
     let sheet = new CSSStyleSheet();
     sheet.replaceSync(css);
-    document.adoptedStyleSheets = [...document.adoptedStyleSheets, sheet];
-    _needsStyles = false;
+    this.shadowRoot.adoptedStyleSheets = [sheet];
   }

   connectedCallback() {
+    if (this.shadowRoot) {
+      return;
+    }
+
     this.#numberOfSparkles = parseInt(
       this.getAttribute("number-of-sparkles") || `${this.#numberOfSparkles}`,
       10
     );

+    this.attachShadow({ mode: "open" });
+    this.shadowRoot.append(document.createElement("slot"));
     this.generateCss();
     this.addSparkles();
   }
@@ -99,7 +100,7 @@ class SparklyText extends HTMLElement {
       Math.random() * 110 - 5
     }% - var(--_sparkle-base-size) / 2)`;

-    this.appendChild(sparkleWrapper);
+    this.shadowRoot.appendChild(sparkleWrapper);
     sparkleWrapper.addEventListener("animationend", () => {
       sparkleWrapper.remove();
     });

In Stefan's version, CSS is injected to the document, with a boolean to make sure it's done only once, and styles are scoped to .sparkle-wrapper descendants of the sparkle-text elements. With shadow DOM, we gain style encapsulation, so no need for that scoping, we can directly target .sparkle-wrapper and svg as they're in the shadow DOM, clearly separate from the HTML that had been authored. We need to do it for each element though (we'll improve that later), but we now need to make sure we initialize the shadow DOM only once instead (I'm going step by step, so leaving this in the connectedCallback).

As a side effect, this also fixes some edge-case bug where the CSS would apply styles to any descendant SVG of the element, whether a sparkle or not (this could have been fixed by only targetting SVG inside .sparkle-wrapper actually); and of course with shadow DOM encapsulation, page author styles won't affect the sparkles either.

Small performance improvements

Those are really small, and probably negligible, but I feel like they're good practice anyway so I didn't even bother measuring actually.

First, as said above, the CSS needs to be somehow injected into each element's shadow DOM, but the constructible stylesheet can actually be shared between all of them. I've thus split construction of the stylesheet with its adoption in the shadow DOM, and made sure construction was only made once. Again, to limit the changes, everything's still in the same method, just move inside an if (I think I would have personally constructed the stylesheet early, as soon as the script is loaded, rather than waiting for the element to actually be used; it probably doesn't make a huge difference).

   generateCss() {
-    const css = `…`;
-    let sheet = new CSSStyleSheet();
-    sheet.replaceSync(css);
+    if (!sheet) {
+      const css = `…`;
+      sheet = new CSSStyleSheet();
+      sheet.replaceSync(css);
+    }
     this.shadowRoot.adoptedStyleSheets = [sheet];
   }

Similarly, sparkles were created by innerHTML the SVG into each. I changed that to using cloneNode(true) on an element prepared only once.

   addSparkle() {
-    const sparkleWrapper = document.createElement("span");
-    sparkleWrapper.classList.add("sparkle-wrapper");
-    sparkleWrapper.innerHTML = this.#sparkleSvg;
+    if (!sparkleTemplate) {
+      sparkleTemplate = document.createElement("span");
+      sparkleTemplate.classList.add("sparkle-wrapper");
+      sparkleTemplate.innerHTML = this.#sparkleSvg;
+    }
+
+    const sparkleWrapper = sparkleTemplate.cloneNode(true);

We actually don't even need the wrapper element, we could directly use the SVG without wrapper.

Handling disconnection

The element uses chained timers (a setTimeout callback that itself ends up calling setTimeout with the same callback, again and again) to re-add sparkles at random intervals (removing the sparkles is done as soon as the animation ends; and all of this is done only if the user didn't configure their browser to prefer reduced motion).

If the element is removed from the DOM, this unnecessarily continues in the background and could create memory leaks (in addition to just doing unnecessary work). I started with a very small change: check whether the element is still connected to the DOM before calling adding the sparkle (and calling setTimeout again). It could have been better (for some definition of better) to track the timer IDs so we could call clearTimeout in disconnectedCallback, but I feel like that would be unnecessarily complex.

       const {matches:motionOK} = window.matchMedia('(prefers-reduced-motion: no-preference)');
-      if (motionOK) this.addSparkle();
+      if (motionOK && this.isConnected) this.addSparkle();

This handles disconnection (as could be done by any destructive change to the DOM, like navigating with Turbo or htmx, I'm not even talking about using the element in a JavaScript-heavy web app) but not reconnection though, and we've exited early from the connectedCallback to avoid initializing the element twice, so this change actually broke our component in these situations where it's moved around, or stashed and then reinserted. To fix that, we need to always call addSparkles in connectedCallback, so move all the rest into an if, that's actually as simple as that… except that when the user prefers reduced motion, sparkles are never removed, so they keep piling in each time the element is connected again. One way to handle that, without introducing our housekeeping of individual timers, is to just remove all sparkles on disconnection. Either that or conditionally add them in connectedCallback if either we're initializing the element (including attaching the shadow DOM) or the user doesn't prefer reduced motion. The difference between both approaches is in whether we want the small animation when the sparkles appear (and appearing at new random locations). I went with the latter.

This still doesn't handle the situation where prefers-reduced-motion changes while the element is displayed though: if it turns to no-preference, then sparkles will start animating (due to CSS) then disappear at the end of their animation (due to JS listening to the animationend event), and no other sparkle will be added (because the setTimeout chain would have been broken earlier). I don't feel like it's worthy enough of a fix for such an element but it's also rather easy to handle so let's do it: listen to the media query change and start the timers whenever the user no longer prefers reduced motion.

@@ -94,6 +94,19 @@ connectedCallback() {
       );
       this.addSparkles();
     }
+
+    motionOK.addEventListener("change", this.motionOkChange);
+  }
+
+  disconnectedCallback() {
+    motionOK.removeEventListener("change", this.motionOkChange);
+  }
+
+  // Declare as an arrow function to get the appropriate 'this'
+  motionOkChange = () => {
+    if (motionOK.matches) {
+      this.addSparkles();
+    }
   }

Browser compatibility

Constructible stylesheets aren't supported in Safari 16.3 and earlier (and possibly other browsers). To avoid the code failing and strange things (probably, I haven't tested) happening, I started by bailing out early if the browser doesn't support constructible stylesheets (the element would then just do nothing; I could have actually even avoided registering it at all). Fwiw, I borrowed the check from Zach's <snow-fall> which works this way already (thanks Zach). As an aside, it's a bit strange that the code assumed construtible stylesheets were available, but tested for the availability of the custom element registry 🤷

   connectedCallback() {
-    if (this.shadowRoot) {
+    // https://caniuse.com/mdn-api_cssstylesheet_replacesync
+    if (this.shadowRoot || !("replaceSync" in CSSStyleSheet.prototype)) {
       return;
     }

But Safari 16.3 and earlier still represent more than a third of users on macOS, and more than a quarter of users on iOS! (according to CanIUse) To widen browser support, I therefore added a workaround, which consists of injecting a <style> element in the shadow DOM. Contrary to the constructible stylesheet, styles cannot be shared by all elements though, as we've seen above, so we only conditionally fallback to that approach, and continue using a constructible stylesheet everywhere it's supported.

-      sheet = new CSSStyleSheet();
-      sheet.replaceSync(css);
+      if (supportsConstructibleStylesheets) {
+        sheet = new CSSStyleSheet();
+        sheet.replaceSync(css);
+      } else {
+        sheet = document.createElement("style");
+        sheet.textContent = css;
+      }
     }

-    this.shadowRoot.adoptedStyleSheets = [sheet];```


+    if (supportsConstructibleStylesheets) {
+      this.shadowRoot.adoptedStyleSheets = [sheet];
+    } else {
+      this.shadowRoot.append(sheet.cloneNode(true));
+    }

Other possible improvements

I stopped there but there's still room for improvement.

For instance, the number-of-sparkles attribute is read once when the element is connected, so changing the attribute afterwards won't have any effect (but will have if you disconnect and then reconnect the element). To handle that situation (if only because you don't control the order of initialization when that element is used within a JavaScript-heavy application with frameworks like React, Vue or Angular), one would have to listen to the attribute change and update the number of sparkles dynamically. This could be done either by removing all sparkles and recreating the correct number of them (with addSparkles()), but this would be a bit abrupt, or by reworking entirely how sparkles are managed so they could adapt dynamically (don't recreate a sparkle, let it expire, when changing the number of sparkles down, or create just as many sparkles as necessary when changing it up). I feel like this would bump complexity by an order of magnitude, so it's probably not worth it for such an element.

The number of sparkles could also be controlled by a property reflecting the attribute; that would make the element more similar to built-in elements. Once the above is in place, this hopefully shouldn't be too hard.

That number of sparkles is expected to be, well, a number, and is currently parsed with parseInt, but the code doesn't handle parsing errors and could set the number of sparkles to NaN. Maybe we'd prefer using the default value in this case, and similarly for a zero or negative value; basically defining the attribute as a number limited to only positive numbers with fallback.

All this added complexity is, to me, what separates so-called HTML web components from others: they're designed to be used from HTML markup and not (or rarely) manipulated afterwards, so shortcuts can be taken to keep them simple.

Still speaking of that number of sparkles, the timers that create new sparkles are entirely disconnected from the animation that also makes them disappear. The animation length is actually configurable through the --sparkly-text-animation-length CSS custom property, but the timers delay is not configurable (a random value between 2 and 3 seconds). This means that if we set the animation length to a higher value than 3 seconds, there will actually be more sparkles than the configured number, as new sparkles will be added before the previous one has disappeared. There are several ways to fix this (if we think it's a bug –this is debatable!– and is worth fixing): for instance we could use the Web Animations API to read the computed timing of the animation and compute the timer's delay based on this value. Or we could let the animation repeat and move the element on animationiteration, rather than remove it and add another, and to add some randomness it could be temporarily paused and then restarted if we wanted (with a timer of some random delay). The code would be much different, but not necessarily more complex.

Regarding the animation events (whether animationend like it is now, or possibly animationiteration), given that they bubble, they could be listened to on a single parent (the element itself –filtering out possible animations on light DOM children– or an intermediate element inserted to contain all sparkles). This could hopefully simplify the code handling each sparkle.

Last, but not least, the addSparkles and addSparkle methods could be made private, as there's no reason to expose them in the element's API.

Final words

Had I started from scratch, I probably wouldn't have written the element the same way. I tried to keep the changes small, one step at a time, rather than doing a big refactoring, or starting from scratch and comparing the outcome to the original, as my goal was to specifically show what I think could be improved and how it wouldn't necessarily involve big changes. Going farther, and/or possibly using a helper library (I have written earlier about their added value), is left as an exercise for the reader.