fix(#129): detect variant payload change in useVariant hook (#132)

thomasheartman · web-flow · commit e95d8d6c84c4 · 2023-09-29T16:08:19.000+02:00
As described in #129, the react SDK didn't take into account if a variant's payload had changed when getting updates, resulting in potentially stale data. This PR addresses that by checking all parts of the variant before determining whether it has changed. Closes #129
diff --git a/src/useVariant.test.tsx b/src/useVariant.test.tsx
@@ -1,7 +1,7 @@
 import { vi } from 'vitest';
 import { renderHook } from '@testing-library/react-hooks/native';
 import { useContext } from 'react';
-import useVariant from './useVariant';
+import useVariant, { variantHasChanged } from './useVariant';
 
 vi.mock('react', async () => {
   const react = (await vi.importActual('react')) as any;
@@ -158,3 +158,87 @@ test('should remove event listeners when unmounted', () => {
   expect(clientMock.off).nthCalledWith(1, ...clientMock.on.mock.calls[0]);
   expect(clientMock.off).nthCalledWith(2, ...clientMock.on.mock.calls[1]);
 });
+
+describe('Variant change detection', () => {
+    test('If the variants are identical, it returns `false`', () => {
+        const a = {
+            name: 'a',
+            enabled: true,
+            payload: {
+                type: 'string',
+                value: 'data',
+            },
+        };
+        const b = {
+            name: 'a',
+            enabled: true,
+            payload: {
+                type: 'string',
+                value: 'data',
+            },
+        };
+
+        expect(variantHasChanged(a, b)).toBeFalsy();
+    });
+
+    test('If the new variant is undefined, it counts as a change', () => {
+        const a = { name: 'a', enabled: true };
+
+        expect(variantHasChanged(a, undefined)).toBeTruthy();
+    });
+
+    test('Name change is detected', () => {
+        const a = { name: 'a', enabled: true };
+        const b = { name: 'b', enabled: true };
+
+        expect(variantHasChanged(a, b)).toBeTruthy();
+    });
+
+    test('Enabled state change is detected', () => {
+        const enabled = { name: 'a', enabled: true };
+        const disabled = { name: 'a', enabled: false };
+
+        expect(variantHasChanged(enabled, disabled)).toBeTruthy();
+    });
+    test('Payload type change is detected', () => {
+        const a = {
+            name: 'a',
+            enabled: true,
+            payload: {
+                type: 'string',
+                value: '{}',
+            },
+        };
+        const b = {
+            name: 'a',
+            enabled: true,
+            payload: {
+                type: 'json',
+                value: '{}',
+            },
+        };
+
+        expect(variantHasChanged(a, b)).toBeTruthy();
+    });
+
+    test('Payload value change is detected', () => {
+        const a = {
+            name: 'a',
+            enabled: true,
+            payload: {
+                type: 'string',
+                value: '1',
+            },
+        };
+        const b = {
+            name: 'a',
+            enabled: true,
+            payload: {
+                type: 'string',
+                value: '2',
+            },
+        };
+
+        expect(variantHasChanged(a, b)).toBeTruthy();
+    });
+});
diff --git a/src/useVariant.ts b/src/useVariant.ts
@@ -2,6 +2,19 @@ import { useContext, useState, useEffect, useRef } from 'react';
 import { IVariant } from 'unleash-proxy-client';
 import FlagContext from './FlagContext';
 
+export const variantHasChanged = (
+    oldVariant: IVariant,
+    newVariant?: IVariant,
+): boolean => {
+    const variantsAreEqual =
+        oldVariant.name === newVariant?.name &&
+        oldVariant.enabled === newVariant?.enabled &&
+        oldVariant.payload?.type === newVariant?.payload?.type &&
+        oldVariant.payload?.value === newVariant?.payload?.value;
+
+    return !variantsAreEqual;
+};
+
 const useVariant = (name: string): Partial<IVariant> => {
   const { getVariant, client } = useContext(FlagContext);
 
@@ -17,10 +30,7 @@ const useVariant = (name: string): Partial<IVariant> => {
 
     const updateHandler = () => {
       const newVariant = getVariant(name);
-      if (
-        variantRef.current.name !== newVariant?.name ||
-        variantRef.current.enabled !== newVariant?.enabled
-      ) {
+      if (variantHasChanged(variantRef.current, newVariant)) {
         setVariant(newVariant);
         variantRef.current = newVariant;
       }
