Skip to content

Commit 49e664f

Browse files
authored
fix: infer domain from client for useContextMutator setContext (#1317)
## This PR As identified when chatting in #1303 (comment), if you pass a `Client` into `<OpenFeatureProvider/>`, then `useContextMutator#setContext` will always see `domain` as `undefined` and so will always mutate the default domain even if the client is not for that domain -- instead, we should rely on the domain that the client tells us it is for! I've also updated the tests generally for `useContextMutator` slightly, as they were sometimes reusing contexts set by other tests, leading to unexpected failures when I added this new test. ### Notes This, and the provider hook, were the only two things relying on the `domain` property being exposed in the internal context, so I've removed it to hopefully prevent any future logic from running into this same trap. Open question that I've left as a TODO, should the hook throw an error if someone tries to use the hook outside of `<OpenFeatureProvider/>` (if they've not set `defaultContext: true`)? Though I fear that would make this a breaking change (though arguably it is still a fix as being able to use this outside a hook today feels like a bug, not an intentional design decision)? --------- Signed-off-by: MattIPv4 <me@mattcowley.co.uk>
1 parent f3f3685 commit 49e664f

File tree

5 files changed

+70
-8
lines changed

5 files changed

+70
-8
lines changed

packages/react/src/context/use-context-mutator.ts

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
import { useCallback, useContext } from 'react';
1+
import { useCallback, useContext, useEffect, useState } from 'react';
22
import type { EvaluationContext } from '@openfeature/web-sdk';
33
import { OpenFeature } from '@openfeature/web-sdk';
44
import { Context } from '../internal';
55

66
export type ContextMutationOptions = {
77
/**
88
* Mutate the default context instead of the domain scoped context applied at the `<OpenFeatureProvider/>`.
9-
* Note, if the `<OpenFeatureProvider/>` has no domain specified, the default is used.
9+
* By default, will use the domain set on `<OpenFeatureProvider/>` (or the domain associated with the client set on `<OpenFeatureProvider/>`).
1010
* See the {@link https://openfeature.dev/docs/reference/technologies/client/web/#manage-evaluation-context-for-domains|documentation} for more information.
1111
* @default false
1212
*/
@@ -33,7 +33,28 @@ export type ContextMutation = {
3333
* @returns {ContextMutation} context-aware function(s) to mutate evaluation context
3434
*/
3535
export function useContextMutator(options: ContextMutationOptions = { defaultContext: false }): ContextMutation {
36-
const { domain } = useContext(Context) || {};
36+
const { client } = useContext(Context) || {};
37+
const domain = client?.metadata.domain;
38+
39+
// TODO: Replace this warning with a thrown error in a future major release,
40+
// to match the behavior of `useOpenFeatureProvider` + `useOpenFeatureClient`,
41+
// when `defaultContext` isn't explicitly set to true.
42+
const [warned, setWarned] = useState(false);
43+
useEffect(() => {
44+
if (options.defaultContext || domain) {
45+
if (warned) {
46+
setWarned(false);
47+
}
48+
return;
49+
}
50+
51+
if (!warned) {
52+
console.warn(
53+
'[useContextMutator] No domain available from OpenFeature context; are you using <OpenFeatureProvider/>? setContext will mutate the default context, as if `defaultContext: true` were set. This may result in a thrown error in the future.',
54+
);
55+
setWarned(true);
56+
}
57+
}, [warned]);
3758

3859
const setContext = useCallback(
3960
async (

packages/react/src/internal/context.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ import { normalizeOptions } from '.';
99
* **DO NOT EXPORT PUBLICLY**
1010
* @internal
1111
*/
12-
export const Context = React.createContext<
13-
{ client: Client; domain?: string; options: ReactFlagEvaluationOptions } | undefined
14-
>(undefined);
12+
export const Context = React.createContext<{ client: Client; options: ReactFlagEvaluationOptions } | undefined>(
13+
undefined,
14+
);
1515

1616
/**
1717
* Get a normalized copy of the options used for this OpenFeatureProvider, see {@link normalizeOptions}.

packages/react/src/provider/provider.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,5 @@ type ProviderProps = {
3434
export function OpenFeatureProvider({ client, domain, children, ...options }: ProviderProps) {
3535
const stableClient = React.useMemo(() => client || OpenFeature.getClient(domain), [client, domain]);
3636

37-
return <Context.Provider value={{ client: stableClient, options, domain }}>{children}</Context.Provider>;
37+
return <Context.Provider value={{ client: stableClient, options }}>{children}</Context.Provider>;
3838
}

packages/react/src/provider/use-open-feature-provider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,5 @@ export function useOpenFeatureProvider(): Provider {
1717
throw new MissingContextError('No OpenFeature context available');
1818
}
1919

20-
return OpenFeature.getProvider(openFeatureContext.domain);
20+
return OpenFeature.getProvider(openFeatureContext.client.metadata.domain);
2121
}

packages/react/test/provider.spec.tsx

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ describe('OpenFeatureProvider', () => {
5151
); // delay init by 100ms
5252
};
5353

54+
beforeEach(async () => {
55+
await OpenFeature.clearContexts();
56+
});
57+
5458
describe('useOpenFeatureClient', () => {
5559
const DOMAIN = 'useOpenFeatureClient';
5660

@@ -230,6 +234,43 @@ describe('OpenFeatureProvider', () => {
230234
expect(screen.getByText('Will says aloha')).toBeInTheDocument();
231235
});
232236

237+
it('should update context when a client is set', async () => {
238+
const DOMAIN = 'mutate-context-client';
239+
OpenFeature.setProvider(DOMAIN, suspendingProvider(), {});
240+
241+
const client = OpenFeature.getClient(DOMAIN);
242+
243+
const changed = jest.fn();
244+
client.addHandler(ProviderEvents.ContextChanged, changed);
245+
246+
render(
247+
<OpenFeatureProvider client={client}>
248+
<React.Suspense fallback={<div>{FALLBACK}</div>}>
249+
<TestComponent name="Will" />
250+
</React.Suspense>
251+
</OpenFeatureProvider>,
252+
);
253+
254+
await waitFor(() => {
255+
expect(screen.getByText('Will says hi')).toBeInTheDocument();
256+
});
257+
258+
act(() => {
259+
fireEvent.click(screen.getByText('Update Context'));
260+
});
261+
expect(screen.getByText('Updating context...')).toBeInTheDocument();
262+
263+
await waitFor(
264+
() => {
265+
expect(screen.getByText('Update Context')).toBeInTheDocument();
266+
},
267+
{ timeout: DELAY * 2 },
268+
);
269+
expect(changed).toHaveBeenCalledTimes(1);
270+
271+
expect(screen.getByText('Will says aloha')).toBeInTheDocument();
272+
});
273+
233274
it('should update nested contexts', async () => {
234275
const DOMAIN1 = 'Wills Domain';
235276
const DOMAIN2 = 'Todds Domain';

0 commit comments

Comments
 (0)