From dd1abc2cb011602a1eb384bb565281d3c6bb8747 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 20 Nov 2025 20:20:55 -0500 Subject: [PATCH 1/3] update posthog, add useFeatureFlagsLoaded --- frontends/main/package.json | 2 +- .../ProductPages/CoursePage.test.tsx | 20 +++++++--- .../src/app-pages/ProductPages/CoursePage.tsx | 8 ++-- .../ProductPages/ProgramPage.test.tsx | 19 ++++++--- .../app-pages/ProductPages/ProgramPage.tsx | 8 ++-- frontends/main/src/common/feature_flags.ts | 9 +++++ .../main/src/common/useFeatureFlagsLoaded.ts | 39 ++++++++++++++++++ .../ConfiguredPostHogProvider.tsx | 26 +++++++----- yarn.lock | 40 ++++++++++++++----- 9 files changed, 134 insertions(+), 37 deletions(-) create mode 100644 frontends/main/src/common/useFeatureFlagsLoaded.ts diff --git a/frontends/main/package.json b/frontends/main/package.json index cfc7859b40..e38f269fff 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.297.2", "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..bea4ef4a2a 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([ + { flagsLoaded: true, isEnabled: true, shouldNotFound: false }, + { flagsLoaded: true, isEnabled: false, shouldNotFound: true }, + { flagsLoaded: false, isEnabled: true, shouldNotFound: false }, + { flagsLoaded: false, isEnabled: false, shouldNotFound: false }, + ])( "Calls noFound if and only the feature flag is disabled", - async (isEnabled) => { + async ({ flagsLoaded, isEnabled, shouldNotFound }) => { mockedUseFeatureFlagEnabled.mockReturnValue(isEnabled) + mockedUseFeatureFlagsLoaded.mockReturnValue(flagsLoaded) 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..aa7db0c4e7 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) { - return notFound() + const flagsLoaded = useFeatureFlagsLoaded() + + if (!enabled) { + return flagsLoaded ? notFound() : null } - 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..8cd71f52ef 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 @@ -67,10 +70,16 @@ describe("ProgramPage", () => { mockedUseFeatureFlagEnabled.mockReturnValue(true) }) - test.each([true, false])( + test.each([ + { flagsLoaded: true, isEnabled: true, shouldNotFound: false }, + { flagsLoaded: true, isEnabled: false, shouldNotFound: true }, + { flagsLoaded: false, isEnabled: true, shouldNotFound: false }, + { flagsLoaded: false, isEnabled: false, shouldNotFound: false }, + ])( "Calls noFound if and only the feature flag is disabled", - async (isEnabled) => { + async ({ flagsLoaded, isEnabled, shouldNotFound }) => { mockedUseFeatureFlagEnabled.mockReturnValue(isEnabled) + mockedUseFeatureFlagsLoaded.mockReturnValue(flagsLoaded) const program = makeProgram() const page = makePage({ program_details: program }) @@ -79,10 +88,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..a3b676d7f5 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) { - return notFound() + const flagsLoaded = useFeatureFlagsLoaded() + + if (!enabled) { + return flagsLoaded ? notFound() : null } - if (!enabled) return const isLoading = pages.isLoading || programs.isLoading || programResources.isLoading diff --git a/frontends/main/src/common/feature_flags.ts b/frontends/main/src/common/feature_flags.ts index f45b5f5c03..84224fbcac 100644 --- a/frontends/main/src/common/feature_flags.ts +++ b/frontends/main/src/common/feature_flags.ts @@ -11,3 +11,12 @@ export enum FeatureFlags { VideoShorts = "video-shorts", ProductPageCourse = "product-page-course", } + +/** + * A special flag that indicates feature flags are in their bootstrapped state, + * not yet loaded from PostHog server. + * + * DO NOT add this flag to PostHog! + */ +export const INTERNAL_BOOTSTRAPPING_FLAG = + "__flags_are_bootstrapped_do_not_add_this_to_posthog__" diff --git a/frontends/main/src/common/useFeatureFlagsLoaded.ts b/frontends/main/src/common/useFeatureFlagsLoaded.ts new file mode 100644 index 0000000000..ac28db9822 --- /dev/null +++ b/frontends/main/src/common/useFeatureFlagsLoaded.ts @@ -0,0 +1,39 @@ +import { INTERNAL_BOOTSTRAPPING_FLAG } from "@/common/feature_flags" +import { useFeatureFlagEnabled, usePostHog } from "posthog-js/react" +import { useEffect, useState } from "react" + +/** + * Returns `true` if feature flags have been loaded via posthog API, else `false`. + * + * NOTES: + * 1. Avoid using this! Really!: + * - This can delay feature flag availability for users who have been to the + * site before. Their flags will be bootstrapped from localStorage, and should + * have the correct values immediately, before contacting the PostHog server. + * - We generally shouldn't care if a flag is "false" or + * "not loaded yet". + * + * USE CASE: One case where this distinction matters is when an entire page + * is behind a feature flag, and we don't want to 404 until the flags are + * loaded. + * 2. Unlike posthog's `onFeatureFlags` callback, this hook enables you to + * distinguish between bootstrapped flags and flags loaded from PostHog server. + */ +const useFeatureFlagsLoaded = () => { + const posthog = usePostHog() + const [hasBootstrapped, setHasBootstrapped] = useState(false) + useEffect(() => { + return posthog.onFeatureFlags(() => { + setHasBootstrapped(true) + }) + }, [posthog]) + const bootstrapFlag = useFeatureFlagEnabled(INTERNAL_BOOTSTRAPPING_FLAG) + /** + * bootstrapFlag will be undefined: + * 1. BEFORE posthog has initialized (nothing bootstrapped yet) + * 2. AFTER posthog has loaded flags from its server. + */ + return hasBootstrapped && bootstrapFlag === undefined +} + +export { useFeatureFlagsLoaded } diff --git a/frontends/main/src/page-components/ConfiguredPostHogProvider/ConfiguredPostHogProvider.tsx b/frontends/main/src/page-components/ConfiguredPostHogProvider/ConfiguredPostHogProvider.tsx index 21d4c17160..0b4a509c3e 100644 --- a/frontends/main/src/page-components/ConfiguredPostHogProvider/ConfiguredPostHogProvider.tsx +++ b/frontends/main/src/page-components/ConfiguredPostHogProvider/ConfiguredPostHogProvider.tsx @@ -2,20 +2,12 @@ import React, { useEffect } from "react" import posthog from "posthog-js" import { PostHogProvider, usePostHog } from "posthog-js/react" import { useUserMe } from "api/hooks/user" +import { INTERNAL_BOOTSTRAPPING_FLAG } from "@/common/feature_flags" const POSTHOG_API_KEY = process.env.NEXT_PUBLIC_POSTHOG_API_KEY const POSTHOG_API_HOST = process.env.NEXT_PUBLIC_POSTHOG_API_HOST const FEATURE_FLAGS = process.env.FEATURE_FLAGS -if (POSTHOG_API_KEY) { - posthog.init(POSTHOG_API_KEY, { - api_host: POSTHOG_API_HOST, - bootstrap: { - featureFlags: FEATURE_FLAGS ? JSON.parse(FEATURE_FLAGS) : null, - }, - }) -} - const PosthogIdentifier = () => { const { data: user } = useUserMe() const posthog = usePostHog() @@ -45,6 +37,22 @@ const PosthogIdentifier = () => { const ConfiguredPostHogProvider: React.FC<{ children: React.ReactNode }> = ({ children, }) => { + useEffect(() => { + if (POSTHOG_API_KEY) { + posthog.init(POSTHOG_API_KEY, { + api_host: POSTHOG_API_HOST, + bootstrap: { + featureFlags: FEATURE_FLAGS + ? { + ...JSON.parse(FEATURE_FLAGS), + [INTERNAL_BOOTSTRAPPING_FLAG]: true, + } + : null, + }, + }) + } + }, []) + if (!POSTHOG_API_KEY) { return children } diff --git a/yarn.lock b/yarn.lock index 56d8d1401f..60eb35f454 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4402,6 +4402,15 @@ __metadata: languageName: node linkType: hard +"@posthog/core@npm:1.5.5": + version: 1.5.5 + resolution: "@posthog/core@npm:1.5.5" + dependencies: + cross-spawn: "npm:^7.0.6" + checksum: 10/f6404b0c5af3676afb37187145f173aab7a7eebf2caaf6831b4645f6fe6683287677bdc77f7239cb448b2d770215aa0eadca89578d83dd73ab6260e907c3a919 + 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.297.2" 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.297.2": + version: 1.297.2 + resolution: "posthog-js@npm:1.297.2" dependencies: + "@posthog/core": "npm:1.5.5" + 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/c41ff83ea60899845942555537894e4c264b143ce3388dfc722191b3e7a4d37a07399f6aebb479ef92cf26e4a04b99f0fc5a7e814a9e0363b4bb5ea40acc5399 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 From bc5095d4cbafa23c5621f9eb15630a009e56325b Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 20 Nov 2025 20:28:55 -0500 Subject: [PATCH 2/3] add a comment --- frontends/main/src/common/useFeatureFlagsLoaded.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/frontends/main/src/common/useFeatureFlagsLoaded.ts b/frontends/main/src/common/useFeatureFlagsLoaded.ts index ac28db9822..eaa9f99f9c 100644 --- a/frontends/main/src/common/useFeatureFlagsLoaded.ts +++ b/frontends/main/src/common/useFeatureFlagsLoaded.ts @@ -23,6 +23,14 @@ const useFeatureFlagsLoaded = () => { const posthog = usePostHog() const [hasBootstrapped, setHasBootstrapped] = useState(false) useEffect(() => { + /** + * NOTE: this does not indicate flags have loaded from server, so they might + * be incomplete. It does at least indicate that localstorage/bootstrapped + * flags are ready + * + * If this event listener is added after posthog has fully loaded flags, + * the callback is still fired. + */ return posthog.onFeatureFlags(() => { setHasBootstrapped(true) }) From 1fc2395a6e15b9b9c02616915bd8bc18b9f875c7 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 20 Nov 2025 20:33:10 -0500 Subject: [PATCH 3/3] add implementation note --- frontends/main/src/common/useFeatureFlagsLoaded.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frontends/main/src/common/useFeatureFlagsLoaded.ts b/frontends/main/src/common/useFeatureFlagsLoaded.ts index eaa9f99f9c..436e51ef2c 100644 --- a/frontends/main/src/common/useFeatureFlagsLoaded.ts +++ b/frontends/main/src/common/useFeatureFlagsLoaded.ts @@ -18,6 +18,12 @@ import { useEffect, useState } from "react" * loaded. * 2. Unlike posthog's `onFeatureFlags` callback, this hook enables you to * distinguish between bootstrapped flags and flags loaded from PostHog server. + * + * IMPLEMENTATION: + * Posthog does not make detecting "flags have loaded from server" easy. + * This approach relies on the fact that bootstrapped flags are completely + * discarded after flags are loaded from server, so `useFlagEnabled` will + * return `undefined` for any flag that was only bootstrapped locally. */ const useFeatureFlagsLoaded = () => { const posthog = usePostHog()