Used shallowequal to compare props in usePopup (#88)

* Used shallowequal to compare props in usePopup

* Removed dependency array

* Updated version to 0.5.1

.vscode/settings.json

@@ -1,5 +1,5 @@
 {
     "editor.codeActionsOnSave": {
-        "source.fixAll": true
+        "source.fixAll": "explicit"
     }
 }

package.json

@@ -1,6 +1,6 @@
 {
     "name": "reactive-popups",
-    "version": "0.5.0",
+    "version": "0.5.1",
     "description": "Easy popup management with React",
     "files": [
         "dist"
@@ -37,6 +37,7 @@
         "@testing-library/react": "^13.3.0",
         "@types/jest": "^29.5.1",
         "@types/react": "^18.0.9",
+        "@types/shallowequal": "^1.1.5",
         "@typescript-eslint/eslint-plugin": "^4.33.0",
         "aqu": "^0.4.3",
         "clean-publish": "^4.2.0",
@@ -56,13 +57,14 @@
     "dependencies": {
         "@sirse-dev/safe-context": "^0.3.0",
         "nanoid": "^3.3.6",
+        "shallowequal": "^1.1.0",
         "tiny-invariant": "^1.2.0"
     },
     "peerDependencies": {
         "react": ">=16"
     },
     "packageManager": "[email protected]",
     "engines": {
-        "node": "18.12.1"
+        "node": "20.11.0"
     }
 }

src/hooks/usePopup.ts

@@ -1,5 +1,6 @@
 import { ComponentType, useCallback, useEffect, useRef } from 'react';
 import { nanoid } from 'nanoid/non-secure';
+import shallowEqual from 'shallowequal';
 
 import { useEvent } from './useEvent';
 import { usePopupsContext } from './usePopupsContext';
@@ -46,9 +47,15 @@ export const usePopup = <P, K extends keyof P>(
         closePopup(popupIdentifier.current);
     }, [closePopup]);
 
+    const oldPropsRef = useRef(props);
+
     useEffect(() => {
-        update(popupIdentifier.current, props);
-    }, [props, update]);
+        if (!shallowEqual(props, oldPropsRef.current)) {
+            update(popupIdentifier.current, props);
+        }
+
+        oldPropsRef.current = props;
+    });
 
     return [open, close];
 };

test/hooks/usePopup.test.tsx

@@ -4,6 +4,8 @@ import { act, renderHook, screen } from '@testing-library/react';
 import { group, TestHookWrapper } from './TestHookWrapper';
 import { useCloseHandler } from '../../src/hooks/useCloseHandler';
 import { usePopup } from '../../src/hooks/usePopup';
+import { PopupsContext } from '../../src/PopupsContext';
+import { PopupsBag } from '../../src/types/PopupsBag';
 
 const SimplePopupComponent: React.FC = jest.fn(() => {
     const unmount = useCloseHandler(() => {
@@ -17,16 +19,32 @@ const CustomizablePopupComponent: React.FC<{ message: string }> = jest.fn(
     ({ message }) => <div>{message}</div>
 );
 
+type ComplexPopupComponentProps = {
+    a: { value: string };
+    b: string;
+};
+
+const ComplexPopupComponent: React.FC<ComplexPopupComponentProps> = ({
+    a,
+    b,
+}) => {
+    return (
+        <div>
+            <span>{a.value}</span>
+            <span>{b}</span>
+        </div>
+    );
+};
+
 const PopupComponentWithProps: React.FC<{
     prop1: number;
     prop2: string;
 }> = jest.fn(() => <div>popup with props</div>);
 
 describe('usePopup', () => {
     it('should render only one popup', () => {
-        const initialProps = {};
         const { result } = renderHook(
-            () => usePopup(SimplePopupComponent, initialProps, group),
+            () => usePopup(SimplePopupComponent, {}, group),
             {
                 wrapper: TestHookWrapper,
             }
@@ -46,9 +64,8 @@ describe('usePopup', () => {
     });
 
     it('should close popup', () => {
-        const initialProps = {};
         const { result } = renderHook(
-            () => usePopup(SimplePopupComponent, initialProps, group),
+            () => usePopup(SimplePopupComponent, {}, group),
             {
                 wrapper: TestHookWrapper,
             }
@@ -71,9 +88,8 @@ describe('usePopup', () => {
         const initialMessage = 'initial message';
         const updatedMessage = 'updated message';
 
-        const initialProps = {};
         const { result } = renderHook(
-            () => usePopup(CustomizablePopupComponent, initialProps, group),
+            () => usePopup(CustomizablePopupComponent, {}, group),
             { wrapper: TestHookWrapper }
         );
 
@@ -93,9 +109,8 @@ describe('usePopup', () => {
     });
 
     it('should merge props', () => {
-        const initialProps = { prop1: 42 };
         const { result } = renderHook(
-            () => usePopup(PopupComponentWithProps, initialProps, group),
+            () => usePopup(PopupComponentWithProps, { prop1: 42 }, group),
             {
                 wrapper: TestHookWrapper,
             }
@@ -137,4 +152,78 @@ describe('usePopup', () => {
 
         expect(screen.getByText('updated')).toBeDefined();
     });
+
+    it.only('should make shallow copy on values in props when updating popup', () => {
+        const initialA = {
+            value: 'A',
+        };
+
+        const mount = jest.fn();
+        const unmount = jest.fn();
+        const close = jest.fn();
+        const update = jest.fn();
+
+        const { result, rerender } = renderHook(
+            (props: ComplexPopupComponentProps) =>
+                usePopup(ComplexPopupComponent, props, group),
+            {
+                wrapper: ({ children }) => (
+                    <PopupsContext.Provider
+                        value={
+                            {
+                                mount,
+                                unmount,
+                                close,
+                                update,
+                            } as unknown as PopupsBag
+                        }
+                    >
+                        {children}
+                    </PopupsContext.Provider>
+                ),
+                initialProps: {
+                    a: initialA,
+                    b: 'B',
+                },
+            }
+        );
+
+        act(() => {
+            result.current[0]();
+        });
+
+        expect(mount).toBeCalled();
+
+        rerender({
+            a: initialA,
+            b: 'B',
+        });
+
+        expect(update).toBeCalledTimes(0);
+
+        rerender({
+            a: initialA,
+            b: 'C',
+        });
+
+        expect(update).toBeCalledTimes(1);
+
+        rerender({
+            a: initialA,
+            b: 'C',
+            c: 'HELLO',
+        } as unknown as ComplexPopupComponentProps);
+
+        expect(update).toBeCalledTimes(2);
+
+        rerender({
+            a: {
+                value: 'A',
+            },
+            b: 'C',
+            c: 'HELLO',
+        } as unknown as ComplexPopupComponentProps);
+
+        expect(update).toBeCalledTimes(3);
+    });
 });