From 4871a15d9f623a0820821a5253e1195152f2abcb Mon Sep 17 00:00:00 2001 From: Alexandre Philibeaux Date: Thu, 12 Oct 2023 16:32:57 +0200 Subject: [PATCH] fix(growthbook): desync of attributes when attributes providers changes --- .changeset/eight-files-dance.md | 5 ++ .../use-growthbook/src/AbTestProvider.tsx | 64 +++++++++++-------- .../@growthbook/growthbook-react.tsx | 3 +- .../src/__tests__/AbTestProvider.tsx | 10 ++- packages/use-growthbook/src/index.ts | 15 +++-- packages/use-growthbook/src/types.ts | 17 +++++ .../use-growthbook/src/useAbTestAttributes.ts | 21 +++--- 7 files changed, 87 insertions(+), 48 deletions(-) create mode 100644 .changeset/eight-files-dance.md diff --git a/.changeset/eight-files-dance.md b/.changeset/eight-files-dance.md new file mode 100644 index 000000000..3267b0940 --- /dev/null +++ b/.changeset/eight-files-dance.md @@ -0,0 +1,5 @@ +--- +'@scaleway/use-growthbook': patch +--- + +when attributes a of provider change there is new instance of growthbook created and can make a desync during render diff --git a/packages/use-growthbook/src/AbTestProvider.tsx b/packages/use-growthbook/src/AbTestProvider.tsx index 556f98237..f4aadc248 100644 --- a/packages/use-growthbook/src/AbTestProvider.tsx +++ b/packages/use-growthbook/src/AbTestProvider.tsx @@ -1,19 +1,14 @@ -import type { Context } from '@growthbook/growthbook-react' import { GrowthBook, GrowthBookProvider } from '@growthbook/growthbook-react' import { type ReactNode, useCallback, useEffect, useMemo } from 'react' -import type { Attributes, LoadConfig } from './types' +import type { + Attributes, + ErrorCallback, + LoadConfig, + ToolConfig, + TrackingCallback, +} from './types' -export type ToolConfig = { - apiHost: string - clientKey: string - enableDevMode: boolean -} - -export type TrackingCallback = NonNullable - -export type ErrorCallback = (error: Error | string) => void - -export type AbTestProviderProps = { +type AbTestProviderProps = { children: ReactNode config: ToolConfig trackingCallback: TrackingCallback @@ -22,44 +17,50 @@ export type AbTestProviderProps = { loadConfig?: LoadConfig } +const defaultLoadConfig: LoadConfig = { + autoRefresh: false, + timeout: 500, + skipCache: false, +} as const + const getGrowthBookInstance = ({ - config: { apiHost, clientKey, enableDevMode }, - attributes, + config: { + apiHost, + clientKey, + enableDevMode, + backgroundSync, + subscribeToChanges, + }, trackingCallback, }: { config: ToolConfig - attributes: Attributes trackingCallback: TrackingCallback }) => new GrowthBook({ apiHost, clientKey, enableDevMode, - attributes, trackingCallback, + backgroundSync, + subscribeToChanges, }) -const defaultLoadConfig = { - autoRefresh: false, - timeout: 500, -} - export const AbTestProvider = ({ children, config, trackingCallback, errorCallback, attributes, - loadConfig = defaultLoadConfig, + loadConfig, }: AbTestProviderProps) => { const growthbook = useMemo( - () => getGrowthBookInstance({ config, attributes, trackingCallback }), - [trackingCallback, config, attributes], + () => getGrowthBookInstance({ config, trackingCallback }), + [trackingCallback, config], ) const loadFeature = useCallback(async () => { if (config.clientKey) { - await growthbook.loadFeatures(loadConfig) + await growthbook.loadFeatures(loadConfig ?? defaultLoadConfig) } }, [growthbook, config, loadConfig]) @@ -67,6 +68,17 @@ export const AbTestProvider = ({ loadFeature().catch(errorCallback) }, [loadFeature, errorCallback]) + // avoid multiple instances of growthbook when attributes of the Providers changes. + useEffect(() => { + const currentAttributes = growthbook.getAttributes() + if (currentAttributes !== attributes) { + growthbook.setAttributes({ + ...currentAttributes, + ...attributes, + }) + } + }, [attributes, growthbook]) + return ( {children} ) diff --git a/packages/use-growthbook/src/__mocks__/@growthbook/growthbook-react.tsx b/packages/use-growthbook/src/__mocks__/@growthbook/growthbook-react.tsx index daccaf138..f041595e1 100644 --- a/packages/use-growthbook/src/__mocks__/@growthbook/growthbook-react.tsx +++ b/packages/use-growthbook/src/__mocks__/@growthbook/growthbook-react.tsx @@ -3,8 +3,9 @@ import type { ReactNode } from 'react' const GrowthBook = jest.fn(() => ({ loadFeatures: jest.fn(), + getAttributes: jest.fn(), + setAttributes: jest.fn(), })) - const GrowthBookProvider = ({ children }: { children: ReactNode }) => children const useGrowthBook = jest.fn() diff --git a/packages/use-growthbook/src/__tests__/AbTestProvider.tsx b/packages/use-growthbook/src/__tests__/AbTestProvider.tsx index 8f10e5601..1022891fb 100644 --- a/packages/use-growthbook/src/__tests__/AbTestProvider.tsx +++ b/packages/use-growthbook/src/__tests__/AbTestProvider.tsx @@ -1,12 +1,18 @@ import { GrowthBook } from '@growthbook/growthbook-react' import { beforeEach, describe, expect, it, jest } from '@jest/globals' import { render } from '@testing-library/react' -import type { TrackingCallback } from '../AbTestProvider' +import type { ComponentProps } from 'react' import { AbTestProvider } from '../AbTestProvider' +type TrackingCallback = ComponentProps< + typeof AbTestProvider +>['trackingCallback'] + +type ErrorCallback = ComponentProps['errorCallback'] + describe('AbTestProvider', () => { let trackingCallback: TrackingCallback - let errorCallback: (error: Error | string) => void + let errorCallback: ErrorCallback beforeEach(() => { trackingCallback = jest.fn() diff --git a/packages/use-growthbook/src/index.ts b/packages/use-growthbook/src/index.ts index d3e6431cf..18c92958e 100644 --- a/packages/use-growthbook/src/index.ts +++ b/packages/use-growthbook/src/index.ts @@ -4,17 +4,18 @@ export { withRunExperiment, useFeatureIsOn, useFeatureValue, + useGrowthBook, + useGrowthBookSSR, } from '@growthbook/growthbook-react' -export type { + +export { FeatureString, FeaturesReady, IfFeatureEnabled, - Context, } from '@growthbook/growthbook-react' + export { useAbTestAttributes } from './useAbTestAttributes' + export { AbTestProvider } from './AbTestProvider' -export type { - TrackingCallback, - ErrorCallback, - ToolConfig, -} from './AbTestProvider' + +export type { TrackingCallback, ErrorCallback, ToolConfig } from './types' diff --git a/packages/use-growthbook/src/types.ts b/packages/use-growthbook/src/types.ts index 44d5488d8..8ad52fa08 100644 --- a/packages/use-growthbook/src/types.ts +++ b/packages/use-growthbook/src/types.ts @@ -1,10 +1,27 @@ +import type { Context } from '@growthbook/growthbook-react' + export type Attributes = Record /** * @param {boolean} [autoRefresh] - false. * @param {number} [timeout] - 500. + * @param {boolean} [skipCache] - false. */ export type LoadConfig = { autoRefresh: boolean timeout: number + skipCache: boolean } + +export type ToolConfig = Pick< + Context, + | 'apiHost' + | 'clientKey' + | 'enableDevMode' + | 'backgroundSync' + | 'subscribeToChanges' +> + +export type TrackingCallback = NonNullable + +export type ErrorCallback = (error: Error | string) => void | null diff --git a/packages/use-growthbook/src/useAbTestAttributes.ts b/packages/use-growthbook/src/useAbTestAttributes.ts index 990eaab28..2a2d5c514 100644 --- a/packages/use-growthbook/src/useAbTestAttributes.ts +++ b/packages/use-growthbook/src/useAbTestAttributes.ts @@ -1,5 +1,4 @@ import { useGrowthBook } from '@growthbook/growthbook-react' -import { useCallback, useMemo } from 'react' import type { Attributes } from './types' export const useAbTestAttributes = (): [ @@ -8,19 +7,17 @@ export const useAbTestAttributes = (): [ ] => { const growthBook = useGrowthBook() - const attributes: Attributes = useMemo( - () => growthBook?.getAttributes() ?? {}, - [growthBook], - ) + if (growthBook) { + const attributes = growthBook.getAttributes() - const setAttributes = useCallback( - (newAttributes: Attributes) => - growthBook?.setAttributes({ + const setAttributes = (newAttributes: Attributes) => + growthBook.setAttributes({ ...attributes, ...newAttributes, - }), - [growthBook, attributes], - ) + }) - return [attributes, setAttributes] + return [attributes, setAttributes] + } + + return [{}, () => {}] }