From 78516f4181c82baf8c42fd64798fc2cfd8ff1056 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Wed, 4 Dec 2024 13:31:56 -0500 Subject: [PATCH] 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 --- packages/react/README.md | 7 +- .../react/src/evaluation/use-feature-flag.ts | 3 +- packages/react/test/evaluation.spec.tsx | 66 ++++++++++++++++--- 3 files changed, 64 insertions(+), 12 deletions(-) diff --git a/packages/react/README.md b/packages/react/README.md index f344a3fa0..68e386c87 100644 --- 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 index 05d6d8841..673e84f34 100644 --- 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( // 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( diff --git a/packages/react/test/evaluation.spec.tsx b/packages/react/test/evaluation.spec.tsx index 530369719..5c9108c5a 100644 --- 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( @@ -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( + + + , + ); + + 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', () => { }); }); }); +