Skip to content

Commit

Permalink
fix: provider initialization (#169)
Browse files Browse the repository at this point in the history
There was a bug where the flagsReady status, returned from the useFlagsStatus hook, remains false even when an already started Unleash client is passed to the Unleash flag provider. This occurred despite the client being initialized and ready.
  • Loading branch information
Tymek authored Jun 28, 2024
1 parent e87a1ab commit 9553a70
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 40 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@
"react-dom": "^18.2.0",
"react-test-renderer": "^18.2.0",
"typescript": "^5.3.2",
"unleash-proxy-client": "^3.4.0",
"unleash-proxy-client": "^3.5.1",
"vite": "^4.5.0",
"vite-plugin-dts": "^3.6.3",
"vitest": "^0.34.6"
},
"peerDependencies": {
"unleash-proxy-client": "^3.4.0"
"unleash-proxy-client": "^3.5.1"
}
}
2 changes: 1 addition & 1 deletion src/FlagContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { UnleashClient } from 'unleash-proxy-client';
export interface IFlagContextValue
extends Pick<
UnleashClient,
'on' | 'updateContext' | 'isEnabled' | 'getVariant'
'on' | 'off' | 'updateContext' | 'isEnabled' | 'getVariant'
> {
client: UnleashClient;
flagsReady: boolean;
Expand Down
55 changes: 22 additions & 33 deletions src/FlagProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
/** @format */

import * as React from 'react';
import { IConfig, UnleashClient } from 'unleash-proxy-client';
import FlagContext, { IFlagContextValue } from './FlagContext';
import React, { type FC, type PropsWithChildren, useEffect, useMemo, useState } from 'react';
import { type IConfig, UnleashClient } from 'unleash-proxy-client';
import FlagContext, { type IFlagContextValue } from './FlagContext';

export interface IFlagProvider {
config?: IConfig;
unleashClient?: UnleashClient;
startClient?: boolean;
stopClient?: boolean;
}

const offlineConfig: IConfig = {
Expand All @@ -24,11 +25,12 @@ const _startTransition = 'startTransition';
// fallback for React <18 which doesn't support startTransition
const startTransition = React[_startTransition] || (fn => fn());

const FlagProvider: React.FC<React.PropsWithChildren<IFlagProvider>> = ({
const FlagProvider: FC<PropsWithChildren<IFlagProvider>> = ({
config: customConfig,
children,
unleashClient,
startClient = true,
stopClient = true,
}) => {
const config = customConfig || offlineConfig;
const client = React.useRef<UnleashClient>(
Expand All @@ -37,13 +39,13 @@ const FlagProvider: React.FC<React.PropsWithChildren<IFlagProvider>> = ({
const [flagsReady, setFlagsReady] = React.useState(
Boolean(
unleashClient
? customConfig?.bootstrap && customConfig?.bootstrapOverride !== false
? (customConfig?.bootstrap && customConfig?.bootstrapOverride !== false) || unleashClient.isReady?.()
: config.bootstrap && config.bootstrapOverride !== false
)
);
const [flagsError, setFlagsError] = React.useState(null);
const [flagsError, setFlagsError] = useState(client.current.getError?.() || null);

React.useEffect(() => {
useEffect(() => {
if (!config && !unleashClient) {
console.error(
`You must provide either a config or an unleash client to the flag provider.
Expand All @@ -54,17 +56,17 @@ const FlagProvider: React.FC<React.PropsWithChildren<IFlagProvider>> = ({

const errorCallback = (e: any) => {
startTransition(() => {
setFlagsError(currentError => currentError || e);
setFlagsError((currentError: any) => currentError || e);
});
};

const clearErrorCallback = (e: any) => {
startTransition(() => {
setFlagsError(null);
});
}
}

let timeout: any;
let timeout: ReturnType<typeof setTimeout> | null = null;
const readyCallback = () => {
// wait for flags to resolve after useFlag gets the same event
timeout = setTimeout(() => {
Expand All @@ -90,37 +92,24 @@ const FlagProvider: React.FC<React.PropsWithChildren<IFlagProvider>> = ({
if (client.current) {
client.current.off('error', errorCallback);
client.current.off('ready', readyCallback);
client.current.off('recovered', clearErrorCallback)
client.current.stop();
client.current.off('recovered', clearErrorCallback);
if (stopClient) {
client.current.stop();
}
}
if (timeout) {
clearTimeout(timeout);
}
};
}, []);

const updateContext: IFlagContextValue['updateContext'] = async (context) => {
await client.current.updateContext(context);
};

const isEnabled: IFlagContextValue['isEnabled'] = (toggleName) => {
return client.current.isEnabled(toggleName);
};

const getVariant: IFlagContextValue['getVariant'] = (toggleName) => {
return client.current.getVariant(toggleName);
};

const on: IFlagContextValue['on'] = (event, callback, ctx) => {
return client.current.on(event, callback, ctx);
};

const context = React.useMemo<IFlagContextValue>(
const context = useMemo<IFlagContextValue>(
() => ({
on,
updateContext,
isEnabled,
getVariant,
on: ((event, callback, ctx) => client.current.on(event, callback, ctx)) as IFlagContextValue['on'],
off: ((event, callback) => client.current.off(event, callback)) as IFlagContextValue['off'],
updateContext: async (context) => await client.current.updateContext(context),
isEnabled: (toggleName) => client.current.isEnabled(toggleName),
getVariant: (toggleName) => client.current.getVariant(toggleName),
client: client.current,
flagsReady,
flagsError,
Expand Down
120 changes: 120 additions & 0 deletions src/useFlagStatus.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import React from 'react';
import { render, screen, waitFor } from '@testing-library/react';
import useFlagsStatus from './useFlagsStatus';
import FlagProvider from './FlagProvider';
import { UnleashClient } from 'unleash-proxy-client';

const TestComponent = () => {
const { flagsReady } = useFlagsStatus();

return <div>{flagsReady ? 'flagsReady' : 'loading'}</div>;
};

const ErrorTestComponent = () => {
const { flagsError } = useFlagsStatus();

return <div>{flagsError ? 'flagsError' : 'no issue'}</div>;
};


const mockClient = {
on: vi.fn(),
off: vi.fn(),
start: vi.fn(),
stop: vi.fn(),
updateContext: vi.fn(),
isEnabled: vi.fn(),
getVariant: vi.fn(),
isReady: vi.fn(),
} as unknown as UnleashClient;

test('should initialize', async () => {
const onEventHandler = (event: string, callback: () => void) => {
if (event === 'ready') {
callback();
}
};

mockClient.on = onEventHandler as typeof mockClient.on;

const ui = (
<FlagProvider unleashClient={mockClient}>
<TestComponent />
</FlagProvider>
);

render(ui);

await waitFor(() => {
expect(screen.queryByText('flagsReady')).toBeInTheDocument();
});
});

// https://github.com/Unleash/proxy-client-react/issues/168
test('should start when already initialized client is passed', async () => {
const client = new UnleashClient({
url: 'http://localhost:4242/api',
fetch: async () =>
new Promise((resolve) => {
setTimeout(() =>
resolve({
status: 200,
ok: true,
json: async () => ({
toggles: [],
}),
headers: new Headers(),
})
);
}),
clientKey: '123',
appName: 'test',
});
await client.start();
expect(client.isReady()).toBe(true);

const ui = (
<FlagProvider unleashClient={client}>
<TestComponent />
</FlagProvider>
);

render(ui);

await waitFor(() => {
expect(screen.queryByText('flagsReady')).toBeInTheDocument();
});
});

test('should handle client errors', async () => {
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {});

const client = new UnleashClient({
url: 'http://localhost:4242/api',
fetch: async () => {
throw new Error('test error');
},
clientKey: '123',
appName: 'test',
});

await client.start();

const ui = (
<FlagProvider unleashClient={client}>
<ErrorTestComponent />
</FlagProvider>
);

render(ui);

await waitFor(() => {
expect(screen.queryByText('flagsError')).toBeInTheDocument();
});

expect(consoleError).toHaveBeenCalledWith(
'Unleash: unable to fetch feature toggles',
expect.any(Error)
);
consoleError.mockRestore();
});
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1972,10 +1972,10 @@ universalify@^0.2.0:
resolved "https://registry.yarnpkg.com/universalify/-/universalify-0.2.0.tgz#6451760566fa857534745ab1dde952d1b1761be0"
integrity sha512-CJ1QgKmNg3CwvAv/kOFmtnEN05f0D/cn9QntgNOQlQF9dgvVTHj3t+8JPdjqawCHk7V/KA+fbUqzZ9XWhcqPUg==

unleash-proxy-client@^3.4.0:
version "3.4.0"
resolved "https://registry.yarnpkg.com/unleash-proxy-client/-/unleash-proxy-client-3.4.0.tgz#c9c4a8b0f18d77dc0b041eb76478c6ce74c98c1e"
integrity sha512-ivCzm//z+S2T3gSBSZY7HN+5GfoLXZIovMyH6lIZRe2/vCicEdXtXD6cnLTQ2LAiXGV7DpoSM1m8WZGoiLRzkw==
unleash-proxy-client@^3.5.1:
version "3.5.1"
resolved "https://registry.yarnpkg.com/unleash-proxy-client/-/unleash-proxy-client-3.5.1.tgz#603af23b4c30e3509b8123e7a9ae99a9c38f335d"
integrity sha512-vfWAozp5O16ZedPPH7wFobsZaj8TQQEp/pfj+4jpWZTnOXyFpH6fAgrztRHO26bQ6iC95vVtfeVRQvgw9lo5zA==
dependencies:
tiny-emitter "^2.1.0"
uuid "^9.0.1"
Expand Down

0 comments on commit 9553a70

Please sign in to comment.