From 1783527c1147f584f30b60f2d7ccc8a53b4a63b7 Mon Sep 17 00:00:00 2001 From: Alejandro Bailo <59607668+alejandrobailo@users.noreply.github.com> Date: Mon, 19 Oct 2026 12:27:46 +0200 Subject: [PATCH] refactor(ui): centralize PostHog feature flag reads in ui/lib/feature-flags (#13089) --- .../use-skill-launcher-variant.test.ts | 43 ++++---- .../use-skill-launcher-variant.ts | 19 ++-- ui/lib/feature-flags/client.test.ts | 100 ++++++++++++++++++ ui/lib/feature-flags/client.ts | 32 ++++++ ui/lib/feature-flags/index.ts | 2 + ui/lib/feature-flags/shared.test.ts | 19 ++++ ui/lib/feature-flags/shared.ts | 17 +++ ui/types/lighthouse-skills.ts | 6 +- 8 files changed, 199 insertions(+), 39 deletions(-) create mode 100644 ui/lib/feature-flags/client.test.ts create mode 100644 ui/lib/feature-flags/client.ts create mode 100644 ui/lib/feature-flags/index.ts create mode 100644 ui/lib/feature-flags/shared.test.ts create mode 100644 ui/lib/feature-flags/shared.ts diff --git a/ui/components/findings/table/resource-detail-drawer/use-skill-launcher-variant.test.ts b/ui/components/findings/table/resource-detail-drawer/use-skill-launcher-variant.test.ts index cbd6491ab5..d20ef45f97 100644 --- a/ui/components/findings/table/resource-detail-drawer/use-skill-launcher-variant.test.ts +++ b/ui/components/findings/table/resource-detail-drawer/use-skill-launcher-variant.test.ts @@ -1,49 +1,44 @@ import { renderHook } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; -const { useFeatureFlagVariantKeyMock } = vi.hoisted(() => ({ - useFeatureFlagVariantKeyMock: vi.fn(), +const { useFeatureFlagVariantMock } = vi.hoisted(() => ({ + useFeatureFlagVariantMock: vi.fn(), })); -vi.mock("posthog-js/react", () => ({ - useFeatureFlagVariantKey: useFeatureFlagVariantKeyMock, +vi.mock("@/lib/feature-flags/client", () => ({ + useFeatureFlagVariant: useFeatureFlagVariantMock, })); import { useSkillLauncherVariant } from "./use-skill-launcher-variant"; describe("useSkillLauncherVariant", () => { beforeEach(() => { - useFeatureFlagVariantKeyMock.mockReset(); + useFeatureFlagVariantMock.mockReset(); }); - it("should ask PostHog for the experiment flag", () => { - useFeatureFlagVariantKeyMock.mockReturnValue(undefined); + it("should read the skill launcher flag with card as the fallback", () => { + useFeatureFlagVariantMock.mockReturnValue("card"); renderHook(() => useSkillLauncherVariant()); - expect(useFeatureFlagVariantKeyMock).toHaveBeenCalledWith( + expect(useFeatureFlagVariantMock).toHaveBeenCalledWith( "finding-detail-skill-launcher", + ["card", "dropdown"], + "card", ); }); - it("should return the dropdown variant when the flag resolves to it", () => { - useFeatureFlagVariantKeyMock.mockReturnValue("dropdown"); - - const { result } = renderHook(() => useSkillLauncherVariant()); - - expect(result.current).toBe("dropdown"); - }); - it.each([ + ["dropdown", "dropdown"], ["card", "card"], - ["unresolved flag", undefined], - ["boolean flag", false], - ["unknown variant", "weird"], - ])("should fall back to card for %s", (_label, flagValue) => { - useFeatureFlagVariantKeyMock.mockReturnValue(flagValue); + ])( + "should return the %s variant the flag module resolves", + (_label, variant) => { + useFeatureFlagVariantMock.mockReturnValue(variant); - const { result } = renderHook(() => useSkillLauncherVariant()); + const { result } = renderHook(() => useSkillLauncherVariant()); - expect(result.current).toBe("card"); - }); + expect(result.current).toBe(variant); + }, + ); }); diff --git a/ui/components/findings/table/resource-detail-drawer/use-skill-launcher-variant.ts b/ui/components/findings/table/resource-detail-drawer/use-skill-launcher-variant.ts index 81fdfd72c1..a4599ec6a9 100644 --- a/ui/components/findings/table/resource-detail-drawer/use-skill-launcher-variant.ts +++ b/ui/components/findings/table/resource-detail-drawer/use-skill-launcher-variant.ts @@ -1,20 +1,17 @@ "use client"; -import { useFeatureFlagVariantKey } from "posthog-js/react"; - +import { FEATURE_FLAG, useFeatureFlagVariant } from "@/lib/feature-flags"; import { - SKILL_LAUNCHER_FLAG, SKILL_LAUNCHER_VARIANT, type SkillLauncherVariant, } from "@/types/lighthouse-skills"; -// The hook reads the global posthog singleton (posthog-js/react's default -// context), so it needs no provider or init here: the cloud fork initializes -// and identifies the client; OSS builds never resolve the flag and fall back -// to the card control, as do unresolved or unknown variants. +// Unresolved or unknown variants (including OSS builds that never init +// PostHog) fall back to the card control. export function useSkillLauncherVariant(): SkillLauncherVariant { - const value = useFeatureFlagVariantKey(SKILL_LAUNCHER_FLAG); - return value === SKILL_LAUNCHER_VARIANT.DROPDOWN - ? SKILL_LAUNCHER_VARIANT.DROPDOWN - : SKILL_LAUNCHER_VARIANT.CARD; + return useFeatureFlagVariant( + FEATURE_FLAG.SKILL_LAUNCHER, + Object.values(SKILL_LAUNCHER_VARIANT), + SKILL_LAUNCHER_VARIANT.CARD, + ); } diff --git a/ui/lib/feature-flags/client.test.ts b/ui/lib/feature-flags/client.test.ts new file mode 100644 index 0000000000..d73c122ee0 --- /dev/null +++ b/ui/lib/feature-flags/client.test.ts @@ -0,0 +1,100 @@ +import { renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const { useFeatureFlagEnabledMock, useFeatureFlagVariantKeyMock } = vi.hoisted( + () => ({ + useFeatureFlagEnabledMock: vi.fn(), + useFeatureFlagVariantKeyMock: vi.fn(), + }), +); + +vi.mock("posthog-js/react", () => ({ + useFeatureFlagEnabled: useFeatureFlagEnabledMock, + useFeatureFlagVariantKey: useFeatureFlagVariantKeyMock, +})); + +import { useFeatureFlag, useFeatureFlagVariant } from "./client"; +import { FEATURE_FLAG } from "./shared"; + +const VARIANTS = ["card", "dropdown"] as const; + +describe("useFeatureFlag", () => { + beforeEach(() => { + useFeatureFlagEnabledMock.mockReset(); + }); + + it("should pass the flag key through to PostHog", () => { + useFeatureFlagEnabledMock.mockReturnValue(undefined); + + renderHook(() => useFeatureFlag(FEATURE_FLAG.SKILL_LAUNCHER)); + + expect(useFeatureFlagEnabledMock).toHaveBeenCalledWith( + "finding-detail-skill-launcher", + ); + }); + + it("should return true when the flag is enabled", () => { + useFeatureFlagEnabledMock.mockReturnValue(true); + + const { result } = renderHook(() => + useFeatureFlag(FEATURE_FLAG.SKILL_LAUNCHER), + ); + + expect(result.current).toBe(true); + }); + + it.each([ + ["disabled", false], + ["unresolved (PostHog not initialized or still loading)", undefined], + ])("should return false when the flag is %s", (_label, flagValue) => { + useFeatureFlagEnabledMock.mockReturnValue(flagValue); + + const { result } = renderHook(() => + useFeatureFlag(FEATURE_FLAG.SKILL_LAUNCHER), + ); + + expect(result.current).toBe(false); + }); +}); + +describe("useFeatureFlagVariant", () => { + beforeEach(() => { + useFeatureFlagVariantKeyMock.mockReset(); + }); + + it("should pass the flag key through to PostHog", () => { + useFeatureFlagVariantKeyMock.mockReturnValue(undefined); + + renderHook(() => + useFeatureFlagVariant(FEATURE_FLAG.SKILL_LAUNCHER, VARIANTS, "card"), + ); + + expect(useFeatureFlagVariantKeyMock).toHaveBeenCalledWith( + "finding-detail-skill-launcher", + ); + }); + + it("should return the resolved key when it is a known variant", () => { + useFeatureFlagVariantKeyMock.mockReturnValue("dropdown"); + + const { result } = renderHook(() => + useFeatureFlagVariant(FEATURE_FLAG.SKILL_LAUNCHER, VARIANTS, "card"), + ); + + expect(result.current).toBe("dropdown"); + }); + + it.each([ + ["an unknown variant", "weird"], + ["unresolved", undefined], + ["a boolean value", true], + ])("should fall back when the flag resolves to %s", (_label, flagValue) => { + useFeatureFlagVariantKeyMock.mockReturnValue(flagValue); + + const { result } = renderHook(() => + useFeatureFlagVariant(FEATURE_FLAG.SKILL_LAUNCHER, VARIANTS, "card"), + ); + + expect(result.current).toBe("card"); + }); +}); diff --git a/ui/lib/feature-flags/client.ts b/ui/lib/feature-flags/client.ts new file mode 100644 index 0000000000..8ec4eb2ddf --- /dev/null +++ b/ui/lib/feature-flags/client.ts @@ -0,0 +1,32 @@ +"use client"; + +import { + useFeatureFlagEnabled, + useFeatureFlagVariantKey, +} from "posthog-js/react"; + +import type { FeatureFlag } from "./shared"; + +// The only place in ui/ that imports posthog-js/react's flag hooks, so +// swapping the flag provider later is a one-file change. The hooks read the +// global posthog singleton from posthog-js/react's default context and need +// no provider here: whoever initializes PostHog (the cloud fork) feeds them. +// OSS builds that never init PostHog resolve every read to its fallback. + +// Boolean flag. Only a resolved `true` counts: undefined (still loading), +// disabled PostHog and a disabled flag all read as false. +export function useFeatureFlag(flag: FeatureFlag): boolean { + return useFeatureFlagEnabled(flag) === true; +} + +// Multivariate flag. Returns the resolved key when it is one of `variants`; +// unresolved, boolean or unknown values fall back to `fallback`. +export function useFeatureFlagVariant( + flag: FeatureFlag, + variants: readonly T[], + fallback: T, +): T { + const value = useFeatureFlagVariantKey(flag); + const match = variants.find((variant) => variant === value); + return match ?? fallback; +} diff --git a/ui/lib/feature-flags/index.ts b/ui/lib/feature-flags/index.ts new file mode 100644 index 0000000000..011f86c371 --- /dev/null +++ b/ui/lib/feature-flags/index.ts @@ -0,0 +1,2 @@ +export * from "./client"; +export * from "./shared"; diff --git a/ui/lib/feature-flags/shared.test.ts b/ui/lib/feature-flags/shared.test.ts new file mode 100644 index 0000000000..dbace91a1e --- /dev/null +++ b/ui/lib/feature-flags/shared.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from "vitest"; + +import { FEATURE_FLAG } from "./shared"; + +describe("FEATURE_FLAG", () => { + it("should map each flag to the key PostHog knows it by", () => { + expect(FEATURE_FLAG).toEqual({ + SKILL_LAUNCHER: "finding-detail-skill-launcher", + ISSUES: "issues-feature", + BILLING_SYSTEM_METRONOME: "BILLING_SYSTEM_METRONOME", + }); + }); + + it("should not contain duplicate keys", () => { + const keys = Object.values(FEATURE_FLAG); + + expect(new Set(keys).size).toBe(keys.length); + }); +}); diff --git a/ui/lib/feature-flags/shared.ts b/ui/lib/feature-flags/shared.ts new file mode 100644 index 0000000000..72dabd38f1 --- /dev/null +++ b/ui/lib/feature-flags/shared.ts @@ -0,0 +1,17 @@ +// Registry of every feature flag key the UI reads. Side-effect free and +// SDK-agnostic so both client and server readers can import it. Every key +// lives here, including the ones only the cloud fork reads, so the registry +// never diverges between repos. +export const FEATURE_FLAG = { + // Multivariate flag deciding which skill launcher the finding detail drawer + // renders ("card" | "dropdown"). + SKILL_LAUNCHER: "finding-detail-skill-launcher", + // Boolean flag gating the experimental Issues feature (contextual findings). + // Read by cloud only. + ISSUES: "issues-feature", + // Boolean flag evaluated server-side per tenant in cloud. When on, it pins + // the tenant to legacy billing; off or absent means the new Metronome system. + BILLING_SYSTEM_METRONOME: "BILLING_SYSTEM_METRONOME", +} as const; + +export type FeatureFlag = (typeof FEATURE_FLAG)[keyof typeof FEATURE_FLAG]; diff --git a/ui/types/lighthouse-skills.ts b/ui/types/lighthouse-skills.ts index e4252829d1..4ec121b1ea 100644 --- a/ui/types/lighthouse-skills.ts +++ b/ui/types/lighthouse-skills.ts @@ -1,9 +1,7 @@ import type { LucideIcon } from "lucide-react"; -// PostHog multivariate flag deciding which skill launcher the finding detail -// drawer renders. Anything other than "dropdown" falls back to the card control. -export const SKILL_LAUNCHER_FLAG = "finding-detail-skill-launcher"; - +// Variants of the FEATURE_FLAG.SKILL_LAUNCHER experiment (see +// lib/feature-flags). Anything other than "dropdown" renders the card control. export const SKILL_LAUNCHER_VARIANT = { CARD: "card", DROPDOWN: "dropdown",