From 51f77c72e95b7de507e6f74fa73f77e205beaded Mon Sep 17 00:00:00 2001 From: Andrew Patton Date: Sun, 1 Sep 2024 16:34:24 -0700 Subject: [PATCH] Refactor useStyles to sanitize auto-href prop + add a test case that fails without this change applied --- packages/styling/src/Style.tsx | 6 +-- packages/styling/src/useStyles.test.tsx | 7 +++ packages/styling/src/useStyles.ts | 64 +++++++++++++++++-------- 3 files changed, 53 insertions(+), 24 deletions(-) diff --git a/packages/styling/src/Style.tsx b/packages/styling/src/Style.tsx index d8cde74c..40ece6a5 100644 --- 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 ); diff --git a/packages/styling/src/useStyles.test.tsx b/packages/styling/src/useStyles.test.tsx index d5974338..c5f0a8e1 100644 --- 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(); + // 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 index 3c0bb9cb..536bc207 100644 --- 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; +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