Refactor useStyles to sanitize auto-href prop

+ add a test case that fails without this change applied
acusti · Sep 1, 2024 · 51f77c7 · 51f77c7
1 parent 81549a6
commit 51f77c7
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 24 deletions.
diff --git a/packages/styling/src/Style.tsx b/packages/styling/src/Style.tsx
@@ -9,15 +9,15 @@ type Props = {
     precedence?: string;
 };
 
-const Style = ({ children, href, precedence = 'medium' }: Props) => {
+const Style = ({ children, href: _href, precedence = 'medium' }: Props) => {
     // Minify CSS styles by replacing consecutive whitespace (including \n) with ' '
-    const styles = useStyles(children);
+    const { href, styles } = useStyles(children, _href);
     // https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react/canary.d.ts
     // https://react.dev/reference/react-dom/components/style#props
     return (
         // @ts-expect-error @types/react is missing new <style> props
         // eslint-disable-next-line react/no-unknown-property
-        <style href={href ?? styles} precedence={precedence}>
+        <style href={href} precedence={precedence}>
             {styles}
         </style>
     );

diff --git a/packages/styling/src/useStyles.test.tsx b/packages/styling/src/useStyles.test.tsx
@@ -60,4 +60,11 @@ describe('@acusti/styling', () => {
             expect(styleRegistry.size).toBe(0);
         });
     });
+
+    it('should sanitize styles used as href prop if no href prop provided', () => {
+        render(<Style>{`div[data-foo-bar] { color: cyan; }`}</Style>);
+        // the two-dash attribute selector results in “Range out of order in character class”
+        // and render() fails with SyntaxError: Invalid regular expression if not sanitized
+        expect(true).toBeTruthy();
+    });
 });
diff --git a/packages/styling/src/useStyles.ts b/packages/styling/src/useStyles.ts
@@ -2,42 +2,58 @@ import { useEffect, useState } from 'react';
 
 import { minifyStyles } from './minifyStyles.js';
 
-type StyleRegistry = Map<string, { referenceCount: number; styles: string }>;
+type StyleRegistry = Map<
+    string,
+    { href: string; referenceCount: number; styles: string }
+>;
 
 const styleRegistry: StyleRegistry = new Map();
 
 export const getStyleRegistry = () => styleRegistry;
 
-export function useStyles(styles: string) {
-    const [minifiedStyles, setMinifiedStyles] = useState(() => {
-        if (!styles) return '';
+export function useStyles(styles: string, initialHref?: string) {
+    const [stylesItem, setStylesItem] = useState(() => {
+        if (!styles) return { href: '', referenceCount: 0, styles: '' };
 
-        let minified = '';
-        const existingStylesItem = styleRegistry.get(styles);
-        if (existingStylesItem) {
-            existingStylesItem.referenceCount++;
-            minified = existingStylesItem.styles;
+        const key = initialHref ?? styles;
+        let item = styleRegistry.get(key);
+
+        if (item) {
+            item.referenceCount++;
         } else {
-            minified = minifyStyles(styles);
-            styleRegistry.set(styles, { referenceCount: 1, styles: minified });
+            const minified = minifyStyles(styles);
+            item = {
+                href: sanitizeHref(initialHref ?? minified),
+                referenceCount: 1,
+                styles: minified,
+            };
+            styleRegistry.set(key, item);
         }
 
-        return minified;
+        return item;
     });
 
     useEffect(() => {
         if (!styles) return;
-        if (!styleRegistry.get(styles)) {
+
+        const key = initialHref ?? styles;
+
+        if (!styleRegistry.get(key)) {
             const minified = minifyStyles(styles);
-            styleRegistry.set(styles, { referenceCount: 1, styles: minified });
-            setMinifiedStyles(minified);
+            const item = {
+                href: sanitizeHref(initialHref ?? minified),
+                referenceCount: 1,
+                styles: minified,
+            };
+            styleRegistry.set(key, item);
+            setStylesItem(item);
         }
 
         return () => {
-            const stylesItem = styleRegistry.get(styles);
-            if (stylesItem) {
-                stylesItem.referenceCount--;
-                if (!stylesItem.referenceCount) {
+            const existingItem = styleRegistry.get(styles);
+            if (existingItem) {
+                existingItem.referenceCount--;
+                if (!existingItem.referenceCount) {
                     // TODO try scheduling this via setTimeout
                     // and add another referenceCount check
                     // to deal with instance where existing <Style>
@@ -46,13 +62,19 @@ export function useStyles(styles: string) {
                 }
             }
         };
-    }, [styles]);
+    }, [initialHref, styles]);
 
-    return minifiedStyles;
+    return stylesItem;
 }
 
 export default useStyles;
 
 export const clearRegistry = () => {
     styleRegistry.clear();
 };
+
+// Dashes in selectors in href prop create happy-dom / jsdom test errors:
+// Invalid regular expression (“Range out of order in character class”)
+function sanitizeHref(text: string) {
+    return text.replace(/-/g, '');
+}