🔨 Fixes referencial equality failures in hook dep arrays

danieldelcore · Jun 16, 2020 · c4eed6e · c4eed6e
1 parent 16fd640
commit c4eed6e
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 27 deletions.
diff --git a/examples/Misc.tsx b/examples/Misc.tsx
@@ -2,7 +2,7 @@ import { storiesOf } from '@storybook/react';
 import React, { FC, Fragment, useState, MouseEventHandler } from 'react';
 import { useStableRefTester, RenderCount } from 'react-stable-ref';
 
-import { css, useStyles } from '@trousers/core';
+import { css, useStyles, useGlobals } from '@trousers/core';
 import styleCollector from '@trousers/collector';
 import { ThemeProvider } from '@trousers/theme';
 
@@ -38,6 +38,59 @@ storiesOf('Miscellaneous', module)
 
         return <TrousersLogo />;
     })
+    .add('State transitions (Globals)', () => {
+        interface Theme {
+            color: string;
+        }
+
+        const theme: Theme = {
+            color: '#ff5d9e',
+        };
+
+        const globalStyles = css<Theme>`
+            * {
+                color: ${theme => theme.color};
+            }
+        `;
+
+        const styles = styleCollector('block').element`
+                background-color: #404b69;
+                color: blue;
+                padding: 20px;
+                border: none;
+                border-radius: 6px;
+                letter-spacing: 1px;
+                font-family: 'Press Start 2P', sans-serif;
+                text-align: center;
+
+                h2 {
+                    font-size: 40px;
+                }
+            `;
+
+        const TextBlock: FC = () => {
+            useGlobals<Theme>(globalStyles);
+            useStableRefTester();
+
+            const classNames = useStyles(styles);
+
+            return (
+                <div className={classNames}>
+                    <p>
+                        Render count: <RenderCount />
+                    </p>
+                    <h2>Themed global Styles!</h2>
+                    <p>I should be pink!</p>
+                </div>
+            );
+        };
+
+        return (
+            <ThemeProvider theme={theme}>
+                <TextBlock />
+            </ThemeProvider>
+        );
+    })
     .add('Alternating style collectors', () => {
         const logoStyles = css`
             width: 150px;

diff --git a/packages/core/src/useGlobals.tsx b/packages/core/src/useGlobals.tsx
@@ -6,7 +6,7 @@ import {
     STYLE_ID,
     CSSProps,
 } from '@trousers/utils';
-import { registry, Registry } from '@trousers/registry';
+import { registry } from '@trousers/registry';
 import { ThemeContext, ThemeCtx } from '@trousers/theme';
 import { ServerContext, ServerCtx } from '@trousers/server';
 import { parseObject } from '@trousers/parser';
@@ -15,43 +15,35 @@ import { toHash } from '@trousers/hash';
 import { interpolateStyles, isBrowser } from './common';
 import css from './css';
 
-function registerGlobals<Theme>(
+export default function useGlobals<Theme>(
     styleCollectors:
         | StyleCollector<Theme>
         | StyleCollector<Theme>[]
         | CSSProps
         | CSSProps[],
-    theme: Theme,
-    registry: Registry,
 ) {
+    const { theme } = useContext<ThemeCtx>(ThemeContext);
+    const serverStyleRegistry = useContext<ServerCtx>(ServerContext);
     const collectors =
         styleCollectors instanceof Array ? styleCollectors : [styleCollectors];
 
-    collectors.forEach(collector => {
+    const activeStyles = collectors.map(collector => {
         const parsedCollector = !(collector as StyleCollector<Theme>).get
             ? css([parseObject(collector)])
             : (collector as StyleCollector<Theme>);
 
-        const styleDefinition = parsedCollector.get()[0];
+        const definition = parsedCollector.get()[0];
         const styles = interpolateStyles(
-            styleDefinition.styles,
-            styleDefinition.expressions,
+            definition.styles,
+            definition.expressions,
             theme,
         );
 
-        registry.register(toHash(styles).toString(), styles, true);
+        return {
+            hash: toHash(styles).toString(),
+            styles,
+        };
     });
-}
-
-export default function useGlobals<Theme>(
-    styleCollectors:
-        | StyleCollector<Theme>
-        | StyleCollector<Theme>[]
-        | CSSProps
-        | CSSProps[],
-) {
-    const { theme } = useContext<ThemeCtx>(ThemeContext);
-    const serverStyleRegistry = useContext<ServerCtx>(ServerContext);
 
     if (!isBrowser() && !serverStyleRegistry) {
         throw Error(
@@ -60,18 +52,25 @@ export default function useGlobals<Theme>(
     }
 
     if (!isBrowser() && !!serverStyleRegistry) {
-        registerGlobals(styleCollectors, theme as Theme, serverStyleRegistry);
+        activeStyles.forEach(({ hash, styles }) => {
+            serverStyleRegistry.register(hash, styles, true);
+        });
     }
 
+    const hash = activeStyles.reduce((accum, { hash }) => accum + hash, '');
+
     useLayoutEffect(() => {
         const headElement = document.getElementsByTagName('head')[0];
         const clientRegistry = registry(headElement, GLOBAL_STYLE_ID, {
             forceNewNode: true,
             appendBefore: STYLE_ID,
         });
 
-        registerGlobals<Theme>(styleCollectors, theme as Theme, clientRegistry);
+        activeStyles.forEach(({ hash, styles }) => {
+            clientRegistry.register(hash, styles, true);
+        });
 
         return () => clientRegistry.clear();
-    }, [theme, styleCollectors]);
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [JSON.stringify(theme), hash]);
 }
diff --git a/packages/core/src/useStyles.ts b/packages/core/src/useStyles.ts
@@ -24,9 +24,7 @@ function getComponentId(
 
 function registerStyle(registry: Registry, definition: ActiveDefinition) {
     const className = `.${definition.componentId}`;
-
     if (registry.has(className)) return;
-
     registry.register(className, definition.styles);
 }
 
@@ -75,14 +73,20 @@ export default function useStyles<Theme = {}>(
         registerStyle(serverStyleRegistry, activeDefinitions[0]);
     }
 
+    const hash = activeDefinitions.reduce(
+        (accum, { componentId }) => accum + componentId,
+        '',
+    );
+
     useLayoutEffect(() => {
         const headElement = document.getElementsByTagName('head')[0];
         const clientRegistry = registry(headElement, STYLE_ID);
 
         activeDefinitions.forEach(definition =>
             registerStyle(clientRegistry, definition),
         );
-    }, [activeDefinitions, styleDefinitions]);
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [hash]);
 
     return activeDefinitions
         .reduce((accum, definition) => `${accum} ${definition.componentId}`, '')