From 9553a709c05b79a245d9713c5cde71383ee42e6b Mon Sep 17 00:00:00 2001 From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Date: Fri, 28 Jun 2024 11:40:26 +0200 Subject: [PATCH] fix: provider initialization (#169) 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. --- package.json | 4 +- src/FlagContext.ts | 2 +- src/FlagProvider.tsx | 55 +++++++---------- src/useFlagStatus.test.tsx | 120 +++++++++++++++++++++++++++++++++++++ yarn.lock | 8 +-- 5 files changed, 149 insertions(+), 40 deletions(-) create mode 100644 src/useFlagStatus.test.tsx diff --git a/package.json b/package.json index 25210e2..968e253 100644 --- a/package.json +++ b/package.json @@ -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" } } diff --git a/src/FlagContext.ts b/src/FlagContext.ts index 597996c..537720f 100644 --- a/src/FlagContext.ts +++ b/src/FlagContext.ts @@ -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; diff --git a/src/FlagProvider.tsx b/src/FlagProvider.tsx index bb3f9c3..3e5a742 100644 --- a/src/FlagProvider.tsx +++ b/src/FlagProvider.tsx @@ -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 = { @@ -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> = ({ +const FlagProvider: FC> = ({ config: customConfig, children, unleashClient, startClient = true, + stopClient = true, }) => { const config = customConfig || offlineConfig; const client = React.useRef( @@ -37,13 +39,13 @@ const FlagProvider: React.FC> = ({ 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. @@ -54,7 +56,7 @@ const FlagProvider: React.FC> = ({ const errorCallback = (e: any) => { startTransition(() => { - setFlagsError(currentError => currentError || e); + setFlagsError((currentError: any) => currentError || e); }); }; @@ -62,9 +64,9 @@ const FlagProvider: React.FC> = ({ startTransition(() => { setFlagsError(null); }); - } + } - let timeout: any; + let timeout: ReturnType | null = null; const readyCallback = () => { // wait for flags to resolve after useFlag gets the same event timeout = setTimeout(() => { @@ -90,8 +92,10 @@ const FlagProvider: React.FC> = ({ 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); @@ -99,28 +103,13 @@ const FlagProvider: React.FC> = ({ }; }, []); - 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( + const context = useMemo( () => ({ - 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, diff --git a/src/useFlagStatus.test.tsx b/src/useFlagStatus.test.tsx new file mode 100644 index 0000000..25663ee --- /dev/null +++ b/src/useFlagStatus.test.tsx @@ -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
{flagsReady ? 'flagsReady' : 'loading'}
; +}; + +const ErrorTestComponent = () => { + const { flagsError } = useFlagsStatus(); + + return
{flagsError ? 'flagsError' : 'no issue'}
; +}; + + +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 = ( + + + + ); + + 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 = ( + + + + ); + + 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 = ( + + + + ); + + render(ui); + + await waitFor(() => { + expect(screen.queryByText('flagsError')).toBeInTheDocument(); + }); + + expect(consoleError).toHaveBeenCalledWith( + 'Unleash: unable to fetch feature toggles', + expect.any(Error) + ); + consoleError.mockRestore(); +}); diff --git a/yarn.lock b/yarn.lock index b3bb1eb..f44da1a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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"