Skip to content

Commit 746376a

Browse files
yungstersfacebook-github-bot
authored andcommitted
RN: Represent Numeric Feature Flags as Double (#51713)
Summary: Pull Request resolved: #51713 Changes the React Native Feature Flags system so that numeric feature flags are represented in native languages as double instead of int, in order to avoid loss of precision for non-integral JavaScript numbers. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D75709111 fbshipit-source-id: d4b2d553ce1e719a5f4b136a4f8d2e46446ef8d8
1 parent 46e70fb commit 746376a

7 files changed

+83
-30
lines changed

packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsDefaults.kt-template.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import type {FeatureFlagDefinitions} from '../../types';
1313
import {
1414
DO_NOT_MODIFY_COMMENT,
1515
getKotlinTypeFromDefaultValue,
16+
getKotlinValueFromDefaultValue,
1617
} from '../../utils';
1718
import signedsource from 'signedsource';
1819

@@ -39,7 +40,7 @@ ${Object.entries(definitions.common)
3940
([flagName, flagConfig]) =>
4041
` override fun ${flagName}(): ${getKotlinTypeFromDefaultValue(
4142
flagConfig.defaultValue,
42-
)} = ${JSON.stringify(flagConfig.defaultValue)}`,
43+
)} = ${getKotlinValueFromDefaultValue(flagConfig.defaultValue)}`,
4344
)
4445
.join('\n\n')}
4546
}

packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsOverrides_RNOSS__Stage__Android.kt-template.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import type {FeatureFlagDefinitions, OSSReleaseStageValue} from '../../types';
1313
import {
1414
DO_NOT_MODIFY_COMMENT,
1515
getKotlinTypeFromDefaultValue,
16+
getKotlinValueFromDefaultValue,
1617
} from '../../utils';
1718
import signedsource from 'signedsource';
1819

@@ -52,7 +53,7 @@ ${Object.entries(definitions.common)
5253
if (flagConfig.ossReleaseStage === ossReleaseStage) {
5354
return ` override fun ${flagName}(): ${getKotlinTypeFromDefaultValue(
5455
flagConfig.metadata.expectedReleaseValue,
55-
)} = ${JSON.stringify(flagConfig.metadata.expectedReleaseValue)}`;
56+
)} = ${getKotlinValueFromDefaultValue(flagConfig.metadata.expectedReleaseValue)}`;
5657
}
5758
})
5859
.filter(Boolean)

packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlagsDefaults.h-template.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@
1010

1111
import type {FeatureFlagDefinitions} from '../../types';
1212

13-
import {DO_NOT_MODIFY_COMMENT, getCxxTypeFromDefaultValue} from '../../utils';
13+
import {
14+
DO_NOT_MODIFY_COMMENT,
15+
getCxxTypeFromDefaultValue,
16+
getCxxValueFromDefaultValue,
17+
} from '../../utils';
1418
import signedsource from 'signedsource';
1519

1620
export default function (definitions: FeatureFlagDefinitions): string {
@@ -41,7 +45,7 @@ ${Object.entries(definitions.common)
4145
` ${getCxxTypeFromDefaultValue(
4246
flagConfig.defaultValue,
4347
)} ${flagName}() override {
44-
return ${JSON.stringify(flagConfig.defaultValue)};
48+
return ${getCxxValueFromDefaultValue(flagConfig.defaultValue)};
4549
}`,
4650
)
4751
.join('\n\n')}

packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlagsDynamicProvider.h-template.js

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,27 +8,15 @@
88
* @format
99
*/
1010

11-
import type {
12-
CommonFeatureFlagConfig,
13-
FeatureFlagDefinitions,
14-
} from '../../types';
11+
import type {FeatureFlagDefinitions} from '../../types';
1512

16-
import {DO_NOT_MODIFY_COMMENT, getCxxTypeFromDefaultValue} from '../../utils';
13+
import {
14+
DO_NOT_MODIFY_COMMENT,
15+
getCxxFollyDynamicAccessorFromDefaultValue,
16+
getCxxTypeFromDefaultValue,
17+
} from '../../utils';
1718
import signedsource from 'signedsource';
1819

