fix(react): re-evaluate flags on re-render to detect silent provider … (#1226)
## This PR - Added `useEffect` that runs on re-render to re-evaluate the flag value - Only updates state if the resolved value actually changed (using `isEqual` comparison) - Used lazy initialization for `useState` to avoid redundant initial evaluation - Added `useCallback` memoization for event handlers - Fixed `AbortController` scope issue ### Notes This resolves a subtle issue where the provider state may update without emitting a change event, leading to confusing results. The `useFlag` hook sets the initial evaluated value in a `useState`. Since this wasn't in a closure, this evaluation happened any time the component using the hook rerendered but the result was essentially ignored. Adding a logging hook shows that the current results but since this evaluation was made in an existing `useState`, the result had no effect. This resolves a subtle issue where the provider state may update without emitting a change event, leading to stale flag values being displayed. The `useFlag` hook was evaluating the flag on every re-render (as part of the `useState` initialization), but because `useState` only uses its initial value on the first render, these subsequent evaluations were being discarded. This meant that even though the hook was fetching the correct updated value from the provider on each re-render, it was throwing that value away and continuing to display the stale cached value. Adding a logging hook would show the correct evaluation happening (proving the provider had the updated value), but the UI would remain stuck with the old value because the `useState` was ignoring the re-evaluated result. The fix ensures that these re-evaluations on re-render actually update the component's state when the resolved value changes. The key insight is that the evaluation WAS happening on every re-render (due to how useState works), but React was discarding the result. Your fix makes those evaluations actually matter by checking if the value changed and updating state accordingly. Original thread: https://cloud-native.slack.com/archives/C06E4DE6S07/p1754508917397519 ### How to test I created a test that reproduced the issue, and it failed. I then implemented the changes and verified that the test passed. --------- Signed-off-by: Developer <developer@example.com> Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com> Co-authored-by: Developer <developer@example.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Lukas Reining <lukas.reining@codecentric.de> Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
This commit is contained in:
parent
1dbbd5161b
commit
3105595926
|
|
@ -8,7 +8,7 @@ import type {
|
||||||
JsonValue,
|
JsonValue,
|
||||||
} from '@openfeature/web-sdk';
|
} from '@openfeature/web-sdk';
|
||||||
import { ProviderEvents, ProviderStatus } from '@openfeature/web-sdk';
|
import { ProviderEvents, ProviderStatus } from '@openfeature/web-sdk';
|
||||||
import { useEffect, useRef, useState } from 'react';
|
import { useCallback, useEffect, useRef, useState } from 'react';
|
||||||
import {
|
import {
|
||||||
DEFAULT_OPTIONS,
|
DEFAULT_OPTIONS,
|
||||||
isEqual,
|
isEqual,
|
||||||
|
|
@ -287,8 +287,7 @@ function attachHandlersAndResolve<T extends FlagValue>(
|
||||||
const client = useOpenFeatureClient();
|
const client = useOpenFeatureClient();
|
||||||
const status = useOpenFeatureClientStatus();
|
const status = useOpenFeatureClientStatus();
|
||||||
const provider = useOpenFeatureProvider();
|
const provider = useOpenFeatureProvider();
|
||||||
|
const isFirstRender = useRef(true);
|
||||||
const controller = new AbortController();
|
|
||||||
|
|
||||||
if (defaultedOptions.suspendUntilReady && status === ProviderStatus.NOT_READY) {
|
if (defaultedOptions.suspendUntilReady && status === ProviderStatus.NOT_READY) {
|
||||||
suspendUntilInitialized(provider, client);
|
suspendUntilInitialized(provider, client);
|
||||||
|
|
@ -298,9 +297,22 @@ function attachHandlersAndResolve<T extends FlagValue>(
|
||||||
suspendUntilReconciled(client);
|
suspendUntilReconciled(client);
|
||||||
}
|
}
|
||||||
|
|
||||||
const [evaluationDetails, setEvaluationDetails] = useState<EvaluationDetails<T>>(
|
const [evaluationDetails, setEvaluationDetails] = useState<EvaluationDetails<T>>(() =>
|
||||||
resolver(client).call(client, flagKey, defaultValue, options),
|
resolver(client).call(client, flagKey, defaultValue, options),
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// Re-evaluate when dependencies change (handles prop changes like flagKey), or if during a re-render, we have detected a change in the evaluated value
|
||||||
|
useEffect(() => {
|
||||||
|
if (isFirstRender.current) {
|
||||||
|
isFirstRender.current = false;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const newDetails = resolver(client).call(client, flagKey, defaultValue, options);
|
||||||
|
if (!isEqual(newDetails.value, evaluationDetails.value)) {
|
||||||
|
setEvaluationDetails(newDetails);
|
||||||
|
}
|
||||||
|
}, [client, flagKey, defaultValue, options, resolver, evaluationDetails]);
|
||||||
|
|
||||||
// Maintain a mutable reference to the evaluation details to have a up-to-date reference in the handlers.
|
// Maintain a mutable reference to the evaluation details to have a up-to-date reference in the handlers.
|
||||||
const evaluationDetailsRef = useRef<EvaluationDetails<T>>(evaluationDetails);
|
const evaluationDetailsRef = useRef<EvaluationDetails<T>>(evaluationDetails);
|
||||||
|
|
@ -308,7 +320,7 @@ function attachHandlersAndResolve<T extends FlagValue>(
|
||||||
evaluationDetailsRef.current = evaluationDetails;
|
evaluationDetailsRef.current = evaluationDetails;
|
||||||
}, [evaluationDetails]);
|
}, [evaluationDetails]);
|
||||||
|
|
||||||
const updateEvaluationDetailsCallback = () => {
|
const updateEvaluationDetailsCallback = useCallback(() => {
|
||||||
const updatedEvaluationDetails = resolver(client).call(client, flagKey, defaultValue, options);
|
const updatedEvaluationDetails = resolver(client).call(client, flagKey, defaultValue, options);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -319,15 +331,19 @@ function attachHandlersAndResolve<T extends FlagValue>(
|
||||||
if (!isEqual(updatedEvaluationDetails.value, evaluationDetailsRef.current.value)) {
|
if (!isEqual(updatedEvaluationDetails.value, evaluationDetailsRef.current.value)) {
|
||||||
setEvaluationDetails(updatedEvaluationDetails);
|
setEvaluationDetails(updatedEvaluationDetails);
|
||||||
}
|
}
|
||||||
};
|
}, [client, flagKey, defaultValue, options, resolver]);
|
||||||
|
|
||||||
const configurationChangeCallback: EventHandler<ClientProviderEvents.ConfigurationChanged> = (eventDetails) => {
|
const configurationChangeCallback = useCallback<EventHandler<ClientProviderEvents.ConfigurationChanged>>(
|
||||||
if (shouldEvaluateFlag(flagKey, eventDetails?.flagsChanged)) {
|
(eventDetails) => {
|
||||||
updateEvaluationDetailsCallback();
|
if (shouldEvaluateFlag(flagKey, eventDetails?.flagsChanged)) {
|
||||||
}
|
updateEvaluationDetailsCallback();
|
||||||
};
|
}
|
||||||
|
},
|
||||||
|
[flagKey, updateEvaluationDetailsCallback],
|
||||||
|
);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
const controller = new AbortController();
|
||||||
if (status === ProviderStatus.NOT_READY) {
|
if (status === ProviderStatus.NOT_READY) {
|
||||||
// update when the provider is ready
|
// update when the provider is ready
|
||||||
client.addHandler(ProviderEvents.Ready, updateEvaluationDetailsCallback, { signal: controller.signal });
|
client.addHandler(ProviderEvents.Ready, updateEvaluationDetailsCallback, { signal: controller.signal });
|
||||||
|
|
@ -348,7 +364,14 @@ function attachHandlersAndResolve<T extends FlagValue>(
|
||||||
// cleanup the handlers
|
// cleanup the handlers
|
||||||
controller.abort();
|
controller.abort();
|
||||||
};
|
};
|
||||||
}, []);
|
}, [
|
||||||
|
client,
|
||||||
|
status,
|
||||||
|
defaultedOptions.updateOnContextChanged,
|
||||||
|
defaultedOptions.updateOnConfigurationChanged,
|
||||||
|
updateEvaluationDetailsCallback,
|
||||||
|
configurationChangeCallback,
|
||||||
|
]);
|
||||||
|
|
||||||
return evaluationDetails;
|
return evaluationDetails;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -924,17 +924,124 @@ describe('evaluation', () => {
|
||||||
OpenFeature.setContext(SUSPEND_OFF, { user: TARGETED_USER });
|
OpenFeature.setContext(SUSPEND_OFF, { user: TARGETED_USER });
|
||||||
});
|
});
|
||||||
|
|
||||||
// expect to see static value until we reconcile
|
// With the fix for useState initialization, the hook now immediately
|
||||||
await waitFor(() => expect(screen.queryByText(STATIC_FLAG_VALUE_A)).toBeInTheDocument(), {
|
// reflects provider state changes. This is intentional to handle cases
|
||||||
timeout: DELAY / 2,
|
// where providers don't emit proper events.
|
||||||
});
|
// The value updates immediately to the targeted value.
|
||||||
|
|
||||||
// make sure we updated after reconciling
|
|
||||||
await waitFor(() => expect(screen.queryByText(TARGETED_FLAG_VALUE)).toBeInTheDocument(), {
|
await waitFor(() => expect(screen.queryByText(TARGETED_FLAG_VALUE)).toBeInTheDocument(), {
|
||||||
timeout: DELAY * 2,
|
timeout: DELAY * 2,
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('re-render behavior when flag values change without provider events', ()=> {
|
||||||
|
it('should reflect provider state changes on re-render even without provider events', async () => {
|
||||||
|
let providerValue = 'initial-value';
|
||||||
|
|
||||||
|
class SilentUpdateProvider extends InMemoryProvider {
|
||||||
|
resolveBooleanEvaluation() {
|
||||||
|
return {
|
||||||
|
value: true,
|
||||||
|
variant: 'on',
|
||||||
|
reason: StandardResolutionReasons.STATIC,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
resolveStringEvaluation() {
|
||||||
|
return {
|
||||||
|
value: providerValue,
|
||||||
|
variant: providerValue,
|
||||||
|
reason: StandardResolutionReasons.STATIC,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const provider = new SilentUpdateProvider({});
|
||||||
|
await OpenFeature.setProviderAndWait('test', provider);
|
||||||
|
|
||||||
|
// The triggerRender prop forces a re-render
|
||||||
|
const TestComponent = ({ triggerRender }: { triggerRender: number }) => {
|
||||||
|
const { value } = useFlag('test-flag', 'default');
|
||||||
|
return <div data-testid="flag-value" data-render-count={triggerRender}>{value}</div>;
|
||||||
|
};
|
||||||
|
|
||||||
|
const WrapperComponent = () => {
|
||||||
|
const [renderCount, setRenderCount] = useState(0);
|
||||||
|
return (
|
||||||
|
<>
|
||||||
|
<button onClick={() => setRenderCount(c => c + 1)}>Force Re-render</button>
|
||||||
|
<TestComponent triggerRender={renderCount} />
|
||||||
|
</>
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
||||||
|
const { getByText } = render(
|
||||||
|
<OpenFeatureProvider client={OpenFeature.getClient('test')}>
|
||||||
|
<WrapperComponent />
|
||||||
|
</OpenFeatureProvider>
|
||||||
|
);
|
||||||
|
|
||||||
|
// Initial value should be rendered
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByTestId('flag-value')).toHaveTextContent('initial-value');
|
||||||
|
});
|
||||||
|
|
||||||
|
// Change the provider's internal state (without emitting events)
|
||||||
|
providerValue = 'updated-value';
|
||||||
|
|
||||||
|
// Force a re-render of the component
|
||||||
|
act(() => {
|
||||||
|
getByText('Force Re-render').click();
|
||||||
|
});
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByTestId('flag-value')).toHaveTextContent('updated-value');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should update flag value when flag key prop changes without provider events', async () => {
|
||||||
|
const provider = new InMemoryProvider({
|
||||||
|
'flag-a': {
|
||||||
|
disabled: false,
|
||||||
|
variants: { on: 'value-a' },
|
||||||
|
defaultVariant: 'on',
|
||||||
|
},
|
||||||
|
'flag-b': {
|
||||||
|
disabled: false,
|
||||||
|
variants: { on: 'value-b' },
|
||||||
|
defaultVariant: 'on',
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await OpenFeature.setProviderAndWait(EVALUATION, provider);
|
||||||
|
|
||||||
|
const TestComponent = ({ flagKey }: { flagKey: string }) => {
|
||||||
|
const { value } = useFlag(flagKey, 'default');
|
||||||
|
return <div data-testid="flag-value">{value}</div>;
|
||||||
|
};
|
||||||
|
|
||||||
|
const { rerender } = render(
|
||||||
|
<OpenFeatureProvider client={OpenFeature.getClient(EVALUATION)}>
|
||||||
|
<TestComponent flagKey="flag-a" />
|
||||||
|
</OpenFeatureProvider>
|
||||||
|
);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByTestId('flag-value')).toHaveTextContent('value-a');
|
||||||
|
});
|
||||||
|
|
||||||
|
// Change to flag-b (without any provider events)
|
||||||
|
rerender(
|
||||||
|
<OpenFeatureProvider client={OpenFeature.getClient(EVALUATION)}>
|
||||||
|
<TestComponent flagKey="flag-b" />
|
||||||
|
</OpenFeatureProvider>
|
||||||
|
);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByTestId('flag-value')).toHaveTextContent('value-b');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('context, hooks and options', () => {
|
describe('context, hooks and options', () => {
|
||||||
|
|
|
||||||
|
|
@ -288,7 +288,7 @@ describe('OpenFeatureProvider', () => {
|
||||||
{ timeout: DELAY * 4 },
|
{ timeout: DELAY * 4 },
|
||||||
);
|
);
|
||||||
|
|
||||||
expect(screen.getByText('Will says hi')).toBeInTheDocument();
|
expect(screen.getByText('Will says aloha')).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue