From 8024758a1038eaaa83eccdaed94dc5b00fc84bca Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Wed, 19 Nov 2025 15:05:32 -0500 Subject: [PATCH] add feature flag loading detection --- frontends/main/package.json | 2 +- .../ProductPages/CoursePage.test.tsx | 20 +++-- .../src/app-pages/ProductPages/CoursePage.tsx | 6 +- .../ProductPages/ProgramPage.test.tsx | 20 +++-- .../app-pages/ProductPages/ProgramPage.tsx | 6 +- .../src/common/useFeatureFlagsLoaded.test.ts | 90 +++++++++++++++++++ .../main/src/common/useFeatureFlagsLoaded.ts | 17 ++++ yarn.lock | 40 ++++++--- 8 files changed, 175 insertions(+), 26 deletions(-) create mode 100644 frontends/main/src/common/useFeatureFlagsLoaded.test.ts create mode 100644 frontends/main/src/common/useFeatureFlagsLoaded.ts diff --git a/frontends/main/package.json b/frontends/main/package.json index cfc7859b40..5708120313 100644 --- a/frontends/main/package.json +++ b/frontends/main/package.json @@ -34,7 +34,7 @@ "next-nprogress-bar": "^2.4.2", "ol-components": "0.0.0", "ol-utilities": "0.0.0", - "posthog-js": "^1.157.2", + "posthog-js": "^1.296.1", "react": "^19.0.0", "react-dom": "^19.0.0", "react-slick": "^0.30.2", diff --git a/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx b/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx index 2ef00c0535..c066247c60 100644 --- a/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx @@ -12,10 +12,13 @@ import { assertHeadings } from "ol-test-utilities" import { notFound } from "next/navigation" import { useFeatureFlagEnabled } from "posthog-js/react" +import { useFeatureFlagsLoaded } from "@/common/useFeatureFlagsLoaded" import invariant from "tiny-invariant" jest.mock("posthog-js/react") const mockedUseFeatureFlagEnabled = jest.mocked(useFeatureFlagEnabled) +jest.mock("@/common/useFeatureFlagsLoaded") +const mockedUseFeatureFlagsLoaded = jest.mocked(useFeatureFlagsLoaded) const makeCourse = factories.courses.course const makePage = factories.pages.coursePageItem @@ -46,12 +49,19 @@ const setupApis = ({ describe("CoursePage", () => { beforeEach(() => { mockedUseFeatureFlagEnabled.mockReturnValue(true) + mockedUseFeatureFlagsLoaded.mockReturnValue(true) }) - test.each([true, false])( + test.each([ + { hasLoaded: true, isEnabled: true, shouldNotFound: false }, + { hasLoaded: true, isEnabled: false, shouldNotFound: true }, + { hasLoaded: false, isEnabled: true, shouldNotFound: false }, + { hasLoaded: false, isEnabled: false, shouldNotFound: false }, + ])( "Calls noFound if and only the feature flag is disabled", - async (isEnabled) => { + async ({ isEnabled, hasLoaded, shouldNotFound }) => { mockedUseFeatureFlagEnabled.mockReturnValue(isEnabled) + mockedUseFeatureFlagsLoaded.mockReturnValue(hasLoaded) const course = makeCourse() const page = makePage({ course_details: course }) @@ -60,10 +70,10 @@ describe("CoursePage", () => { url: `/courses/${course.readable_id}/`, }) - if (isEnabled) { - expect(notFound).not.toHaveBeenCalled() - } else { + if (shouldNotFound) { expect(notFound).toHaveBeenCalled() + } else { + expect(notFound).not.toHaveBeenCalled() } }, ) diff --git a/frontends/main/src/app-pages/ProductPages/CoursePage.tsx b/frontends/main/src/app-pages/ProductPages/CoursePage.tsx index c91c40312f..2d877d6839 100644 --- a/frontends/main/src/app-pages/ProductPages/CoursePage.tsx +++ b/frontends/main/src/app-pages/ProductPages/CoursePage.tsx @@ -22,6 +22,7 @@ import ProductPageTemplate, { } from "./ProductPageTemplate" import { CoursePageItem } from "@mitodl/mitxonline-api-axios/v2" import { DEFAULT_RESOURCE_IMG } from "ol-utilities" +import { useFeatureFlagsLoaded } from "@/common/useFeatureFlagsLoaded" type CoursePageProps = { readableId: string @@ -76,10 +77,11 @@ const CoursePage: React.FC = ({ readableId }) => { const page = pages.data?.items[0] const course = courses.data?.results?.[0] const enabled = useFeatureFlagEnabled(FeatureFlags.ProductPageCourse) - if (enabled === false) { + const flagsLoaded = useFeatureFlagsLoaded() + if (!flagsLoaded) return null + if (!enabled) { return notFound() } - if (!enabled) return const doneLoading = pages.isSuccess && courses.isSuccess diff --git a/frontends/main/src/app-pages/ProductPages/ProgramPage.test.tsx b/frontends/main/src/app-pages/ProductPages/ProgramPage.test.tsx index 6e750707d1..f4f0987538 100644 --- a/frontends/main/src/app-pages/ProductPages/ProgramPage.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProgramPage.test.tsx @@ -18,9 +18,12 @@ import { notFound } from "next/navigation" import { useFeatureFlagEnabled } from "posthog-js/react" import invariant from "tiny-invariant" import { ResourceTypeEnum } from "api" +import { useFeatureFlagsLoaded } from "@/common/useFeatureFlagsLoaded" jest.mock("posthog-js/react") const mockedUseFeatureFlagEnabled = jest.mocked(useFeatureFlagEnabled) +jest.mock("@/common/useFeatureFlagsLoaded") +const mockedUseFeatureFlagsLoaded = jest.mocked(useFeatureFlagsLoaded) const makeProgram = factories.programs.program const makePage = factories.pages.programPageItem @@ -65,12 +68,19 @@ const setupApis = ({ describe("ProgramPage", () => { beforeEach(() => { mockedUseFeatureFlagEnabled.mockReturnValue(true) + mockedUseFeatureFlagsLoaded.mockReturnValue(true) }) - test.each([true, false])( + test.each([ + { hasLoaded: true, isEnabled: true, shouldNotFound: false }, + { hasLoaded: true, isEnabled: false, shouldNotFound: true }, + { hasLoaded: false, isEnabled: true, shouldNotFound: false }, + { hasLoaded: false, isEnabled: false, shouldNotFound: false }, + ])( "Calls noFound if and only the feature flag is disabled", - async (isEnabled) => { + async ({ isEnabled, hasLoaded, shouldNotFound }) => { mockedUseFeatureFlagEnabled.mockReturnValue(isEnabled) + mockedUseFeatureFlagsLoaded.mockReturnValue(hasLoaded) const program = makeProgram() const page = makePage({ program_details: program }) @@ -79,10 +89,10 @@ describe("ProgramPage", () => { url: `/programs/${program.readable_id}/`, }) - if (isEnabled) { - expect(notFound).not.toHaveBeenCalled() - } else { + if (shouldNotFound) { expect(notFound).toHaveBeenCalled() + } else { + expect(notFound).not.toHaveBeenCalled() } }, ) diff --git a/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx b/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx index c4142f6940..d59faf6afa 100644 --- a/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx @@ -24,6 +24,7 @@ import { ProgramSummary } from "./ProductSummary" import { DEFAULT_RESOURCE_IMG } from "ol-utilities" import { learningResourceQueries } from "api/hooks/learningResources" import { ResourceTypeEnum } from "api" +import { useFeatureFlagsLoaded } from "@/common/useFeatureFlagsLoaded" type ProgramPageProps = { readableId: string @@ -89,10 +90,11 @@ const ProgramPage: React.FC = ({ readableId }) => { const program = programs.data?.results?.[0] const programResource = programResources.data?.results?.[0] const enabled = useFeatureFlagEnabled(FeatureFlags.ProductPageCourse) - if (enabled === false) { + const flagsLoaded = useFeatureFlagsLoaded() + if (!flagsLoaded) return null + if (!enabled) { return notFound() } - if (!enabled) return const isLoading = pages.isLoading || programs.isLoading || programResources.isLoading diff --git a/frontends/main/src/common/useFeatureFlagsLoaded.test.ts b/frontends/main/src/common/useFeatureFlagsLoaded.test.ts new file mode 100644 index 0000000000..2cf89d41ea --- /dev/null +++ b/frontends/main/src/common/useFeatureFlagsLoaded.test.ts @@ -0,0 +1,90 @@ +import { renderHook, waitFor } from "@testing-library/react" +import { act } from "react" +import { usePostHog } from "posthog-js/react" +import type { PostHog } from "posthog-js" + +// Import the real implementation, not the mock +const { useFeatureFlagsLoaded } = jest.requireActual("./useFeatureFlagsLoaded") + +jest.mock("posthog-js/react", () => { + return { + __esModule: true, + usePostHog: jest.fn(), + } +}) + +const mockUsePostHog = jest.mocked(usePostHog) + +describe("useFeatureFlagsLoaded", () => { + let onFeatureFlagsCallback: + | ((flags: string[], variants: Record) => void) + | null = null + + const createPostHogMock = (hasLoadedFlags: boolean) => { + return { + featureFlags: { + hasLoadedFlags, + }, + onFeatureFlags: jest.fn((callback) => { + onFeatureFlagsCallback = callback + return () => {} // Return cleanup function + }), + } as unknown as PostHog + } + + beforeEach(() => { + onFeatureFlagsCallback = null + }) + + afterEach(() => { + jest.clearAllMocks() + }) + + test("Returns false when flags have not loaded yet", () => { + mockUsePostHog.mockReturnValue(createPostHogMock(false)) + + const { result } = renderHook(() => useFeatureFlagsLoaded()) + + expect(result.current).toBe(false) + }) + + test("Returns true when flags have already loaded", () => { + mockUsePostHog.mockReturnValue(createPostHogMock(true)) + + const { result } = renderHook(() => useFeatureFlagsLoaded()) + + expect(result.current).toBe(true) + }) + + test("Returned value is reactive and re-renders when onFeatureFlags callback runs", async () => { + mockUsePostHog.mockReturnValue(createPostHogMock(false)) + + const { result } = renderHook(() => useFeatureFlagsLoaded()) + + // Initially should be false + expect(result.current).toBe(false) + + // Simulate flags loading by calling the callback + expect(onFeatureFlagsCallback).not.toBeNull() + act(() => { + onFeatureFlagsCallback!([], {}) + }) + + // Wait for the state to update + await waitFor(() => { + expect(result.current).toBe(true) + }) + }) + + test("onFeatureFlags callback is registered on mount", () => { + const mockPostHog = createPostHogMock(false) + mockUsePostHog.mockReturnValue(mockPostHog) + + renderHook(() => useFeatureFlagsLoaded()) + + expect(mockPostHog.onFeatureFlags).toHaveBeenCalledTimes(1) + expect(mockPostHog.onFeatureFlags).toHaveBeenCalledWith( + expect.any(Function), + ) + }) +}) diff --git a/frontends/main/src/common/useFeatureFlagsLoaded.ts b/frontends/main/src/common/useFeatureFlagsLoaded.ts new file mode 100644 index 0000000000..119b7b677d --- /dev/null +++ b/frontends/main/src/common/useFeatureFlagsLoaded.ts @@ -0,0 +1,17 @@ +import { usePostHog } from "posthog-js/react" +import { useState, useEffect } from "react" + +const useFeatureFlagsLoaded = () => { + const posthog = usePostHog() + const [hasLoaded, setHasLoaded] = useState( + posthog.featureFlags.hasLoadedFlags, + ) + useEffect(() => { + posthog.onFeatureFlags(() => { + setHasLoaded(true) + }) + }, [posthog]) + return hasLoaded +} + +export { useFeatureFlagsLoaded } diff --git a/yarn.lock b/yarn.lock index 56d8d1401f..2ea0c789cf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4402,6 +4402,15 @@ __metadata: languageName: node linkType: hard +"@posthog/core@npm:1.5.2": + version: 1.5.2 + resolution: "@posthog/core@npm:1.5.2" + dependencies: + cross-spawn: "npm:^7.0.6" + checksum: 10/157f28357f44ab3fcd42fc1ef2b027b89c5c12dde449df98a1d6b16f9db0c3340e705827029d5cf8ba5f1193465eabe6c3ef7f2beccc25033009fa3b9cfe8416 + languageName: node + linkType: hard + "@prisma/instrumentation@npm:6.13.0": version: 6.13.0 resolution: "@prisma/instrumentation@npm:6.13.0" @@ -9915,6 +9924,13 @@ __metadata: languageName: node linkType: hard +"core-js@npm:^3.38.1": + version: 3.47.0 + resolution: "core-js@npm:3.47.0" + checksum: 10/c02dc6a091c7e6799e3527dc06a428c44bbcff7f8f6ee700ff818b90aa2ebaf1f17b0234146e692811da97cda5a39a6095ecadec9fd1a74b1135103eb0e96cb1 + languageName: node + linkType: hard + "core-util-is@npm:~1.0.0": version: 1.0.3 resolution: "core-util-is@npm:1.0.3" @@ -10029,7 +10045,7 @@ __metadata: languageName: node linkType: hard -"cross-spawn@npm:^7.0.0, cross-spawn@npm:^7.0.2, cross-spawn@npm:^7.0.3": +"cross-spawn@npm:^7.0.0, cross-spawn@npm:^7.0.2, cross-spawn@npm:^7.0.3, cross-spawn@npm:^7.0.6": version: 7.0.6 resolution: "cross-spawn@npm:7.0.6" dependencies: @@ -15327,7 +15343,7 @@ __metadata: ol-components: "npm:0.0.0" ol-test-utilities: "npm:0.0.0" ol-utilities: "npm:0.0.0" - posthog-js: "npm:^1.157.2" + posthog-js: "npm:^1.296.1" react: "npm:^19.0.0" react-dom: "npm:^19.0.0" react-slick: "npm:^0.30.2" @@ -17874,14 +17890,16 @@ __metadata: languageName: node linkType: hard -"posthog-js@npm:^1.157.2": - version: 1.167.0 - resolution: "posthog-js@npm:1.167.0" +"posthog-js@npm:^1.296.1": + version: 1.296.1 + resolution: "posthog-js@npm:1.296.1" dependencies: + "@posthog/core": "npm:1.5.2" + core-js: "npm:^3.38.1" fflate: "npm:^0.4.8" preact: "npm:^10.19.3" - web-vitals: "npm:^4.0.1" - checksum: 10/7b1332cecb86094f9c08394065f459dfcb99d5de72deb79a36f1fa16be6bf88ec3524d1699b8cf1cac3d637fdf307ad95e614477cdbc3ad703d0cecfb2f3374a + web-vitals: "npm:^4.2.4" + checksum: 10/8209cb287674d9b4657f3d27488afabcbf22fc98b9b02a4bbe4652bf0d9151a71484e94cb404fa9b4469fde825f87cb0186a069257bbf725c83e160802ea7805 languageName: node linkType: hard @@ -22352,10 +22370,10 @@ __metadata: languageName: node linkType: hard -"web-vitals@npm:^4.0.1": - version: 4.2.3 - resolution: "web-vitals@npm:4.2.3" - checksum: 10/f4f1b0d6e0dd06b50a1d48c5cbe8a2804f26c5778d7c9bd0a8f591c147fd044f35465a3e95659b9ca801bfd85742e0ac70c3416228c4241d858fcecb8b396503 +"web-vitals@npm:^4.2.4": + version: 4.2.4 + resolution: "web-vitals@npm:4.2.4" + checksum: 10/68cd1c2625a04b26e7eab67110623396afc6c9ef8c3a76f4e780aefe5b7d4ca1691737a0b99119e1d1ca9a463c4d468c0f0090b1875b6d784589d3a4a8503313 languageName: node linkType: hard