19-
function getFollyDynamicAccessor(config: CommonFeatureFlagConfig<>): string {
20-
switch (typeof config.defaultValue) {
21-
case 'boolean':
22-
return 'getBool';
23-
case 'number':
24-
return 'getInt';
25-
case 'string':
26-
return 'getString';
27-
default:
28-
throw new Error(`Unsupported type: ${typeof config.defaultValue}`);
29-
}
30-
}
31-
3220
export default function (definitions: FeatureFlagDefinitions): string {
3321
return signedsource.signFile(`/*
3422
* Copyright (c) Meta Platforms, Inc. and affiliates.
@@ -77,7 +65,7 @@ ${Object.entries(definitions.common)
7765
)} ${flagName}() override {
7866
auto value = values_["${flagName}"];
7967
if (!value.isNull()) {
80-
return value.${getFollyDynamicAccessor(flagConfig)}();
68+
return value.${getCxxFollyDynamicAccessorFromDefaultValue(flagConfig.defaultValue)}();
8169
}
8270
8371
return ReactNativeFeatureFlagsDefaults::${flagName}();

packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlagsOverridesOSS_Stage_.h-template.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@
1010

1111
import type {FeatureFlagDefinitions, OSSReleaseStageValue} from '../../types';
1212

13-
import {DO_NOT_MODIFY_COMMENT, getCxxTypeFromDefaultValue} from '../../utils';
13+
import {
14+
DO_NOT_MODIFY_COMMENT,
15+
getCxxTypeFromDefaultValue,
16+
getCxxValueFromDefaultValue,
17+
} from '../../utils';
1418
import signedsource from 'signedsource';
1519

1620
function getClassName(ossReleaseStage: OSSReleaseStageValue): string {
@@ -64,7 +68,7 @@ ${Object.entries(definitions.common)
6468
return ` ${getCxxTypeFromDefaultValue(
6569
flagConfig.metadata.expectedReleaseValue,
6670
)} ${flagName}() override {
67-
return ${JSON.stringify(flagConfig.metadata.expectedReleaseValue)};
71+
return ${getCxxValueFromDefaultValue(flagConfig.metadata.expectedReleaseValue)};
6872
}`;
6973
}
7074
})

packages/react-native/scripts/featureflags/templates/js/NativeReactNativeFeatureFlags.cpp-template.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@
1010

1111
import type {FeatureFlagDefinitions} from '../../types';
1212

13-
import {DO_NOT_MODIFY_COMMENT, getCxxTypeFromDefaultValue} from '../../utils';
13+
import {
14+
DO_NOT_MODIFY_COMMENT,
15+
getCxxTypeFromDefaultValue,
16+
getCxxValueFromDefaultValue,
17+
} from '../../utils';
1418
import signedsource from 'signedsource';
1519

1620
export default function (definitions: FeatureFlagDefinitions): string {
@@ -54,7 +58,7 @@ ${Object.entries(definitions.common)
5458
jsi::Runtime& /*runtime*/) {
5559
// This flag is configured with \`skipNativeAPI: true\`.
5660
// TODO(T204838867): Implement support for optional methods in C++ TM codegen and remove the method definition altogether.
57-
return ${JSON.stringify(flagConfig.defaultValue)};
61+
return ${getCxxValueFromDefaultValue(flagConfig.defaultValue)};
5862
}`
5963
: `${getCxxTypeFromDefaultValue(
6064
flagConfig.defaultValue,

packages/react-native/scripts/featureflags/utils.js

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,55 @@ export function getCxxTypeFromDefaultValue(
1717
case 'boolean':
1818
return 'bool';
1919
case 'number':
20-
return 'int';
20+
return 'double';
2121
case 'string':
2222
return 'std::string';
2323
default:
2424
throw new Error(`Unsupported default value type: ${typeof defaultValue}`);
2525
}
2626
}
2727

28+
export function getCxxValueFromDefaultValue(
29+
defaultValue: FeatureFlagValue,
30+
): string {
31+
switch (typeof defaultValue) {
32+
case 'boolean':
33+
return defaultValue.toString();
34+
case 'number':
35+
const numericString = defaultValue.toString();
36+
// If the number is an integer, we need to append ".0" so that the result
37+
// is interpeted as a double in C++.
38+
return numericString.includes('.') ? numericString : `${numericString}.0`;
39+
case 'string':
40+
return JSON.stringify(defaultValue);
41+
default:
42+
throw new Error(`Unsupported default value type: ${typeof defaultValue}`);
43+
}
44+
}
45+
46+
export function getCxxFollyDynamicAccessorFromDefaultValue(
47+
defaultValue: FeatureFlagValue,
48+
): string {
49+
switch (typeof defaultValue) {
50+
case 'boolean':
51+
return 'getBool';
52+
case 'number':
53+
return 'getDouble';
54+
case 'string':
55+
return 'getString';
56+
default:
57+
throw new Error(`Unsupported default value type: ${typeof defaultValue}`);
58+
}
59+
}
60+
2861
export function getCxxJNITypeFromDefaultValue(
2962
defaultValue: FeatureFlagValue,
3063
): string {
3164
switch (typeof defaultValue) {
3265
case 'boolean':
3366
return 'jboolean';
3467
case 'number':
35-
return 'jint';
68+
return 'jdouble';
3669
case 'string':
3770
return 'jstring';
3871
default:
@@ -47,14 +80,32 @@ export function getKotlinTypeFromDefaultValue(
4780
case 'boolean':
4881
return 'Boolean';
4982
case 'number':
50-
return 'Int';
83+
return 'Double';
5184
case 'string':
5285
return 'String';
5386
default:
5487
throw new Error(`Unsupported default value type: ${typeof defaultValue}`);
5588
}
5689
}
5790

91+
export function getKotlinValueFromDefaultValue(
92+
defaultValue: FeatureFlagValue,
93+
): string {
94+
switch (typeof defaultValue) {
95+
case 'boolean':
96+
return defaultValue.toString();
97+
case 'number':
98+
const numericString = defaultValue.toString();
99+
// If the number is an integer, we need to append ".0" so that the result
100+
// is interpeted as a double in Kotlin.
101+
return numericString.includes('.') ? numericString : `${numericString}.0`;
102+
case 'string':
103+
return JSON.stringify(defaultValue);
104+
default:
105+
throw new Error(`Unsupported default value type: ${typeof defaultValue}`);
106+
}
107+
}
108+
58109
export const DO_NOT_MODIFY_COMMENT = `/**
59110
* IMPORTANT: Do NOT modify this file directly.
60111
*

0 commit comments

Comments
 (0)