Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 26 additions & 7 deletions react.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import React, { createContext, FC, useContext, useEffect, useMemo, useRef, useState } from 'react'
import Emitter from './utils/emitter'
const events = new Emitter()

import { IFlagsmith, IFlagsmithTrait, IFlagsmithFeature, IState } from './types'

export const FlagsmithContext = createContext<IFlagsmith<string, string> | null>(null)

// Each FlagsmithProvider owns its own Emitter instance (see EventsContext
// below) so that separate provider trees never share an event bus. This
// context is internal: consumers should not rely on it directly.
const EventsContext = createContext<Emitter | null>(null)

export type FlagsmithContextType = {
flagsmith: IFlagsmith // The flagsmith instance
options?: Parameters<IFlagsmith['init']>[0] // Initialisation options, if you do not provide this you will have to call init manually
Expand All @@ -14,6 +19,12 @@ export type FlagsmithContextType = {

export const FlagsmithProvider: FC<FlagsmithContextType> = ({ flagsmith, options, serverState, children }) => {
const firstRenderRef = useRef(true)
const eventsRef = useRef<Emitter | null>(null)
if (eventsRef.current === null) {
eventsRef.current = new Emitter()
}
const events = eventsRef.current

if (flagsmith && !flagsmith?._trigger) {
flagsmith._trigger = () => {
// @ts-expect-error using internal function, consumers would never call this
Expand Down Expand Up @@ -52,7 +63,11 @@ export const FlagsmithProvider: FC<FlagsmithContextType> = ({ flagsmith, options
})
}
}
return <FlagsmithContext.Provider value={flagsmith}>{children}</FlagsmithContext.Provider>
return (
<EventsContext.Provider value={events}>
<FlagsmithContext.Provider value={flagsmith}>{children}</FlagsmithContext.Provider>
</EventsContext.Provider>
)
}

const useConstant = function <T>(value: T): T {
Expand Down Expand Up @@ -96,18 +111,19 @@ const getExperimentRenderKey = (flagsmith: IFlagsmith | null, key: string): stri

export function useFlagsmithLoading() {
const flagsmith = useContext(FlagsmithContext)
const events = useContext(EventsContext)
const [loadingState, setLoadingState] = useState(flagsmith?.loadingState)

useEffect(() => {
if (!flagsmith) return
if (!flagsmith || !events) return
setLoadingState(flagsmith.loadingState)
const unsubscribe = events.on('loading_event', () => {
setLoadingState(flagsmith.loadingState)
})
return () => {
unsubscribe()
}
}, [flagsmith])
}, [flagsmith, events])

return loadingState
}
Expand Down Expand Up @@ -144,10 +160,11 @@ export function useFlags<F extends string | Record<string, any>, T extends strin
const flags = useConstant<string[]>(flagsAsArray(_flags))
const traits = useConstant<string[]>(flagsAsArray(_traits))
const flagsmith = useContext(FlagsmithContext)
const events = useContext(EventsContext)
const [renderRef, setRenderRef] = useState(getRenderKey(flagsmith as IFlagsmith, flags, traits))

useEffect(() => {
if (!flagsmith) return
if (!flagsmith || !events) return
setRenderRef(getRenderKey(flagsmith, flags, traits))
const unsubscribe = events.on('event', () => {
setRenderRef((prev) => {
Expand All @@ -161,7 +178,7 @@ export function useFlags<F extends string | Record<string, any>, T extends strin
return () => {
unsubscribe()
}
}, [flagsmith, flags, traits])
}, [flagsmith, events, flags, traits])

const res = useMemo(() => {
const res: any = {}
Expand Down Expand Up @@ -201,11 +218,13 @@ export function useFlags<F extends string | Record<string, any>, T extends strin
*/
export function useExperiment(featureName: string): IFlagsmithFeature | null {
const flagsmith = useContext(FlagsmithContext)
const events = useContext(EventsContext)
const key = normalizeFlagKey(featureName)
const lastExposureKey = useRef<string | null>(null)
const [, setRenderKey] = useState<string>(() => getExperimentRenderKey(flagsmith, key))

useEffect(() => {
if (!events) return
const listener = () => {
const next = getExperimentRenderKey(flagsmith, key)
setRenderKey((prev) => (prev !== next ? next : prev))
Expand All @@ -215,7 +234,7 @@ export function useExperiment(featureName: string): IFlagsmithFeature | null {
return () => {
off()
}
}, [flagsmith, key])
}, [flagsmith, events, key])

const flag = (flagsmith?.getAllFlags()?.[key] as IFlagsmithFeature | undefined) ?? null
const identifier = flagsmith?.getContext().identity?.identifier ?? null
Expand Down
78 changes: 78 additions & 0 deletions test/react-provider-isolation.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import React, { FC } from 'react'
import { act, render, screen, waitFor } from '@testing-library/react'
import { FlagsmithProvider, useFlags } from '../react'
import { experimentIdentity, getFlagsmith } from './test-constants'

// Regression test for https://github.com/Flagsmith/flagsmith-js-client/issues/391
//
// react.tsx used to instantiate a single Emitter at module scope, so every
// FlagsmithProvider in the process shared the same event bus: an internal
// `_trigger()` call on ANY flagsmith instance would notify hooks subscribed
// to every provider tree, not just the one that changed. This test renders
// two independent FlagsmithProvider trees and asserts that firing instance
// `a`'s internal trigger only re-renders consumers of `a`'s tree, never `b`'s.
//
// Flags are mutated directly (rather than via a mocked fetch + getFlags())
// because `_fetch` in flagsmith-core is a module-level variable shared by
// all instances, which is an unrelated, pre-existing limitation of the
// mock-fetch test setup for concurrent instances - orthogonal to the
// per-provider event bus behaviour this test targets.

const Probe: FC<{ label: string; onRender: () => void }> = ({ label, onRender }) => {
const flags = useFlags(['font_size'])
onRender()
return <div data-testid={label}>{JSON.stringify(flags.font_size)}</div>
}

describe('FlagsmithProvider event isolation', () => {
it('does not notify one provider tree when another provider tree updates', async () => {
const a = getFlagsmith({ identity: experimentIdentity })
const b = getFlagsmith({ identity: experimentIdentity })

const renderCountA = jest.fn()
const renderCountB = jest.fn()

render(
<>
<FlagsmithProvider flagsmith={a.flagsmith} options={a.initConfig}>
<Probe label="a" onRender={renderCountA} />
</FlagsmithProvider>
<FlagsmithProvider flagsmith={b.flagsmith} options={b.initConfig}>
<Probe label="b" onRender={renderCountB} />
</FlagsmithProvider>
</>
)

await waitFor(() => {
expect(JSON.parse(screen.getByTestId('a').innerHTML)).toEqual({ enabled: true, value: 16, variant: 'control' })
expect(JSON.parse(screen.getByTestId('b').innerHTML)).toEqual({ enabled: true, value: 16, variant: 'control' })
})

// FlagsmithProvider wires up `_trigger` on the flagsmith instance it
// was given; both instances must be initialised for this to be set.
expect(typeof (a.flagsmith as any)._trigger).toBe('function')
expect(typeof (b.flagsmith as any)._trigger).toBe('function')

renderCountA.mockClear()
renderCountB.mockClear()

// Mutate only instance `a`'s flags in place, then fire the same
// internal trigger flagsmith-core calls after a real flag update.
act(() => {
const flagsmithInternal = a.flagsmith as unknown as { flags: Record<string, { value: unknown }> }
flagsmithInternal.flags = {
...flagsmithInternal.flags,
font_size: { ...flagsmithInternal.flags.font_size, value: 32 },
}
;(a.flagsmith as any)._trigger()
})

await waitFor(() => {
expect(JSON.parse(screen.getByTestId('a').innerHTML)).toEqual({ enabled: true, value: 32, variant: 'control' })
})

// Tree `b` must never have re-rendered or changed as a result of `a`'s trigger.
expect(renderCountB).not.toHaveBeenCalled()
expect(JSON.parse(screen.getByTestId('b').innerHTML)).toEqual({ enabled: true, value: 16, variant: 'control' })
})
})