Skip to content

Commit 4bce2a0

Browse files
authored
fix: delayed suspense causes "flicker" (#921)
<!-- Please use this template for your pull request. --> <!-- Please use the sections that you need and delete other sections --> ## This PR <!-- add the description of the PR here --> - Add an internal `useOpenFeatureClientStatus()` function to make it slightly easier to keep track of the state of the current Client, and know when to suspend or not. - Replaces the `suspend()` function with a `suspendUntilReady()` function. The `suspendUntilReady()` function fires immediatly during the first render or when the component cannot render due to the client having changed status. - Put the Client status and the feature flag details inside React states. This is required to make APIs like `startTransition` work properly with the library ### Related Issues <!-- add here the GitHub issue that this PR resolves if applicable --> Fixes #920 ### Notes <!-- any additional notes for this PR --> ### Follow-up Tasks - [x] Add more tests. Suspense and Transitions are very tricky. While these changes don't seem to have broken any existing tests, we should add new tests to make sure Suspense and Transitions continue to work properly in the future. - [ ] ~I think there might still be some reactivity issues with some parts of the codebase. The client returned by `useOpenFeatureClient()` didn't seem to update properly when the client changed, which made `useOpenFeatureClientStatus()` not update properly.~ Out of the scope of this PR ### How to test <!-- if applicable, add testing instructions under this section --> I built the application with `npm build`, then `npm pack`. I used the modified code on a local project to see if it fixed the issues. I built projects in CodeSanbox that demonstrate the issues. For Suspense: https://codesandbox.io/embed/openfeature-suspense-bug-5j7yll?fontsize=14&hidenavigation=1&theme=dark For Transitions: https://codesandbox.io/embed/openfeature-suspense-bug-forked-lqhyf3?fontsize=14&hidenavigation=1&theme=dark --------- Signed-off-by: Tommy Josépovic <[email protected]>
1 parent 86a9230 commit 4bce2a0

File tree

7 files changed

+213
-138
lines changed

7 files changed

+213
-138
lines changed

packages/react/src/common/suspend.ts

Lines changed: 0 additions & 75 deletions
This file was deleted.
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { Client, ProviderEvents } from '@openfeature/web-sdk';
2+
3+
/**
4+
* Suspends until the client is ready to evaluate feature flags.
5+
* DO NOT EXPORT PUBLICLY
6+
* @param {Client} client OpenFeature client
7+
*/
8+
export function suspendUntilReady(client: Client): Promise<void> {
9+
let resolve: (value: unknown) => void;
10+
let reject: () => void;
11+
throw new Promise((_resolve, _reject) => {
12+
resolve = _resolve;
13+
reject = _reject;
14+
client.addHandler(ProviderEvents.Ready, resolve);
15+
client.addHandler(ProviderEvents.Error, reject);
16+
}).finally(() => {
17+
client.removeHandler(ProviderEvents.Ready, resolve);
18+
client.removeHandler(ProviderEvents.Ready, reject);
19+
});
20+
}

packages/react/src/evaluation/use-feature-flag.ts

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,10 @@ import {
1010
} from '@openfeature/web-sdk';
1111
import { useEffect, useState } from 'react';
1212
import { DEFAULT_OPTIONS, ReactFlagEvaluationOptions, normalizeOptions } from '../common/options';
13-
import { suspend } from '../common/suspend';
13+
import { suspendUntilReady } from '../common/suspense';
1414
import { useProviderOptions } from '../provider/context';
1515
import { useOpenFeatureClient } from '../provider/use-open-feature-client';
16+
import { useOpenFeatureClientStatus } from '../provider/use-open-feature-client-status';
1617
import { FlagQuery } from '../query';
1718

1819
// This type is a bit wild-looking, but I think we need it.
@@ -246,58 +247,56 @@ function attachHandlersAndResolve<T extends FlagValue>(
246247
options?: ReactFlagEvaluationOptions,
247248
): EvaluationDetails<T> {
248249
// highest priority > evaluation hook options > provider options > default options > lowest priority
249-
const defaultedOptions = { ...DEFAULT_OPTIONS, ...useProviderOptions(), ...normalizeOptions(options)};
250-
const [, updateState] = useState<object | undefined>();
250+
const defaultedOptions = { ...DEFAULT_OPTIONS, ...useProviderOptions(), ...normalizeOptions(options) };
251251
const client = useOpenFeatureClient();
252-
const forceUpdate = () => {
253-
updateState({});
254-
};
255-
const suspendRef = () => {
256-
suspend(
257-
client,
258-
updateState,
259-
ProviderEvents.ContextChanged,
260-
ProviderEvents.ConfigurationChanged,
261-
ProviderEvents.Ready,
262-
);
252+
const status = useOpenFeatureClientStatus();
253+
254+
// suspense
255+
if (defaultedOptions.suspendUntilReady && status === ProviderStatus.NOT_READY) {
256+
suspendUntilReady(client);
257+
}
258+
259+
if (defaultedOptions.suspendWhileReconciling && status === ProviderStatus.RECONCILING) {
260+
suspendUntilReady(client);
261+
}
262+
263+
const [evalutationDetails, setEvaluationDetails] = useState<EvaluationDetails<T>>(
264+
resolver(client).call(client, flagKey, defaultValue, options),
265+
);
266+
267+
const updateEvaluationDetailsRef = () => {
268+
setEvaluationDetails(resolver(client).call(client, flagKey, defaultValue, options));
263269
};
264270

265271
useEffect(() => {
266-
if (client.providerStatus === ProviderStatus.NOT_READY) {
272+
if (status === ProviderStatus.NOT_READY) {
267273
// update when the provider is ready
268-
client.addHandler(ProviderEvents.Ready, forceUpdate);
269-
if (defaultedOptions.suspendUntilReady) {
270-
suspend(client, updateState, ProviderEvents.Ready);
271-
}
274+
client.addHandler(ProviderEvents.Ready, updateEvaluationDetailsRef);
272275
}
273276

274277
if (defaultedOptions.updateOnContextChanged) {
275278
// update when the context changes
276-
client.addHandler(ProviderEvents.ContextChanged, forceUpdate);
277-
if (defaultedOptions.suspendWhileReconciling) {
278-
client.addHandler(ProviderEvents.Reconciling, suspendRef);
279-
}
279+
client.addHandler(ProviderEvents.ContextChanged, updateEvaluationDetailsRef);
280280
}
281281
return () => {
282282
// cleanup the handlers
283-
client.removeHandler(ProviderEvents.Ready, forceUpdate);
284-
client.removeHandler(ProviderEvents.ContextChanged, forceUpdate);
285-
client.removeHandler(ProviderEvents.Reconciling, suspendRef);
283+
client.removeHandler(ProviderEvents.Ready, updateEvaluationDetailsRef);
284+
client.removeHandler(ProviderEvents.ContextChanged, updateEvaluationDetailsRef);
286285
};
287286
}, []);
288287

289288
useEffect(() => {
290289
if (defaultedOptions.updateOnConfigurationChanged) {
291290
// update when the provider configuration changes
292-
client.addHandler(ProviderEvents.ConfigurationChanged, forceUpdate);
291+
client.addHandler(ProviderEvents.ConfigurationChanged, updateEvaluationDetailsRef);
293292
}
294293
return () => {
295294
// cleanup the handlers
296-
client.removeHandler(ProviderEvents.ConfigurationChanged, forceUpdate);
295+
client.removeHandler(ProviderEvents.ConfigurationChanged, updateEvaluationDetailsRef);
297296
};
298297
}, []);
299298

300-
return resolver(client).call(client, flagKey, defaultValue, options);
299+
return evalutationDetails;
301300
}
302301

303302
// FlagQuery implementation, do not export
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { useEffect, useState } from 'react';
2+
import { useOpenFeatureClient } from './use-open-feature-client';
3+
import { ProviderEvents, ProviderStatus } from '@openfeature/web-sdk';
4+
5+
/**
6+
* Get the {@link ProviderStatus} for the OpenFeatureClient.
7+
* @returns {ProviderStatus} status of the client for this scope
8+
*/
9+
export function useOpenFeatureClientStatus(): ProviderStatus {
10+
const client = useOpenFeatureClient();
11+
const [status, setStatus] = useState(client.providerStatus);
12+
13+
useEffect(() => {
14+
const updateStatus = () => setStatus(client.providerStatus);
15+
client.addHandler(ProviderEvents.ConfigurationChanged, updateStatus);
16+
client.addHandler(ProviderEvents.ContextChanged, updateStatus);
17+
client.addHandler(ProviderEvents.Error, updateStatus);
18+
client.addHandler(ProviderEvents.Ready, updateStatus);
19+
client.addHandler(ProviderEvents.Stale, updateStatus);
20+
client.addHandler(ProviderEvents.Reconciling, updateStatus);
21+
return () => {
22+
client.removeHandler(ProviderEvents.ConfigurationChanged, updateStatus);
23+
client.removeHandler(ProviderEvents.ContextChanged, updateStatus);
24+
client.removeHandler(ProviderEvents.Error, updateStatus);
25+
client.removeHandler(ProviderEvents.Ready, updateStatus);
26+
client.removeHandler(ProviderEvents.Stale, updateStatus);
27+
client.removeHandler(ProviderEvents.Reconciling, updateStatus);
28+
};
29+
}, [client]);
30+
31+
return status;
32+
}
Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
import { ProviderEvents, ProviderStatus } from '@openfeature/web-sdk';
2-
import { useEffect, useState } from 'react';
1+
import { ProviderStatus } from '@openfeature/web-sdk';
32
import { DEFAULT_OPTIONS, ReactFlagEvaluationOptions, normalizeOptions } from '../common/options';
4-
import { suspend } from '../common/suspend';
53
import { useProviderOptions } from './context';
64
import { useOpenFeatureClient } from './use-open-feature-client';
5+
import { useOpenFeatureClientStatus } from './use-open-feature-client-status';
6+
import { suspendUntilReady } from '../common/suspense';
77

88
type Options = Pick<ReactFlagEvaluationOptions, 'suspendUntilReady'>;
99

@@ -15,28 +15,15 @@ type Options = Pick<ReactFlagEvaluationOptions, 'suspendUntilReady'>;
1515
* @returns {boolean} boolean indicating if provider is {@link ProviderStatus.READY}, useful if suspense is disabled and you want to handle loaders on your own
1616
*/
1717
export function useWhenProviderReady(options?: Options): boolean {
18-
const [, updateState] = useState<object | undefined>();
1918
const client = useOpenFeatureClient();
19+
const status = useOpenFeatureClientStatus();
2020
// highest priority > evaluation hook options > provider options > default options > lowest priority
2121
const defaultedOptions = { ...DEFAULT_OPTIONS, ...useProviderOptions(), ...normalizeOptions(options) };
22-
const updateStateRef = () => {
23-
updateState({});
24-
};
2522

26-
useEffect(() => {
27-
if (client.providerStatus === ProviderStatus.NOT_READY) {
28-
// re-render when provider is ready
29-
client.addHandler(ProviderEvents.Ready, updateStateRef);
30-
if (defaultedOptions.suspendUntilReady) {
31-
// suspend and update when the provider is ready
32-
suspend(client, updateState, ProviderEvents.Ready);
33-
}
34-
}
35-
return () => {
36-
// cleanup the handler
37-
client.removeHandler(ProviderEvents.Ready, updateStateRef);
38-
};
39-
}, []);
23+
// suspense
24+
if (defaultedOptions.suspendUntilReady && status === ProviderStatus.NOT_READY) {
25+
suspendUntilReady(client);
26+
}
4027

41-
return client.providerStatus === ProviderStatus.READY;
28+
return status === ProviderStatus.READY;
4229
}

0 commit comments

Comments
 (0)