From 0bc6f64f9f55e75ab30f56c2b56ed56ba96e8d35 Mon Sep 17 00:00:00 2001 From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Date: Wed, 15 Feb 2023 14:53:38 +0100 Subject: [PATCH] fix: make sure flagsReady doesn't go before isEnabled (#105) * fix: make sure flagsReady doesn't go before isEnabled * add new integration tests * update provider config setup --- src/FlagProvider.tsx | 25 ++++- src/integration.test.tsx | 225 +++++++++++++++++++++++++++++++++++---- 2 files changed, 222 insertions(+), 28 deletions(-) diff --git a/src/FlagProvider.tsx b/src/FlagProvider.tsx index c76920d..a29848a 100644 --- a/src/FlagProvider.tsx +++ b/src/FlagProvider.tsx @@ -10,7 +10,7 @@ export interface IFlagProvider { startClient?: boolean; } -const offlineConfig = { +const offlineConfig: IConfig = { bootstrap: [], disableRefresh: true, disableMetrics: true, @@ -20,15 +20,22 @@ const offlineConfig = { }; const FlagProvider: React.FC> = ({ - config, + config: customConfig, children, unleashClient, startClient = true, }) => { + const config = customConfig || offlineConfig; const client = React.useRef( - unleashClient || new UnleashClient(config || offlineConfig) + unleashClient || new UnleashClient(config) + ); + const [flagsReady, setFlagsReady] = React.useState( + Boolean( + unleashClient + ? customConfig?.bootstrap && customConfig?.bootstrapOverride !== false + : config.bootstrap && config.bootstrapOverride !== false + ) ); - const [flagsReady, setFlagsReady] = React.useState(false); const [flagsError, setFlagsError] = React.useState(null); const flagsErrorRef = React.useRef(null); @@ -49,8 +56,13 @@ const FlagProvider: React.FC> = ({ setFlagsError(e); } }; + + let timeout: any; const readyCallback = () => { - setFlagsReady(true); + // wait for flags to resolve after useFlag gets the same event + timeout = setTimeout(() => { + setFlagsReady(true); + }, 0); }; client.current.on('ready', readyCallback); @@ -71,6 +83,9 @@ const FlagProvider: React.FC> = ({ client.current.off('ready', readyCallback); client.current.stop(); } + if (timeout) { + clearTimeout(timeout); + } }; }, []); diff --git a/src/integration.test.tsx b/src/integration.test.tsx index 76cebe6..c6d0e33 100644 --- a/src/integration.test.tsx +++ b/src/integration.test.tsx @@ -3,8 +3,8 @@ * @jest-environment jsdom */ -import React from 'react'; -import { render, screen } from '@testing-library/react'; +import React, { useContext } from 'react'; +import { render, screen, waitFor } from '@testing-library/react'; import { EVENTS, UnleashClient } from 'unleash-proxy-client'; import '@testing-library/jest-dom'; @@ -13,31 +13,32 @@ import useFlagsStatus from './useFlagsStatus'; import { act } from 'react-dom/test-utils'; import useFlag from './useFlag'; import useVariant from './useVariant'; +import FlagContext from './FlagContext'; -test('should render toggles', async () => { - const fetchMock = jest.fn(() => { - return Promise.resolve({ - ok: true, - status: 200, - headers: new Headers({}), - json: () => { - return Promise.resolve({ - toggles: [ - { - name: 'test-flag', +const fetchMock = jest.fn(() => { + return Promise.resolve({ + ok: true, + status: 200, + headers: new Headers({}), + json: () => { + return Promise.resolve({ + toggles: [ + { + name: 'test-flag', + enabled: true, + variant: { + name: 'A', + payload: { type: 'string', value: 'A' }, enabled: true, - variant: { - name: 'A', - payload: { type: 'string', value: 'A' }, - enabled: true, - }, }, - ], - }); - }, - }); + }, + ], + }); + }, }); +}); +test('should render toggles', async () => { const client = new UnleashClient({ url: 'http://localhost:4242/api/frontend', appName: 'test', @@ -77,7 +78,9 @@ test('should render toggles', async () => { await act( () => new Promise((resolve) => { - client.on(EVENTS.READY, resolve); + client.on(EVENTS.READY, () => { + setTimeout(resolve, 1); + }); }) ); @@ -90,3 +93,179 @@ test('should render toggles', async () => { '{"name":"A","payload":{"type":"string","value":"A"},"enabled":true}' ); }); + +test('should be ready from the start if bootstrapped', () => { + const Component = React.memo(() => { + const { flagsReady } = useContext(FlagContext); + + return <>{flagsReady ? 'ready' : ''}; + }); + + render( + + + + ); + + expect(screen.getByText('ready')).toBeInTheDocument(); +}); + +test('should immediately return value if boostrapped', () => { + const Component = () => { + const enabled = useFlag('test-flag'); + + return <>{enabled ? 'enabled' : ''}; + }; + + render( + + + + ); + + expect(screen.queryByText('enabled')).toBeInTheDocument(); +}); + +test('should render limited times when bootstrapped', async () => { + let renders = 0; + const config = { + url: 'http://localhost:4242/api/frontend', + appName: 'test', + clientKey: 'test', + bootstrap: [ + { + name: 'test-flag', + enabled: true, + variant: { + name: 'A', + enabled: true, + payload: { type: 'string', value: 'A' }, + }, + impressionData: false, + }, + ], + fetch: fetchMock, + }; + const client = new UnleashClient(config); + + const Component = () => { + const enabled = useFlag('test-flag'); + const { flagsReady } = useContext(FlagContext); + + renders += 1; + + return ( + <> + {flagsReady ? 'flagsReady' : ''} + {enabled ? 'enabled' : ''} + + ); + }; + + render( + + + + ); + + expect(screen.queryByText('enabled')).toBeInTheDocument(); + expect(screen.queryByText('flagsReady')).toBeInTheDocument(); + expect(renders).toBe(1); + + // Wait for client initialization + await act( + () => + new Promise((resolve) => { + client.on(EVENTS.READY, () => { + setTimeout(resolve, 1); + }); + }) + ); + + expect(renders).toBe(1); +}); + +test('should resolve values before setting flagsReady', async () => { + const client = new UnleashClient({ + url: 'http://localhost:4242/api/frontend', + appName: 'test', + clientKey: 'test', + fetch: fetchMock, + }); + let renders = 0; + + const Component = () => { + const enabled = useFlag('test-flag'); + const { flagsReady } = useContext(FlagContext); + + renders += 1; + + return ( + <> + {flagsReady ? 'flagsReady' : ''} + {enabled ? 'enabled' : ''} + + ); + }; + + const ui = ( + + + + ); + + render(ui); + expect(renders).toBe(1); + expect(screen.queryByText('flagsReady')).not.toBeInTheDocument(); + expect(screen.queryByText('enabled')).not.toBeInTheDocument(); + await waitFor(() => + expect(screen.queryByText('enabled')).toBeInTheDocument() + ); + expect(screen.queryByText('flagsReady')).toBeNull(); + expect(renders).toBe(2); + await waitFor(() => + expect(screen.queryByText('flagsReady')).toBeInTheDocument() + ); + expect(screen.queryByText('enabled')).toBeInTheDocument(); + expect(renders).toBe(3); +});