From 966d8659f9dcfdc9baa25a234fd4fa298e3961ba Mon Sep 17 00:00:00 2001
From: Andrew Patton <andrew@acusti.ca>
Date: Sun, 1 Sep 2024 09:53:19 -0700
Subject: [PATCH] Refactor <Style> component for react >=19
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

uses the new <style> element special rendering behavior and props introduced in react v19 to handle style de-duplication + SSR + render behavior (e.g. suspense while loading):
• https://react.dev/reference/react-dom/components/style#props
• https://react.dev/reference/react-dom/components/style#special-rendering-behavior
---
 packages/styling/package.json               |   4 +-
 packages/styling/src/Style.tsx              |  68 +++--------
 packages/styling/src/index.ts               |   1 +
 packages/styling/src/minifyStyles.test.ts   |  15 +++
 packages/styling/src/minifyStyles.ts        |   8 ++
 packages/styling/src/style-registry.test.ts | 129 --------------------
 packages/styling/src/style-registry.ts      | 115 -----------------
 packages/styling/src/useStyles.test.tsx     |  63 ++++++++++
 packages/styling/src/useStyles.ts           |  58 +++++++++
 9 files changed, 161 insertions(+), 300 deletions(-)
 create mode 100644 packages/styling/src/minifyStyles.test.ts
 create mode 100644 packages/styling/src/minifyStyles.ts
 delete mode 100644 packages/styling/src/style-registry.test.ts
 delete mode 100644 packages/styling/src/style-registry.ts
 create mode 100644 packages/styling/src/useStyles.test.tsx
 create mode 100644 packages/styling/src/useStyles.ts

diff --git a/packages/styling/package.json b/packages/styling/package.json
index 5d789a38..2d200356 100644
--- a/packages/styling/package.json
+++ b/packages/styling/package.json
@@ -48,7 +48,7 @@
         "vitest": "^1.1.0"
     },
     "peerDependencies": {
-        "react": "^16.8 || ^17 || ^18 || ^19.0.0-0",
-        "react-dom": "^16.8 || ^17 || ^18 || ^19.0.0-0"
+        "react": "^19.0.0-0",
+        "react-dom": "^19.0.0-0"
     }
 }
diff --git a/packages/styling/src/Style.tsx b/packages/styling/src/Style.tsx
index 00e4d6c4..c87d43e9 100644
--- a/packages/styling/src/Style.tsx
+++ b/packages/styling/src/Style.tsx
@@ -1,65 +1,25 @@
-import * as React from 'react';
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
+import React from 'react';
 
-import {
-    getRegisteredStyles,
-    registerStyles,
-    unregisterStyles,
-    updateStyles,
-} from './style-registry.js';
-
-const { useCallback, useEffect, useMemo, useRef, useState } = React;
+import { useStyles } from './useStyles.js';
 
 type Props = {
     children: string;
+    precedence?: string;
 };
 
