-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: import newremote feature flag controller with override functionality #23487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
579e947
5587dfe
369d995
caaafbf
06336f2
eee57f6
4cdecbd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,10 @@ | ||
| import React, { useCallback, useEffect, useMemo, useState } from 'react'; | ||
| import React, { | ||
| useCallback, | ||
| useEffect, | ||
| useMemo, | ||
| useState, | ||
| useRef, | ||
| } from 'react'; | ||
| import { ScrollView, Alert, TextInput, Switch, View } from 'react-native'; | ||
| import { useNavigation } from '@react-navigation/native'; | ||
| import { useTailwind } from '@metamask/design-system-twrnc-preset'; | ||
|
|
@@ -23,8 +29,12 @@ import { | |
| } from '../../../util/feature-flags'; | ||
| import { useFeatureFlagOverride } from '../../../contexts/FeatureFlagOverrideContext'; | ||
| import { useFeatureFlagStats } from '../../../hooks/useFeatureFlagStats'; | ||
| import { FeatureFlagNames } from '../../hooks/useFeatureFlag'; | ||
|
|
||
| import { | ||
| selectrawRemoteFeatureFlags, | ||
| selectLocalOverrides, | ||
| } from '../../../selectors/featureFlagController'; | ||
| import { useSelector } from 'react-redux'; | ||
| import SelectOptionSheet from '../../UI/SelectOptionSheet'; | ||
| interface FeatureFlagRowProps { | ||
| flag: FeatureFlagInfo; | ||
| onToggle: (key: string, newValue: unknown) => void; | ||
|
|
@@ -35,9 +45,24 @@ export interface MinimumVersionFlagValue { | |
| minimumVersion: string; | ||
| } | ||
| const FeatureFlagRow: React.FC<FeatureFlagRowProps> = ({ flag, onToggle }) => { | ||
| const rawRemoteFeatureFlags = useSelector(selectrawRemoteFeatureFlags); | ||
| const override = useSelector(selectLocalOverrides); | ||
| const tw = useTailwind(); | ||
| const theme = useTheme(); | ||
| const [localValue, setLocalValue] = useState(flag.value); | ||
| const prevIsOverriddenRef = useRef(flag.isOverridden); | ||
|
|
||
| useEffect(() => { | ||
| const wasOverridden = prevIsOverriddenRef.current; | ||
| const isNowOverridden = flag.isOverridden; | ||
|
|
||
| if (wasOverridden && !isNowOverridden) { | ||
| // Reset localValue to flag.value when override is cleared | ||
| setLocalValue(flag.value); | ||
| } | ||
|
|
||
| prevIsOverriddenRef.current = isNowOverridden; | ||
| }, [override, flag.value, flag.isOverridden, flag.key]); | ||
| const minimumVersion = (localValue as MinimumVersionFlagValue) | ||
| ?.minimumVersion; | ||
| const isVersionSupported = useMemo( | ||
|
|
@@ -57,12 +82,7 @@ const FeatureFlagRow: React.FC<FeatureFlagRowProps> = ({ flag, onToggle }) => { | |
| <Box twClassName="items-end"> | ||
| <Switch | ||
| value={(localValue as MinimumVersionFlagValue).enabled} | ||
| disabled={ | ||
| !isVersionSupported && | ||
| !Object.values(FeatureFlagNames).includes( | ||
| flag.key as FeatureFlagNames, | ||
| ) | ||
| } | ||
| disabled={!isVersionSupported} | ||
| onValueChange={(newValue: boolean) => { | ||
| const updatedValue = { | ||
| ...(localValue as MinimumVersionFlagValue), | ||
|
|
@@ -96,11 +116,6 @@ const FeatureFlagRow: React.FC<FeatureFlagRowProps> = ({ flag, onToggle }) => { | |
| case 'boolean': | ||
| return ( | ||
| <Switch | ||
| disabled={ | ||
| !Object.values(FeatureFlagNames).includes( | ||
| flag.key as FeatureFlagNames, | ||
| ) | ||
| } | ||
| value={localValue as boolean} | ||
| onValueChange={(newValue: boolean) => { | ||
| setLocalValue(newValue); | ||
|
|
@@ -114,6 +129,41 @@ const FeatureFlagRow: React.FC<FeatureFlagRowProps> = ({ flag, onToggle }) => { | |
| ios_backgroundColor={theme.colors.border.muted} | ||
| /> | ||
| ); | ||
| case 'abTest': { | ||
| interface AbTestType { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move type outside of component |
||
| name: string; | ||
| value: unknown; | ||
| } | ||
| const abTestOptions: AbTestType[] = Array.isArray( | ||
| rawRemoteFeatureFlags[flag.key], | ||
| ) | ||
| ? (rawRemoteFeatureFlags[flag.key] as AbTestType[]) | ||
| : []; | ||
| const flagValue = flag.value as AbTestType; | ||
|
|
||
| const handleSelectOption = (name: string) => { | ||
| const selectedOption = abTestOptions.find( | ||
| (option: { name: string }) => option.name === name, | ||
| ); | ||
| setLocalValue(selectedOption); | ||
| onToggle(flag.key, selectedOption); | ||
| }; | ||
|
|
||
| return ( | ||
| <Box twClassName="flex-1 ml-2 justify-center min-w-[160px]"> | ||
| <SelectOptionSheet | ||
| options={abTestOptions.map((option: AbTestType) => ({ | ||
| label: option.name, | ||
| value: option.name, | ||
| }))} | ||
| label={flag.key} | ||
| defaultValue={flagValue.name} | ||
| onValueChange={handleSelectOption} | ||
| selectedValue={(localValue as AbTestType).name} | ||
| /> | ||
| </Box> | ||
| ); | ||
| } | ||
| case 'string': | ||
| case 'number': | ||
| return ( | ||
|
|
@@ -255,8 +305,14 @@ const FeatureFlagOverride: React.FC = () => { | |
| const tw = useTailwind(); | ||
|
|
||
| const flagStats = useFeatureFlagStats(); | ||
| const { setOverride, removeOverride, clearAllOverrides, featureFlagsList } = | ||
| useFeatureFlagOverride(); | ||
| const { | ||
| setOverride, | ||
| removeOverride, | ||
| clearAllOverrides, | ||
| featureFlagsList, | ||
| getOverrideCount, | ||
| } = useFeatureFlagOverride(); | ||
| const overrideCount = getOverrideCount(); | ||
|
|
||
| const [searchQuery, setSearchQuery] = useState(''); | ||
| const [typeFilter, setTypeFilter] = useState<'all' | 'boolean'>('all'); | ||
|
|
@@ -426,7 +482,7 @@ const FeatureFlagOverride: React.FC = () => { | |
| size={ButtonSize.Sm} | ||
| onPress={handleClearAllOverrides} | ||
| > | ||
| Clear All Overrides | ||
| {`Clear All Overrides (${overrideCount})`} | ||
| </Button> | ||
| </Box> | ||
| </Box> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,17 @@ | ||
| import React, { | ||
| createContext, | ||
| useContext, | ||
| useState, | ||
| useCallback, | ||
| ReactNode, | ||
| useMemo, | ||
| useEffect, | ||
| } from 'react'; | ||
| import { useSelector } from 'react-redux'; | ||
| import { selectRemoteFeatureFlags } from '../selectors/featureFlagController'; | ||
| import { | ||
| selectRemoteFeatureFlags, | ||
| selectLocalOverrides, | ||
| selectRawFeatureFlags, | ||
| } from '../selectors/featureFlagController'; | ||
| import { | ||
| FeatureFlagInfo, | ||
| getFeatureFlagDescription, | ||
|
|
@@ -20,11 +24,48 @@ import { | |
| } from '../component-library/components/Toast'; | ||
| import { MinimumVersionFlagValue } from '../components/Views/FeatureFlagOverride/FeatureFlagOverride'; | ||
| import useMetrics from '../components/hooks/useMetrics/useMetrics'; | ||
| import Engine from '../core/Engine'; | ||
| import type { Json } from '@metamask/utils'; | ||
| import type { RemoteFeatureFlagController } from '@metamask/remote-feature-flag-controller'; | ||
|
|
||
| interface FeatureFlagOverrides { | ||
| [key: string]: unknown; | ||
| } | ||
|
|
||
| // Extended interface for controller methods not in the base type definition | ||
| // These methods exist at runtime in the mobile app's version of RemoteFeatureFlagController | ||
| // but are not included in the @metamask/remote-feature-flag-controller type definitions | ||
| export interface ExtendedRemoteFeatureFlagController | ||
| extends RemoteFeatureFlagController { | ||
| setFlagOverride: (key: string, value: Json) => void; | ||
| clearFlagOverride: (key: string) => void; | ||
| getAllFlags: () => FeatureFlagOverrides; | ||
| clearAllOverrides: () => void; | ||
| } | ||
|
|
||
| // Helper to safely access the RemoteFeatureFlagController with proper typing | ||
| const getRemoteFeatureFlagController = (): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does not need to be a function since it's just returning a reference |
||
| | ExtendedRemoteFeatureFlagController | ||
| | undefined => Engine.context?.RemoteFeatureFlagController as | ||
| | ExtendedRemoteFeatureFlagController | ||
| | undefined; | ||
|
|
||
| // Helper to safely execute controller methods with error handling | ||
| const withRemoteFeatureFlagController = ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the
|
||
| fn: (controller: ExtendedRemoteFeatureFlagController) => void, | ||
| errorMessage: string, | ||
| ): void => { | ||
| const controller = getRemoteFeatureFlagController(); | ||
| if (!controller) { | ||
| return; | ||
| } | ||
| try { | ||
| fn(controller); | ||
| } catch (error) { | ||
| console.error(errorMessage, error); | ||
| } | ||
| }; | ||
|
|
||
| export interface FeatureFlagOverrideContextType { | ||
| featureFlags: { [key: string]: FeatureFlagInfo }; | ||
| originalFlags: FeatureFlagOverrides; | ||
|
|
@@ -35,9 +76,6 @@ export interface FeatureFlagOverrideContextType { | |
| removeOverride: (key: string) => void; | ||
| clearAllOverrides: () => void; | ||
| hasOverride: (key: string) => boolean; | ||
| getOverride: (key: string) => unknown; | ||
| getAllOverrides: () => FeatureFlagOverrides; | ||
| applyOverrides: (originalFlags: FeatureFlagOverrides) => FeatureFlagOverrides; | ||
| getOverrideCount: () => number; | ||
| } | ||
|
|
||
|
|
@@ -54,70 +92,71 @@ export const FeatureFlagOverrideProvider: React.FC< | |
| > = ({ children }) => { | ||
| const { addTraitsToUser } = useMetrics(); | ||
| // Get the initial feature flags from Redux | ||
| const rawFeatureFlagsSelected = useSelector(selectRemoteFeatureFlags); | ||
| const rawFeatureFlags = useMemo( | ||
| () => rawFeatureFlagsSelected || {}, | ||
| [rawFeatureFlagsSelected], | ||
| ); | ||
| const featureFlagsWithOverrides = useSelector(selectRemoteFeatureFlags); | ||
| const rawFeatureFlags = useSelector(selectRawFeatureFlags); | ||
|
|
||
| // Get overrides from controller state via Redux | ||
| const overrides = useSelector(selectLocalOverrides); | ||
| const toastContext = useContext(ToastContext); | ||
| const toastRef = toastContext?.toastRef; | ||
|
|
||
| // Local state for overrides | ||
| const [overrides, setOverrides] = useState<FeatureFlagOverrides>({}); | ||
| // Subscribe to controller state changes to ensure we stay in sync | ||
| useEffect(() => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't seem like it should do anything since it's triggering an empty function from inside of the useEffect. What's the intended purpose here - is it to get the latest state every time the feature flag changes? |
||
| const handler = () => { | ||
| // State change will trigger Redux update via selector | ||
| // No need to do anything here as Redux will handle the update | ||
| }; | ||
|
|
||
| try { | ||
| Engine.controllerMessenger?.subscribe( | ||
| 'RemoteFeatureFlagController:stateChange', | ||
| handler, | ||
| ); | ||
| } catch (error) { | ||
| // Engine might not be fully initialized yet, ignore error | ||
| console.warn( | ||
| 'Failed to subscribe to RemoteFeatureFlagController state changes:', | ||
| error, | ||
| ); | ||
| } | ||
|
|
||
| return () => { | ||
| // Note: Messenger subscribe doesn't return unsubscribe, but the subscription | ||
| // will be cleaned up when the component unmounts | ||
| }; | ||
| }, []); | ||
|
|
||
| const setOverride = useCallback((key: string, value: unknown) => { | ||
| setOverrides((prev) => ({ | ||
| ...prev, | ||
| [key]: value, | ||
| })); | ||
| withRemoteFeatureFlagController((controller) => { | ||
| // Use the controller's setFlagOverride method which properly updates localOverrides in state | ||
| controller.setFlagOverride(key, value as Json); | ||
| }, 'Failed to set feature flag override:'); | ||
| }, []); | ||
|
|
||
| const removeOverride = useCallback((key: string) => { | ||
| setOverrides((prev) => { | ||
| const newOverrides = { ...prev }; | ||
| delete newOverrides[key]; | ||
| return newOverrides; | ||
| }); | ||
| withRemoteFeatureFlagController( | ||
| (controller) => controller.clearFlagOverride(key), | ||
| 'Failed to remove feature flag override:', | ||
| ); | ||
| }, []); | ||
|
|
||
| const clearAllOverrides = useCallback(() => { | ||
| setOverrides({}); | ||
| withRemoteFeatureFlagController( | ||
| (controller) => controller.clearAllOverrides(), | ||
| 'Failed to clear feature flag overrides:', | ||
| ); | ||
| }, []); | ||
|
|
||
| const hasOverride = useCallback( | ||
| (key: string): boolean => key in overrides, | ||
| [overrides], | ||
| ); | ||
|
|
||
| const getOverride = useCallback( | ||
| (key: string): unknown => overrides[key], | ||
| [overrides], | ||
| ); | ||
|
|
||
| const getAllOverrides = useCallback( | ||
| (): FeatureFlagOverrides => ({ ...overrides }), | ||
| [overrides], | ||
| ); | ||
|
|
||
| const applyOverrides = useCallback( | ||
| (originalFlags: FeatureFlagOverrides): FeatureFlagOverrides => ({ | ||
| ...originalFlags, | ||
| ...overrides, | ||
| }), | ||
| [overrides], | ||
| ); | ||
|
|
||
| const featureFlagsWithOverrides = useMemo( | ||
| () => applyOverrides(rawFeatureFlags), | ||
| [rawFeatureFlags, applyOverrides], | ||
| ); | ||
|
|
||
| const featureFlags = useMemo(() => { | ||
| // Get all unique keys from both raw and overridden flags | ||
| const allKeys = new Set([ | ||
| ...Object.keys(rawFeatureFlags), | ||
| ...Object.keys(featureFlagsWithOverrides), | ||
| ...Object.keys(getAllOverrides()), | ||
| ]); | ||
| const allFlags: { [key: string]: FeatureFlagInfo } = {}; | ||
|
|
||
|
|
@@ -138,12 +177,7 @@ export const FeatureFlagOverrideProvider: React.FC< | |
| allFlags[key] = flagValue; | ||
| }); | ||
| return allFlags; | ||
| }, [ | ||
| rawFeatureFlags, | ||
| featureFlagsWithOverrides, | ||
| hasOverride, | ||
| getAllOverrides, | ||
| ]); | ||
| }, [rawFeatureFlags, featureFlagsWithOverrides, hasOverride]); | ||
|
|
||
| const featureFlagsList = useMemo( | ||
| () => | ||
|
|
@@ -224,9 +258,6 @@ export const FeatureFlagOverrideProvider: React.FC< | |
| removeOverride, | ||
| clearAllOverrides, | ||
| hasOverride, | ||
| getOverride, | ||
| getAllOverrides, | ||
| applyOverrides, | ||
| getOverrideCount, | ||
| }), | ||
| [ | ||
|
|
@@ -239,9 +270,6 @@ export const FeatureFlagOverrideProvider: React.FC< | |
| removeOverride, | ||
| clearAllOverrides, | ||
| hasOverride, | ||
| getOverride, | ||
| getAllOverrides, | ||
| applyOverrides, | ||
| getOverrideCount, | ||
| ], | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While extracting types, also cleaner to extract these into enums