refactor(ui): centralize PostHog feature flag reads in ui/lib/feature-flags (#13089)

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);
+    },
+  );
 });

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,
+  );
 }

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");
+  });
+});

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<T extends string>(
+  flag: FeatureFlag,
+  variants: readonly T[],
+  fallback: T,
+): T {
+  const value = useFeatureFlagVariantKey(flag);
+  const match = variants.find((variant) => variant === value);
+  return match ?? fallback;
+}

ui/lib/feature-flags/index.ts

@@ -0,0 +1,2 @@
+export * from "./client";
+export * from "./shared";

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);
+  });
+});

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];

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",