From fb5429bfe7a14c0eb4ae9657e4eb7e00a4ca4250 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Thu, 27 Nov 2025 17:21:51 -0800 Subject: [PATCH] Remove Feature Flags --- .../components/SidebarProvider.tsx | 11 +- ui/litellm-dashboard/src/app/layout.tsx | 8 +- ui/litellm-dashboard/src/app/page.tsx | 2 - .../src/components/navbar.tsx | 16 +- .../src/components/public_model_hub.test.tsx | 7 +- .../src/hooks/useFeatureFlags.test.tsx | 296 ------------------ .../src/hooks/useFeatureFlags.tsx | 137 -------- 7 files changed, 6 insertions(+), 471 deletions(-) delete mode 100644 ui/litellm-dashboard/src/hooks/useFeatureFlags.test.tsx delete mode 100644 ui/litellm-dashboard/src/hooks/useFeatureFlags.tsx diff --git a/ui/litellm-dashboard/src/app/(dashboard)/components/SidebarProvider.tsx b/ui/litellm-dashboard/src/app/(dashboard)/components/SidebarProvider.tsx index cb7f0f1a32..c522d4ce1e 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/components/SidebarProvider.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/components/SidebarProvider.tsx @@ -1,21 +1,16 @@ -import useFeatureFlags from "@/hooks/useFeatureFlags"; -import Sidebar from "@/components/leftnav"; -import Sidebar2 from "@/app/(dashboard)/components/Sidebar2"; import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; +import Sidebar from "@/components/leftnav"; interface SidebarProviderProps { + setPage: (page: string) => void; defaultSelectedKey: string; - setPage: (newPage: string) => void; sidebarCollapsed: boolean; } const SidebarProvider = ({ setPage, defaultSelectedKey, sidebarCollapsed }: SidebarProviderProps) => { - const { refactoredUIFlag } = useFeatureFlags(); const { accessToken, userRole } = useAuthorized(); - return refactoredUIFlag ? ( - - ) : ( + return ( ) { return ( - - - - {children} - - + {children} ); } diff --git a/ui/litellm-dashboard/src/app/page.tsx b/ui/litellm-dashboard/src/app/page.tsx index 56c35804df..422d140e07 100644 --- a/ui/litellm-dashboard/src/app/page.tsx +++ b/ui/litellm-dashboard/src/app/page.tsx @@ -40,7 +40,6 @@ import UIThemeSettings from "@/components/ui_theme_settings"; import { CostTrackingSettings } from "@/components/CostTrackingSettings"; import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; import { cx } from "@/lib/cva.config"; -import useFeatureFlags from "@/hooks/useFeatureFlags"; import SidebarProvider from "@/app/(dashboard)/components/SidebarProvider"; import OldTeams from "@/components/OldTeams"; import { SearchTools } from "@/components/search_tools"; @@ -147,7 +146,6 @@ export default function CreateKeyPage() { const [createClicked, setCreateClicked] = useState(false); const [authLoading, setAuthLoading] = useState(true); const [userID, setUserID] = useState(null); - const { refactoredUIFlag } = useFeatureFlags(); const invitation_id = searchParams.get("invitation_id"); diff --git a/ui/litellm-dashboard/src/components/navbar.tsx b/ui/litellm-dashboard/src/components/navbar.tsx index 667691168c..9113a0f54c 100644 --- a/ui/litellm-dashboard/src/components/navbar.tsx +++ b/ui/litellm-dashboard/src/components/navbar.tsx @@ -1,7 +1,7 @@ import Link from "next/link"; import React, { useState, useEffect } from "react"; import type { MenuProps } from "antd"; -import { Dropdown, Tooltip, Switch } from "antd"; +import { Dropdown, Tooltip } from "antd"; import { getProxyBaseUrl } from "@/components/networking"; import { UserOutlined, @@ -15,7 +15,6 @@ import { import { clearTokenCookies } from "@/utils/cookieUtils"; import { fetchProxySettings } from "@/utils/proxyUtils"; import { useTheme } from "@/contexts/ThemeContext"; -import useFeatureFlags from "@/hooks/useFeatureFlags"; interface NavbarProps { userID: string | null; @@ -45,7 +44,6 @@ const Navbar: React.FC = ({ const baseUrl = getProxyBaseUrl(); const [logoutUrl, setLogoutUrl] = useState(""); const { logoUrl } = useTheme(); - const { refactoredUIFlag, setRefactoredUIFlag } = useFeatureFlags(); // Simple logo URL: use custom logo if available, otherwise default const imageUrl = logoUrl || `${baseUrl}/get_image`; @@ -114,18 +112,6 @@ const Navbar: React.FC = ({ {userEmail || "Unknown"} - - {/* NEW: Feature flag label + toggle below the email field */} -
- Refactored UI - setRefactoredUIFlag(checked)} - aria-label="Toggle refactored UI feature flag" - /> -
), diff --git a/ui/litellm-dashboard/src/components/public_model_hub.test.tsx b/ui/litellm-dashboard/src/components/public_model_hub.test.tsx index 8ff57b49e0..47a44fa514 100644 --- a/ui/litellm-dashboard/src/components/public_model_hub.test.tsx +++ b/ui/litellm-dashboard/src/components/public_model_hub.test.tsx @@ -1,7 +1,6 @@ import { describe, it, expect, vi, beforeAll, beforeEach } from "vitest"; import { render } from "@testing-library/react"; import PublicModelHub from "./public_model_hub"; -import { FeatureFlagsProvider } from "@/hooks/useFeatureFlags"; vi.mock("next/navigation", () => ({ useRouter: vi.fn(() => ({ @@ -58,11 +57,7 @@ beforeEach(() => { describe("PublicModelHub", () => { it("renders", () => { - const { container } = render( - - - , - ); + const { container } = render(); expect(container).toBeInTheDocument(); }); }); diff --git a/ui/litellm-dashboard/src/hooks/useFeatureFlags.test.tsx b/ui/litellm-dashboard/src/hooks/useFeatureFlags.test.tsx deleted file mode 100644 index ca0529b0f2..0000000000 --- a/ui/litellm-dashboard/src/hooks/useFeatureFlags.test.tsx +++ /dev/null @@ -1,296 +0,0 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; -import { renderHook, waitFor } from "@testing-library/react"; -import { useRouter } from "next/navigation"; -import useFeatureFlags, { FeatureFlagsProvider } from "./useFeatureFlags"; - -// Mock next/navigation -vi.mock("next/navigation", () => ({ - useRouter: vi.fn(), -})); - -// Mock the networking module to control serverRootPath -vi.mock("@/components/networking", () => ({ - serverRootPath: "/", -})); - -describe("useFeatureFlags", () => { - let mockReplace: ReturnType; - let originalLocation: Location; - - beforeEach(() => { - // Mock router - mockReplace = vi.fn(); - (useRouter as ReturnType).mockReturnValue({ - replace: mockReplace, - }); - - // Store original location - originalLocation = window.location; - - // Mock localStorage - Storage.prototype.getItem = vi.fn(() => null); - Storage.prototype.setItem = vi.fn(); - Storage.prototype.removeItem = vi.fn(); - }); - - afterEach(() => { - vi.clearAllMocks(); - // Restore location - Object.defineProperty(window, "location", { - writable: true, - value: originalLocation, - }); - }); - - describe("FeatureFlagsProvider - redirect logic", () => { - it("should not redirect when refactoredUIFlag is true", async () => { - // Set flag to true - Storage.prototype.getItem = vi.fn(() => "true"); - - const { result } = renderHook(() => useFeatureFlags(), { - wrapper: FeatureFlagsProvider, - }); - - expect(result.current.refactoredUIFlag).toBe(true); - - // Wait for any effects - await waitFor(() => { - expect(mockReplace).not.toHaveBeenCalled(); - }); - }); - - it("should not redirect when already on a /ui path (race condition protection)", async () => { - // Set flag to false to trigger redirect logic - Storage.prototype.getItem = vi.fn(() => "false"); - - // Mock window.location to be on a custom UI path - delete (window as any).location; - window.location = { - pathname: "/my-custom-path/ui/", - } as Location; - - renderHook(() => useFeatureFlags(), { - wrapper: FeatureFlagsProvider, - }); - - // Wait for timeout and check redirect was NOT called - await new Promise((resolve) => setTimeout(resolve, 150)); - - expect(mockReplace).not.toHaveBeenCalled(); - }); - - it("should not redirect when on /ui path without custom root", async () => { - // Set flag to false - Storage.prototype.getItem = vi.fn(() => "false"); - - // Mock window.location to be on standard UI path - delete (window as any).location; - window.location = { - pathname: "/ui/", - } as Location; - - renderHook(() => useFeatureFlags(), { - wrapper: FeatureFlagsProvider, - }); - - // Wait for timeout and check redirect was NOT called - await new Promise((resolve) => setTimeout(resolve, 150)); - - expect(mockReplace).not.toHaveBeenCalled(); - }); - - it("should redirect when flag is false and not on a /ui path", async () => { - // Set flag to false - Storage.prototype.getItem = vi.fn(() => "false"); - - // Mock window.location to be on a non-UI path - delete (window as any).location; - window.location = { - pathname: "/some-other-path/", - } as Location; - - renderHook(() => useFeatureFlags(), { - wrapper: FeatureFlagsProvider, - }); - - // Wait for timeout plus a bit more - await new Promise((resolve) => setTimeout(resolve, 150)); - - // Should have called replace to redirect to base path - expect(mockReplace).toHaveBeenCalledWith("/"); - }); - - it("should not redirect if already at base path", async () => { - // Set flag to false - Storage.prototype.getItem = vi.fn(() => "false"); - - // Mock window.location to be at root - delete (window as any).location; - window.location = { - pathname: "/", - } as Location; - - renderHook(() => useFeatureFlags(), { - wrapper: FeatureFlagsProvider, - }); - - // Wait for timeout - await new Promise((resolve) => setTimeout(resolve, 150)); - - expect(mockReplace).not.toHaveBeenCalled(); - }); - }); - - describe("useFeatureFlags - flag management", () => { - it("should initialize with false when no value in localStorage", () => { - Storage.prototype.getItem = vi.fn(() => null); - - const { result } = renderHook(() => useFeatureFlags(), { - wrapper: FeatureFlagsProvider, - }); - - expect(result.current.refactoredUIFlag).toBe(false); - }); - - it("should initialize with true when localStorage has true", () => { - Storage.prototype.getItem = vi.fn(() => "true"); - - const { result } = renderHook(() => useFeatureFlags(), { - wrapper: FeatureFlagsProvider, - }); - - expect(result.current.refactoredUIFlag).toBe(true); - }); - - it("should update localStorage when setRefactoredUIFlag is called", () => { - const setItemMock = vi.fn(); - Storage.prototype.setItem = setItemMock; - Storage.prototype.getItem = vi.fn(() => "false"); - - const { result } = renderHook(() => useFeatureFlags(), { - wrapper: FeatureFlagsProvider, - }); - - result.current.setRefactoredUIFlag(true); - - expect(setItemMock).toHaveBeenCalledWith( - "feature.refactoredUIFlag", - "true" - ); - }); - - it("should handle malformed localStorage values gracefully", () => { - Storage.prototype.getItem = vi.fn(() => "invalid-value"); - - const { result } = renderHook(() => useFeatureFlags(), { - wrapper: FeatureFlagsProvider, - }); - - // Should default to false for malformed values - expect(result.current.refactoredUIFlag).toBe(false); - }); - }); - - describe("getBasePath logic with serverRootPath", () => { - it("should handle serverRootPath being set to custom path", async () => { - // Mock the networking module with custom serverRootPath - vi.doMock("@/components/networking", () => ({ - serverRootPath: "/my-custom-path", - })); - - // Set flag to false to trigger redirect - Storage.prototype.getItem = vi.fn(() => "false"); - - // Mock location to be on wrong path - delete (window as any).location; - window.location = { - pathname: "/wrong-path/", - } as Location; - - renderHook(() => useFeatureFlags(), { - wrapper: FeatureFlagsProvider, - }); - - // Wait for timeout - await new Promise((resolve) => setTimeout(resolve, 150)); - - // With default NEXT_PUBLIC_BASE_URL being empty, should redirect to "/" - // (In reality, with serverRootPath="/my-custom-path", it would be "/my-custom-path/") - expect(mockReplace).toHaveBeenCalled(); - }); - }); - - describe("storage event synchronization", () => { - it("should update flag when storage event is fired", async () => { - Storage.prototype.getItem = vi.fn(() => "false"); - - const { result } = renderHook(() => useFeatureFlags(), { - wrapper: FeatureFlagsProvider, - }); - - expect(result.current.refactoredUIFlag).toBe(false); - - // Simulate storage event from another tab - const storageEvent = new StorageEvent("storage", { - key: "feature.refactoredUIFlag", - newValue: "true", - }); - - window.dispatchEvent(storageEvent); - - await waitFor(() => { - expect(result.current.refactoredUIFlag).toBe(true); - }); - }); - - it("should self-heal when storage key is cleared", async () => { - const setItemMock = vi.fn(); - Storage.prototype.setItem = setItemMock; - Storage.prototype.getItem = vi.fn(() => "true"); - - renderHook(() => useFeatureFlags(), { - wrapper: FeatureFlagsProvider, - }); - - // Simulate storage event where key was cleared - const storageEvent = new StorageEvent("storage", { - key: "feature.refactoredUIFlag", - newValue: null, - }); - - window.dispatchEvent(storageEvent); - - await waitFor(() => { - expect(setItemMock).toHaveBeenCalledWith( - "feature.refactoredUIFlag", - "false" - ); - }); - }); - }); - - describe("timeout cleanup", () => { - it("should cleanup timeout on unmount", async () => { - Storage.prototype.getItem = vi.fn(() => "false"); - - delete (window as any).location; - window.location = { - pathname: "/some-path/", - } as Location; - - const { unmount } = renderHook(() => useFeatureFlags(), { - wrapper: FeatureFlagsProvider, - }); - - // Unmount immediately before timeout fires - unmount(); - - // Wait past the timeout - await new Promise((resolve) => setTimeout(resolve, 150)); - - // Should not have called replace since component unmounted - expect(mockReplace).not.toHaveBeenCalled(); - }); - }); -}); - diff --git a/ui/litellm-dashboard/src/hooks/useFeatureFlags.tsx b/ui/litellm-dashboard/src/hooks/useFeatureFlags.tsx deleted file mode 100644 index 03b4465b09..0000000000 --- a/ui/litellm-dashboard/src/hooks/useFeatureFlags.tsx +++ /dev/null @@ -1,137 +0,0 @@ -"use client"; - -import React, { createContext, useContext, useEffect, useState } from "react"; -import { useRouter } from "next/navigation"; -import { serverRootPath } from "@/components/networking"; - -const getBasePath = () => { - const raw = process.env.NEXT_PUBLIC_BASE_URL ?? ""; - const trimmed = raw.replace(/^\/+|\/+$/g, ""); // strip leading/trailing slashes - const uiPath = trimmed ? `/${trimmed}/` : "/"; - - // If serverRootPath is set and not "/", prepend it to the UI path - if (serverRootPath && serverRootPath !== "/") { - // Remove trailing slash from serverRootPath and ensure uiPath has no leading slash for proper joining - const cleanServerRoot = serverRootPath.replace(/\/+$/, ""); - const cleanUiPath = uiPath.replace(/^\/+/, ""); - return `${cleanServerRoot}/${cleanUiPath}`; - } - - return uiPath; -} - -type Flags = { - refactoredUIFlag: boolean; - setRefactoredUIFlag: (v: boolean) => void; -}; - -const STORAGE_KEY = "feature.refactoredUIFlag"; - -const FeatureFlagsCtx = createContext(null); - -/** Safely read the flag from localStorage. If anything goes wrong, reset to false. */ -function readFlagSafely(): boolean { - try { - const raw = localStorage.getItem(STORAGE_KEY); - if (raw === null) { - localStorage.setItem(STORAGE_KEY, "false"); - return false; - } - - const v = raw.trim().toLowerCase(); - if (v === "true" || v === "1") return true; - if (v === "false" || v === "0") return false; - - // Last chance: try JSON.parse in case something odd was stored. - const parsed = JSON.parse(raw); - if (typeof parsed === "boolean") return parsed; - - // Malformed → reset to false - localStorage.setItem(STORAGE_KEY, "false"); - return false; - } catch { - // If even accessing localStorage throws, best effort reset then default to false - try { - localStorage.setItem(STORAGE_KEY, "false"); - } catch {} - return false; - } -} - -function writeFlagSafely(v: boolean) { - try { - localStorage.setItem(STORAGE_KEY, String(v)); - } catch { - // Ignore write errors; state will still reflect the intended value. - } -} - -export const FeatureFlagsProvider = ({ children }: { children: React.ReactNode }) => { - const router = useRouter(); // ⟵ add this - - // Lazy init reads from localStorage only on the client - const [refactoredUIFlag, setRefactoredUIFlagState] = useState(() => readFlagSafely()); - - const setRefactoredUIFlag = (v: boolean) => { - setRefactoredUIFlagState(v); - writeFlagSafely(v); - }; - - // Keep this flag in sync across tabs/windows. - useEffect(() => { - const onStorage = (e: StorageEvent) => { - if (e.key === STORAGE_KEY && e.newValue != null) { - const next = e.newValue.trim().toLowerCase(); - setRefactoredUIFlagState(next === "true" || next === "1"); - } - // If the key was cleared elsewhere, self-heal to false. - if (e.key === STORAGE_KEY && e.newValue === null) { - writeFlagSafely(false); - setRefactoredUIFlagState(false); - } - }; - window.addEventListener("storage", onStorage); - return () => window.removeEventListener("storage", onStorage); - }, []); - - // Redirect to base path the moment the flag is OFF. - useEffect(() => { - if (refactoredUIFlag) return; // only act when turned off - - // Wait a moment for serverRootPath to be initialized from getUiConfig() - // This prevents a race condition where we redirect before knowing the correct path - const checkAndRedirect = () => { - const base = getBasePath(); - const normalize = (p: string) => (p.endsWith("/") ? p : p + "/"); - const current = normalize(window.location.pathname); - - // Don't redirect if we're already on a UI path (even if serverRootPath hasn't loaded yet) - // This handles the case where the page is mounted at a custom server root path - if (current.includes("/ui")) { - return; - } - - // Avoid a redirect loop if we're already at the base path. - if (current !== base) { - // Replace so the "off" redirect doesn't pollute history. - router.replace(base); - } - }; - - // Small delay to allow serverRootPath to be set by getUiConfig() - const timeoutId = setTimeout(checkAndRedirect, 100); - return () => clearTimeout(timeoutId); - }, [refactoredUIFlag, router]); - - return ( - {children} - ); -}; - -const useFeatureFlags = () => { - const ctx = useContext(FeatureFlagsCtx); - if (!ctx) throw new Error("useFeatureFlags must be used within FeatureFlagsProvider"); - return ctx; -}; - -export default useFeatureFlags;