feat: re-render if flagsChanged is falsy (#1095)

Adds an improvement to the React SDK which supports re-renders if the [flags changed](https://open-feature.github.io/js-sdk/types/_openfeature_server_sdk.ConfigChangeEvent.html) array from a provider event is falsy. Since some providers have no knowledge of flags which are changed, this allows them to support dynamic re-rendering by not defining this property. If the prop is null/undefined, we diff all flags... If the property is explicitly set to an empty array, that means no flags have changed and the React SDK skips all diff checks. Signed-off-by: Todd Baert <[email protected]>
open-feature · Dec 4, 2024 · 78516f4 · 78516f4
1 parent 5ece80e
commit 78516f4
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 12 deletions.
diff --git a/packages/react/README.md b/packages/react/README.md
@@ -233,14 +233,17 @@ function Page() {
 }
 ```
 
-Note that if your provider doesn't support updates, this configuration has no impact.
+If your provider doesn't support updates, this configuration has no impact.
+
+> [!NOTE]
+> If your provider includes a list of [flags changed](https://open-feature.github.io/js-sdk/types/_openfeature_server_sdk.ConfigChangeEvent.html) in its `PROVIDER_CONFIGURATION_CHANGED` event, that list of flags is used to decide which flag evaluation hooks should re-run by diffing the latest value of these flags with the previous render.
+> If your provider event does not the include the `flags changed` list, then the SDK diffs all flags with the previous render to determine which hooks should re-run.
 
 #### Suspense Support
 
 > [!NOTE]
 > React suspense is an experimental feature and is subject to change in future versions.
 
-
 Frequently, providers need to perform some initial startup tasks.
 It may be desirable not to display components with feature flags until this is complete or when the context changes.
 Built-in [suspense](https://react.dev/reference/react/Suspense) support makes this easy.

diff --git a/packages/react/src/evaluation/use-feature-flag.ts b/packages/react/src/evaluation/use-feature-flag.ts
@@ -267,7 +267,8 @@ 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);
+  // if flagsChange is missing entirely, we don't know what to re-render
+  return !flagsChanged || flagsChanged.includes(flagKey);
 }
 
 function attachHandlersAndResolve<T extends FlagValue>(

diff --git a/packages/react/test/evaluation.spec.tsx b/packages/react/test/evaluation.spec.tsx
@@ -1,10 +1,17 @@
+import { jest } from '@jest/globals';
+import type { ProviderEmittableEvents } from '@openfeature/web-sdk';
+import { ClientProviderEvents } from '@openfeature/web-sdk';
+import type { FlagConfiguration } from '@openfeature/web-sdk/src/provider/in-memory-provider/flag-configuration';
 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 { startTransition, useState } from 'react';
 import type {
   EvaluationContext,
   EvaluationDetails,
-  Hook} from '../src/';
+  EventContext,
+  Hook
+} from '../src/';
 import {
   ErrorCode,
   InMemoryProvider,
@@ -20,12 +27,20 @@ import {
   useObjectFlagValue,
   useStringFlagDetails,
   useStringFlagValue,
-  useSuspenseFlag,
+  useSuspenseFlag
 } from '../src/';
-import { TestingProvider } from './test.utils';
 import { HookFlagQuery } from '../src/evaluation/hook-flag-query';
-import { startTransition, useState } from 'react';
-import { jest } from '@jest/globals';
+import { TestingProvider } from './test.utils';
+
+// custom provider to have better control over the emitted events
+class CustomEventInMemoryProvider extends InMemoryProvider {
+
+  putConfigurationWithCustomEvent(flagConfiguration: FlagConfiguration, event: ProviderEmittableEvents, eventContext: EventContext) {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    this['_flagConfiguration'] = { ...flagConfiguration }; // private access hack
+    this.events.emit(event, eventContext);
+  }
+}
 
 describe('evaluation', () => {
   const EVALUATION = 'evaluation';
@@ -262,7 +277,7 @@ describe('evaluation', () => {
 
     describe('re-render', () => {
       const RERENDER_DOMAIN = 'rerender';
-      const rerenderProvider = new InMemoryProvider(FLAG_CONFIG);
+      const rerenderProvider = new CustomEventInMemoryProvider(FLAG_CONFIG);
 
       function TestComponentFactory() {
         let renderCount = 0;
@@ -370,7 +385,7 @@ 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 () => {
+      it('should not render on flag change when the provider change event has empty flagsChanged', async () => {
         const TestComponent = TestComponentFactory();
         render(
           <OpenFeatureProvider domain={RERENDER_DOMAIN}>
@@ -380,14 +395,46 @@ describe('evaluation', () => {
 
         expect(screen.queryByTestId('render-count')).toHaveTextContent('1');
         await act(async () => {
-          await rerenderProvider.putConfiguration({
+          await rerenderProvider.putConfigurationWithCustomEvent({
             ...FLAG_CONFIG,
-          });
+            [BOOL_FLAG_KEY]: {
+              ...FLAG_CONFIG[BOOL_FLAG_KEY],
+              // Change the default; this should be ignored and not cause a re-render because flagsChanged is empty
+              defaultVariant: 'off',
+            },
+             // if the flagsChanged is empty, we know nothing has changed, so we don't bother diffing
+          }, ClientProviderEvents.ConfigurationChanged, { flagsChanged: [] });
+
         });
 
         expect(screen.queryByTestId('render-count')).toHaveTextContent('1');
       });
 
+      it('should re-render on flag change because the provider change event has falsy flagsChanged', async () => {
+        const TestComponent = TestComponentFactory();
+        render(
+          <OpenFeatureProvider domain={RERENDER_DOMAIN}>
+            <TestComponent></TestComponent>
+          </OpenFeatureProvider>,
+        );
+
+        expect(screen.queryByTestId('render-count')).toHaveTextContent('1');
+        await act(async () => {
+          await rerenderProvider.putConfigurationWithCustomEvent({
+            ...FLAG_CONFIG,
+            [BOOL_FLAG_KEY]: {
+              ...FLAG_CONFIG[BOOL_FLAG_KEY],
+              // Change the default variant to trigger a rerender since not only do we check flagsChanged, but we also diff the value
+              defaultVariant: 'off',
+            },
+             // if the flagsChanged is falsy, we don't know what flags changed - so we attempt to diff everything
+          }, ClientProviderEvents.ConfigurationChanged, { flagsChanged: undefined });
+
+        });
+
+        expect(screen.queryByTestId('render-count')).toHaveTextContent('2');
+      });
+
       it('should not rerender on flag change because the evaluated values did not change', async () => {
         const TestComponent = TestComponentFactory();
         render(
@@ -1105,3 +1152,4 @@ describe('evaluation', () => {
     });
   });
 });
+