Skip to content

Commit f06f9c9

Browse files
yungstersfacebook-github-bot
authored andcommitted
RN: Remove Feature Flag Override Argument (#53513)
Summary: Pull Request resolved: #53513 D62853299 introduced the `defaultValue` argument to feature flag override functions, with the intent of enabling override functions to do something like this: ``` myFeatureFlag: (defaultValueForFlag) => someCondition ? value : defaultValueForFlag ``` However, there are no current use cases for this. This particular use case can also be solved by expanding support for override functions to return `null` or `undefined` which falls back to using the default value. Furthermore, the type system has a difficult time representing the constraints when there are non-boolean JavaScript-only overrides (which was introduced recently). This diff removes the argument and adds support for override functions to return `null` or `undefined`. Changelog: [Internal] Reviewed By: lunaleaps Differential Revision: D81163557 fbshipit-source-id: 38876c83d51d857dbea889928248410041c5d6d7
1 parent 41a1467 commit f06f9c9

File tree

2 files changed

+18
-12
lines changed

2 files changed

+18
-12
lines changed

packages/react-native/src/private/featureflags/ReactNativeFeatureFlagsBase.js

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ const clearCachedValuesFns: Array<() => void> = [];
2424

2525
export type Getter<T> = () => T;
2626

27-
// This defines the types for the overrides object, whose methods also receive
28-
// the default value as a parameter.
27+
// This defines the types for the overrides object, whose methods can return
28+
// null or undefined to fallback to the default value.
2929
export type OverridesFor<T> = Partial<{
30-
[key in keyof T]: (ReturnType<T[key]>) => ReturnType<T[key]>,
30+
[key in keyof T]: Getter<?ReturnType<T[key]>>,
3131
}>;
3232

3333
function createGetter<T: boolean | number | string>(
@@ -61,8 +61,7 @@ export function createJavaScriptFlagGetter<
6161
configName,
6262
() => {
6363
accessedFeatureFlags.add(configName);
64-
// $FlowFixMe[incompatible-type] - `defaultValue` is not refined.
65-
return overrides?.[configName]?.(defaultValue);
64+
return overrides?.[configName]?.();
6665
},
6766
defaultValue,
6867
);

packages/react-native/src/private/featureflags/__tests__/ReactNativeFeatureFlags-test.js

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ describe('ReactNativeFeatureFlags', () => {
6161
it('should access and cache overridden JS-only flags', () => {
6262
const ReactNativeFeatureFlags = require('../ReactNativeFeatureFlags');
6363

64-
const jsOnlyTestFlagFn = jest.fn((defaultValue: boolean) => true);
64+
const jsOnlyTestFlagFn = jest.fn(() => true);
6565
ReactNativeFeatureFlags.override({
6666
jsOnlyTestFlag: jsOnlyTestFlagFn,
6767
});
@@ -117,17 +117,24 @@ describe('ReactNativeFeatureFlags', () => {
117117
).toThrow('Feature flags cannot be overridden more than once');
118118
});
119119

120-
it('should pass the default value of the feature flag to the function that provides its override', () => {
120+
it('should evaluate to default value if the override returns null', () => {
121121
const ReactNativeFeatureFlags = require('../ReactNativeFeatureFlags');
122122

123123
ReactNativeFeatureFlags.override({
124-
jsOnlyTestFlag: (defaultValue: boolean) => {
125-
expect(defaultValue).toBe(false);
126-
return true;
127-
},
124+
jsOnlyTestFlag: () => null,
128125
});
129126

130-
expect(ReactNativeFeatureFlags.jsOnlyTestFlag()).toBe(true);
127+
expect(ReactNativeFeatureFlags.jsOnlyTestFlag()).toBe(false);
128+
});
129+
130+
it('should evaluate to default value if the override returns undefined', () => {
131+
const ReactNativeFeatureFlags = require('../ReactNativeFeatureFlags');
132+
133+
ReactNativeFeatureFlags.override({
134+
jsOnlyTestFlag: () => undefined,
135+
});
136+
137+
expect(ReactNativeFeatureFlags.jsOnlyTestFlag()).toBe(false);
131138
});
132139

133140
describe('when the native module is NOT available', () => {

0 commit comments

Comments
 (0)