-const Style = ({ children }: Props) => {
+const Style = ({ children, precedence = 'medium' }: Props) => {
     // Minify CSS styles by replacing consecutive whitespace (including \n) with ' '
-    const styles = useMemo(() => children.replace(/\s+/gm, ' '), [children]);
-    const [ownerDocument, setOwnerDocument] = useState<Document | null>(null);
-    const isMountedRef = useRef<boolean>(false);
-
-    useEffect(() => {
-        isMountedRef.current = true;
-        unregisterStyles({ ownerDocument: 'global', styles });
-    }, []); // eslint-disable-line react-hooks/exhaustive-deps
-
-    useEffect(
-        () => () => {
-            if (!ownerDocument) return;
-            unregisterStyles({ ownerDocument, styles });
-        },
-        [ownerDocument], // eslint-disable-line react-hooks/exhaustive-deps
+    const styles = useStyles(children);
+    // 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={styles} precedence={precedence}>
+            {styles}
+        </style>
     );
-
-    const previousStylesRef = useRef<string>('');
-
-    useEffect(() => {
-        if (!ownerDocument) return;
-
-        updateStyles({
-            ownerDocument,
-            previousStyles: previousStylesRef.current,
-            styles,
-        });
-
-        previousStylesRef.current = styles;
-    }, [ownerDocument, styles]);
-
-    const handleRef = useCallback((element: HTMLElement | null) => {
-        if (!element) return;
-        setOwnerDocument(element.ownerDocument);
-    }, []);
-
-    if (ownerDocument) return null;
-
-    // Avoid duplicate style rendering during SSR via style registry
-    if (!isMountedRef.current) {
-        if (getRegisteredStyles({ ownerDocument: 'global', styles })) return null;
-        registerStyles({ ownerDocument: 'global', styles });
-    }
-
-    return <style dangerouslySetInnerHTML={{ __html: styles }} ref={handleRef} />;
 };
 
 export default Style;
diff --git a/packages/styling/src/index.ts b/packages/styling/src/index.ts
index 6a5d851f..ef54deb9 100644
--- a/packages/styling/src/index.ts
+++ b/packages/styling/src/index.ts
@@ -1,3 +1,4 @@
+/// <reference types="react/canary" />
 export { default as Style } from './Style.js';
 
 export const SYSTEM_UI_FONT =
diff --git a/packages/styling/src/minifyStyles.test.ts b/packages/styling/src/minifyStyles.test.ts
new file mode 100644
index 00000000..9a12f580
--- /dev/null
+++ b/packages/styling/src/minifyStyles.test.ts
@@ -0,0 +1,15 @@
+import { describe, expect, it } from 'vitest';
+
+import { minifyStyles } from './minifyStyles.js';
+
+describe('@acusti/styling', () => {
+    describe('minifyStyles.ts', () => {
+        it("replaces consecutive whitespace (including \\n) with ' '", () => {
+            expect(
+                minifyStyles(`.foo {
+    color: red;
+}`),
+            ).toBe('.foo { color: red; }');
+        });
+    });
+});
diff --git a/packages/styling/src/minifyStyles.ts b/packages/styling/src/minifyStyles.ts
new file mode 100644
index 00000000..17fb9413
--- /dev/null
+++ b/packages/styling/src/minifyStyles.ts
@@ -0,0 +1,8 @@
+// TODO use techniques from https://github.com/jbleuzen/node-cssmin/blob/master/cssmin.js
+// (check https://github.com/jbleuzen/node-cssmin/pull/19/files also)
+export function minifyStyles(styles: string) {
+    // Minify CSS styles by replacing consecutive whitespace (including \n) with ' '
+    return styles.replace(/\s+/gm, ' ');
+}
+
+export default minifyStyles;
diff --git a/packages/styling/src/style-registry.test.ts b/packages/styling/src/style-registry.test.ts
deleted file mode 100644
index e9801831..00000000
--- a/packages/styling/src/style-registry.test.ts
+++ /dev/null
@@ -1,129 +0,0 @@
-// @vitest-environment happy-dom
-import { beforeEach, describe, expect, it } from 'vitest';
-
-import {
-    clearRegistry,
-    getRegisteredStyles,
-    getStyleRegistryKeys,
-    registerStyles,
-    unregisterStyles,
-    updateStyles,
-} from './style-registry.js';
-
-describe('@acusti/styling', () => {
-    describe('style-registry.ts', () => {
-        const mockStyles = '.test { color: red; }';
-
-        // reset styleRegistry before each test
-        beforeEach(clearRegistry);
-
-        describe('registerStyles', () => {
-            it('should add styles to the registry keyed by the style string', () => {
-                const payload = { ownerDocument: document, styles: mockStyles };
-                registerStyles(payload);
-                const styleRegistryKeys = getStyleRegistryKeys();
-                const keysArray = [...styleRegistryKeys];
-                expect(keysArray.length).toBe(1);
-                expect(keysArray[0]).toBe(mockStyles);
-                const result = getRegisteredStyles(payload);
-                expect(result!.element).toBeDefined();
-                expect(result!.referenceCount).toBe(1);
-            });
-
-            it('should allow registering styles without a DOM via ownerDocument: "global"', () => {
-                const payload = Object.freeze({
-                    ownerDocument: 'global',
-                    styles: mockStyles,
-                });
-                registerStyles(payload);
-                const registryKeys = [...getStyleRegistryKeys()];
-                expect(registryKeys.length).toBe(1);
-                expect(registryKeys[0]).toBe(mockStyles);
-                const result = getRegisteredStyles(payload);
-                expect(result!.element).toBeNull();
-                expect(result!.referenceCount).toBe(1);
-            });
-        });
-
-        describe('getRegisteredStyles', () => {
-            it('should retrieve registered styles', () => {
-                const payload = { ownerDocument: document, styles: mockStyles };
-                registerStyles(payload);
-                const result = getRegisteredStyles(payload);
-                expect(result!.element!.tagName).toBe('STYLE');
-                expect(result!.referenceCount).toBe(1);
-            });
-        });
-
-        describe('unregisterStyles', () => {
-            it('should remove styles from the registry if no other references to same styles exist', () => {
-                const payload = { ownerDocument: document, styles: mockStyles };
-                const otherPayload = Object.freeze({
-                    ownerDocument: 'global',
-                    styles: mockStyles,
-                });
-                registerStyles(payload);
-                registerStyles(otherPayload);
-                expect(getStyleRegistryKeys().next().value).toBe(mockStyles);
-                expect(getRegisteredStyles(payload)!.referenceCount).toBe(1);
-                expect(getRegisteredStyles(otherPayload)!.referenceCount).toBe(1);
-                unregisterStyles(payload);
-                // style registry for this style should still exist
-                expect(getStyleRegistryKeys().next().value).toBe(mockStyles);
-                // but this document’s styles item should be cleared
-                expect(getRegisteredStyles(payload)).toBeNull();
-                unregisterStyles(otherPayload);
-                // now the style registry should be empty
-                expect([...getStyleRegistryKeys()].length).toBe(0);
-            });
-        });
-
-        describe('updateStyles', () => {
-            it('should update styles in the registry, reusing the existing DOM element if no other reference', () => {
-                const previousStyles = '.previous { color: blue; }';
-                const payload = {
-                    ownerDocument: document,
-                    styles: previousStyles,
-                };
-                registerStyles(payload);
-                const previousStylesItem = getRegisteredStyles(payload)!;
-                expect(previousStylesItem.element!.innerText).toBe(previousStyles);
-                updateStyles({
-                    ownerDocument: document,
-                    previousStyles,
-                    styles: mockStyles,
-                });
-                const stylesItem = getRegisteredStyles({
-                    ownerDocument: document,
-                    styles: mockStyles,
-                })!;
-                expect(previousStylesItem.element).toBe(stylesItem.element);
-                expect(stylesItem.element!.innerText).toBe(mockStyles);
-                expect(getRegisteredStyles(payload)).toBeNull();
-            });
-
-            it('should update styles in the registry, creating a new DOM element if there are other references', () => {
-                const previousStyles = '.previous { color: blue; }';
-                const payload = { ownerDocument: document, styles: previousStyles };
-                registerStyles(payload);
-                // create multiple references to the same styles and document
-                registerStyles(payload);
-                const previousStylesItem = getRegisteredStyles(payload)!;
-                expect(previousStylesItem.referenceCount).toBe(2);
-                expect(previousStylesItem.element!.innerText).toBe(previousStyles);
-                updateStyles({
-                    ownerDocument: document,
-                    previousStyles,
-                    styles: mockStyles,
-                });
-                const stylesItem = getRegisteredStyles({
-                    ownerDocument: document,
-                    styles: mockStyles,
-                })!;
-                expect(previousStylesItem.element).not.toBe(stylesItem.element);
-                expect(stylesItem.element!.innerText).toBe(mockStyles);
-                expect(getRegisteredStyles(payload)!.referenceCount).toBe(1);
-            });
-        });
-    });
-});
diff --git a/packages/styling/src/style-registry.ts b/packages/styling/src/style-registry.ts
deleted file mode 100644
index 930ce434..00000000
--- a/packages/styling/src/style-registry.ts
+++ /dev/null
@@ -1,115 +0,0 @@
-type StyleRegistry = Map<
-    string,
-    Map<Document | 'global', { element: HTMLStyleElement | null; referenceCount: number }>
->;
-
-const styleRegistry: StyleRegistry = new Map();
-
-type Payload = { ownerDocument: Document | 'global'; styles: string };
-
-export const getRegisteredStyles = ({ ownerDocument, styles }: Payload) => {
-    if (!styles) return null;
-    const stylesMap = styleRegistry.get(styles);
-    if (!stylesMap) return null;
-    return stylesMap.get(ownerDocument) ?? null;
-};
-
-// NOTE a more idiomatic API than (register|unregister)Styles would be
-// to make registerStyles a thunk that returns a cleanup function
-export const registerStyles = ({ ownerDocument, styles }: Payload) => {
-    if (!styles) return;
-
-    const existingStylesItem = getRegisteredStyles({ ownerDocument, styles });
-    if (existingStylesItem) {
-        existingStylesItem.referenceCount++;
-        return;
-    }
-
-    if (ownerDocument === 'global') {
-        const stylesItem = { element: null, referenceCount: 1 };
-        let stylesMap = styleRegistry.get(styles);
-        if (stylesMap) {
-            stylesMap.set(ownerDocument, stylesItem);
-        } else {
-            stylesMap = new Map([[ownerDocument, stylesItem]]);
-        }
-        styleRegistry.set(styles, stylesMap);
-        return;
-    }
-
-    const element = ownerDocument.createElement('style');
-    element.setAttribute('data-ukt-styling', '');
-    element.innerHTML = styles;
-    ownerDocument.head.appendChild(element);
-    const stylesItem = { element, referenceCount: 1 };
-
-    const stylesMap = styleRegistry.get(styles);
-    if (stylesMap) {
-        stylesMap.set(ownerDocument, stylesItem);
-        return;
-    }
-
-    styleRegistry.set(styles, new Map([[ownerDocument, stylesItem]]));
-};
-
-export const unregisterStyles = ({ ownerDocument, styles }: Payload) => {
-    if (!styles) return;
-
-    const stylesItem = getRegisteredStyles({ ownerDocument, styles });
-    if (!stylesItem) return;
-
-    stylesItem.referenceCount--;
-    if (stylesItem.referenceCount) return;
-
-    // If no more references to these styles in this document, remove <style> element from the DOM
-    if (stylesItem.element) {
-        const { parentElement } = stylesItem.element;
-        if (parentElement) {
-            parentElement.removeChild(stylesItem.element);
-        }
-    }
-
-    // Then remove the document Map
-    const stylesMap = styleRegistry.get(styles)!;
-    stylesMap.delete(ownerDocument);
-
-    if (stylesMap.size) return;
-    // If no more references to these styles in any document, remove it entirely
-    styleRegistry.delete(styles);
-};
-
-type UpdatePayload = { ownerDocument: Document; previousStyles: string; styles: string };
-
-export const updateStyles = ({
-    ownerDocument,
-    previousStyles,
-    styles,
-}: UpdatePayload) => {
-    if (previousStyles === styles) return;
-
-    const stylesMap = styleRegistry.get(previousStyles);
-    const stylesItem = stylesMap?.get(ownerDocument);
-    if (stylesMap && stylesItem?.element && stylesItem?.referenceCount === 1) {
-        // Mutate existing <style> element with updated styles
-        stylesItem.element.innerHTML = styles;
-        styleRegistry.set(styles, new Map([[ownerDocument, stylesItem]]));
-        // Cleanup previous stylesMap
-        stylesMap.delete(ownerDocument);
-        if (!stylesMap.size) {
-            styleRegistry.delete(previousStyles);
-        }
-        return;
-    }
-
-    if (previousStyles) {
-        unregisterStyles({ ownerDocument, styles: previousStyles });
-    }
-
-    registerStyles({ ownerDocument, styles });
-};
-
-export const getStyleRegistryKeys = () => styleRegistry.keys();
-
-export const clearRegistry = () => {
-    styleRegistry.clear();
-};
diff --git a/packages/styling/src/useStyles.test.tsx b/packages/styling/src/useStyles.test.tsx
new file mode 100644
index 00000000..6f5afae9
--- /dev/null
+++ b/packages/styling/src/useStyles.test.tsx
@@ -0,0 +1,63 @@
+// @vitest-environment happy-dom
+import { render } from '@testing-library/react';
+import React from 'react';
+import { beforeEach, describe, expect, it } from 'vitest';
+
+import Style from './Style.js';
+import { getStyleRegistry } from './useStyles.js';
+
+describe('@acusti/styling', () => {
+    describe('useStyles.ts', () => {
+        const mockStylesA = '.test-a {\n  color: cyan;\n}';
+        const mockStylesB = '.test-b {\n  padding: 10px;\n}';
+
+        // reset styleRegistry before each test
+        beforeEach(() => {
+            getStyleRegistry().clear();
+        });
+
+        it('should cache minified styles in the registry keyed by the style string', () => {
+            const styleRegistry = getStyleRegistry();
+
+            const { rerender } = render(
+                <React.Fragment>
+                    <Style>{mockStylesA}</Style>
+                    <Style>{mockStylesA}</Style>
+                </React.Fragment>,
+            );
+
+            let stylesItemA = styleRegistry.get(mockStylesA);
+            expect(stylesItemA?.referenceCount).toBe(2);
+            expect(stylesItemA?.styles).toBe('.test-a { color: cyan; }');
+            expect(styleRegistry.size).toBe(1);
+
+            rerender(<Style>{mockStylesA}</Style>);
+            expect(stylesItemA?.referenceCount).toBe(1);
+            expect(stylesItemA).toBe(styleRegistry.get(mockStylesA));
+            expect(styleRegistry.size).toBe(1);
+
+            rerender(<Style>{mockStylesB}</Style>);
+            stylesItemA = styleRegistry.get(mockStylesA);
+            expect(stylesItemA).toBe(undefined);
+            let stylesItemB = styleRegistry.get(mockStylesB);
+            expect(stylesItemB?.referenceCount).toBe(1);
+            expect(styleRegistry.size).toBe(1);
+
+            rerender(
+                <React.Fragment>
+                    <Style>{mockStylesA}</Style>
+                    <Style>{mockStylesB}</Style>
+                </React.Fragment>,
+            );
+            stylesItemA = styleRegistry.get(mockStylesA);
+            expect(stylesItemA?.referenceCount).toBe(1);
+            expect(stylesItemA).toBe(styleRegistry.get(mockStylesA));
+            stylesItemB = styleRegistry.get(mockStylesB);
+            expect(stylesItemB?.referenceCount).toBe(1);
+            expect(styleRegistry.size).toBe(2);
+
+            rerender(<div />);
+            expect(styleRegistry.size).toBe(0);
+        });
+    });
+});
diff --git a/packages/styling/src/useStyles.ts b/packages/styling/src/useStyles.ts
new file mode 100644
index 00000000..3c0bb9cb
--- /dev/null
+++ b/packages/styling/src/useStyles.ts
@@ -0,0 +1,58 @@
+import { useEffect, useState } from 'react';
+
+import { minifyStyles } from './minifyStyles.js';
+
+type StyleRegistry = Map<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 '';
+
+        let minified = '';
+        const existingStylesItem = styleRegistry.get(styles);
+        if (existingStylesItem) {
+            existingStylesItem.referenceCount++;
+            minified = existingStylesItem.styles;
+        } else {
+            minified = minifyStyles(styles);
+            styleRegistry.set(styles, { referenceCount: 1, styles: minified });
+        }
+
+        return minified;
+    });
+
+    useEffect(() => {
+        if (!styles) return;
+        if (!styleRegistry.get(styles)) {
+            const minified = minifyStyles(styles);
+            styleRegistry.set(styles, { referenceCount: 1, styles: minified });
+            setMinifiedStyles(minified);
+        }
+
+        return () => {
+            const stylesItem = styleRegistry.get(styles);
+            if (stylesItem) {
+                stylesItem.referenceCount--;
+                if (!stylesItem.referenceCount) {
+                    // TODO try scheduling this via setTimeout
+                    // and add another referenceCount check
+                    // to deal with instance where existing <Style>
+                    // component is moved in the tree or re-keyed
+                    styleRegistry.delete(styles);
+                }
+            }
+        };
+    }, [styles]);
+
+    return minifiedStyles;
+}
+
+export default useStyles;
+
+export const clearRegistry = () => {
+    styleRegistry.clear();
+};