From f3319885be9336a5f58556eb4a0666470e56da9a Mon Sep 17 00:00:00 2001 From: Anshul Sharma Date: Tue, 18 Aug 2026 09:58:43 +0530 Subject: [PATCH] Move module-scope Emitter in react.tsx into the Provider --- react.tsx | 33 ++++++++--- test/react-provider-isolation.test.tsx | 78 ++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 test/react-provider-isolation.test.tsx diff --git a/react.tsx b/react.tsx index 23356bf..22cd86d 100644 --- a/react.tsx +++ b/react.tsx @@ -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 | 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(null) + export type FlagsmithContextType = { flagsmith: IFlagsmith // The flagsmith instance options?: Parameters[0] // Initialisation options, if you do not provide this you will have to call init manually @@ -14,6 +19,12 @@ export type FlagsmithContextType = { export const FlagsmithProvider: FC = ({ flagsmith, options, serverState, children }) => { const firstRenderRef = useRef(true) + const eventsRef = useRef(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 @@ -52,7 +63,11 @@ export const FlagsmithProvider: FC = ({ flagsmith, options }) } } - return {children} + return ( + + {children} + + ) } const useConstant = function (value: T): T { @@ -96,10 +111,11 @@ 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) @@ -107,7 +123,7 @@ export function useFlagsmithLoading() { return () => { unsubscribe() } - }, [flagsmith]) + }, [flagsmith, events]) return loadingState } @@ -144,10 +160,11 @@ export function useFlags, T extends strin const flags = useConstant(flagsAsArray(_flags)) const traits = useConstant(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) => { @@ -161,7 +178,7 @@ export function useFlags, T extends strin return () => { unsubscribe() } - }, [flagsmith, flags, traits]) + }, [flagsmith, events, flags, traits]) const res = useMemo(() => { const res: any = {} @@ -201,11 +218,13 @@ export function useFlags, 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(null) const [, setRenderKey] = useState(() => getExperimentRenderKey(flagsmith, key)) useEffect(() => { + if (!events) return const listener = () => { const next = getExperimentRenderKey(flagsmith, key) setRenderKey((prev) => (prev !== next ? next : prev)) @@ -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 diff --git a/test/react-provider-isolation.test.tsx b/test/react-provider-isolation.test.tsx new file mode 100644 index 0000000..16f6465 --- /dev/null +++ b/test/react-provider-isolation.test.tsx @@ -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
{JSON.stringify(flags.font_size)}
+} + +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( + <> + + + + + + + + ) + + 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 } + 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' }) + }) +})