feat: avoid re-resolving flags unaffected by a change event (#1024)

## This PR - avoid re-resolving flags unaffected by a change event ### Notes If the provider sends the change event payload, the React SDK uses the list of change flags. If the list is missing or empty, all events are triggered. This is a way to avoid sending unnecessary evaluation telemetry data. --------- Signed-off-by: Michael Beemer <[email protected]> Co-authored-by: Todd Baert <[email protected]>
open-feature · Oct 1, 2024 · b8f9b4e · b8f9b4e
1 parent c1374bb
commit b8f9b4e
Show file tree

Hide file tree

Showing 2 changed files with 93 additions and 10 deletions.
diff --git a/packages/react/src/evaluation/use-feature-flag.ts b/packages/react/src/evaluation/use-feature-flag.ts
@@ -1,6 +1,8 @@
 import {
   Client,
+  ClientProviderEvents,
   EvaluationDetails,
+  EventHandler,
   FlagEvaluationOptions,
   FlagValue,
   JsonValue,
@@ -264,6 +266,11 @@ export function useObjectFlagDetails<T extends JsonValue = JsonValue>(
   );
 }
 
+// determines if a flag should be re-evaluated based on a list of changed flags
+function shouldEvaluateFlag(flagKey: string, flagsChanged?: string[]): boolean {
+  return !!flagsChanged && flagsChanged.includes(flagKey);
+}
+
 function attachHandlersAndResolve<T extends FlagValue>(
   flagKey: string,
   defaultValue: T,
@@ -309,6 +316,12 @@ function attachHandlersAndResolve<T extends FlagValue>(
     }
   };
 
+  const configurationChangeCallback: EventHandler<ClientProviderEvents.ConfigurationChanged> = (eventDetails) => {
+    if (shouldEvaluateFlag(flagKey, eventDetails?.flagsChanged)) {
+      updateEvaluationDetailsCallback();
+    }
+  };
+
   useEffect(() => {
     if (status === ProviderStatus.NOT_READY) {
       // update when the provider is ready
@@ -329,11 +342,11 @@ function attachHandlersAndResolve<T extends FlagValue>(
   useEffect(() => {
     if (defaultedOptions.updateOnConfigurationChanged) {
       // update when the provider configuration changes
-      client.addHandler(ProviderEvents.ConfigurationChanged, updateEvaluationDetailsCallback);
+      client.addHandler(ProviderEvents.ConfigurationChanged, configurationChangeCallback);
     }
     return () => {
       // cleanup the handlers
-      client.removeHandler(ProviderEvents.ConfigurationChanged, updateEvaluationDetailsCallback);
+      client.removeHandler(ProviderEvents.ConfigurationChanged, configurationChangeCallback);
     };
   }, []);
 

diff --git a/packages/react/test/evaluation.spec.tsx b/packages/react/test/evaluation.spec.tsx
@@ -1,17 +1,15 @@
+import '@testing-library/jest-dom'; // see: https://testing-library.com/docs/react-testing-library/setup
+import { act, render, renderHook, screen, waitFor } from '@testing-library/react';
+import * as React from 'react';
 import {
+  ErrorCode,
   EvaluationContext,
+  EvaluationDetails,
   Hook,
   InMemoryProvider,
   OpenFeature,
-  StandardResolutionReasons,
-  EvaluationDetails,
-  ErrorCode,
-} from '@openfeature/web-sdk';
-import '@testing-library/jest-dom'; // see: https://testing-library.com/docs/react-testing-library/setup
-import { act, render, renderHook, screen, waitFor } from '@testing-library/react';
-import * as React from 'react';
-import {
   OpenFeatureProvider,
+  StandardResolutionReasons,
   useBooleanFlagDetails,
   useBooleanFlagValue,
   useFlag,
@@ -26,6 +24,7 @@ import {
 import { TestingProvider } from './test.utils';
 import { HookFlagQuery } from '../src/evaluation/hook-flag-query';
 import { startTransition, useState } from 'react';
+import { jest } from '@jest/globals';
 
 describe('evaluation', () => {
   const EVALUATION = 'evaluation';
@@ -332,6 +331,10 @@ describe('evaluation', () => {
         await OpenFeature.setContext(RERENDER_DOMAIN, {});
       });
 
+      afterEach(() => {
+        jest.clearAllMocks();
+      });
+
       it('should not rerender on context change because the evaluated values did not change', async () => {
         const TestComponent = TestComponentFactory();
         render(
@@ -366,6 +369,24 @@ describe('evaluation', () => {
         expect(screen.queryByTestId('render-count')).toHaveTextContent('2');
       });
 
+      it('should not render on flag change because the provider did not include changed flags in the change event', async () => {
+        const TestComponent = TestComponentFactory();
+        render(
+          <OpenFeatureProvider domain={RERENDER_DOMAIN}>
+            <TestComponent></TestComponent>
+          </OpenFeatureProvider>,
+        );
+
+        expect(screen.queryByTestId('render-count')).toHaveTextContent('1');
+        await act(async () => {
+          await rerenderProvider.putConfiguration({
+            ...FLAG_CONFIG,
+          });
+        });
+
+        expect(screen.queryByTestId('render-count')).toHaveTextContent('1');
+      });
+
       it('should not rerender on flag change because the evaluated values did not change', async () => {
         const TestComponent = TestComponentFactory();
         render(
@@ -393,8 +414,37 @@ describe('evaluation', () => {
         expect(screen.queryByTestId('render-count')).toHaveTextContent('1');
       });
 
+      it('should not rerender on flag change because the config values did not change', async () => {
+        const TestComponent = TestComponentFactory();
+        const resolverSpy = jest.spyOn(rerenderProvider, 'resolveBooleanEvaluation');
+        render(
+          <OpenFeatureProvider domain={RERENDER_DOMAIN}>
+            <TestComponent></TestComponent>
+          </OpenFeatureProvider>,
+        );
+
+        expect(screen.queryByTestId('render-count')).toHaveTextContent('1');
+
+        await act(async () => {
+          await rerenderProvider.putConfiguration({
+            ...FLAG_CONFIG,
+          });
+        });
+
+        expect(screen.queryByTestId('render-count')).toHaveTextContent('1');
+        // The resolver should not be called again because the flag config did not change
+        expect(resolverSpy).toHaveBeenNthCalledWith(
+          1,
+          BOOL_FLAG_KEY,
+          expect.anything(),
+          expect.anything(),
+          expect.anything(),
+        );
+      });
+
       it('should rerender on flag change because the evaluated values changed', async () => {
         const TestComponent = TestComponentFactory();
+        const resolverSpy = jest.spyOn(rerenderProvider, 'resolveBooleanEvaluation');
         render(
           <OpenFeatureProvider domain={RERENDER_DOMAIN}>
             <TestComponent></TestComponent>
@@ -415,6 +465,26 @@ describe('evaluation', () => {
         });
 
         expect(screen.queryByTestId('render-count')).toHaveTextContent('2');
+
+        await act(async () => {
+          await rerenderProvider.putConfiguration({
+            ...FLAG_CONFIG,
+            [BOOL_FLAG_KEY]: {
+              ...FLAG_CONFIG[BOOL_FLAG_KEY],
+              // Change the default variant to trigger a rerender
+              defaultVariant: 'on',
+            },
+          });
+        });
+
+        expect(screen.queryByTestId('render-count')).toHaveTextContent('3');
+        expect(resolverSpy).toHaveBeenNthCalledWith(
+          3,
+          BOOL_FLAG_KEY,
+          expect.anything(),
+          expect.anything(),
+          expect.anything(),
+        );
       });
     });
   });