From 953407a2e155626910992401a1cbb87a4eb06522 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Thu, 17 Oct 2024 01:57:55 -0700 Subject: [PATCH] Add support for feature flags without native implementation (#47059) Summary: Changelog: [internal] This adds a new configuration for feature flags to preserve their definition only in JavaScript and skip their native API and implementations. This is useful to preserve the API in JavaScript when JavaScript changes progress faster than native changes. Reviewed By: sammy-SC Differential Revision: D64464779 --- .../NativeReactNativeFeatureFlags.cpp | 9 +++- .../NativeReactNativeFeatureFlags.h | 4 +- .../ReactNativeFeatureFlags.config.js | 9 ++++ .../scripts/featureflags/generateFiles.js | 26 +++++++++-- ...iveReactNativeFeatureFlags.cpp-template.js | 18 ++++++-- .../scripts/featureflags/types.js | 46 ++++++++++++------- .../featureflags/ReactNativeFeatureFlags.js | 7 ++- .../specs/NativeReactNativeFeatureFlags.js | 3 +- 8 files changed, 93 insertions(+), 29 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.cpp b/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.cpp index 7f6b37060564a5..700ec221db43a2 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.cpp @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<81047b339ef89dfdfa10b58e22d9407d>> + * @generated SignedSource<<61c80a52ec4c2cf0904bb17c8f479f9d>> */ /** @@ -42,6 +42,13 @@ bool NativeReactNativeFeatureFlags::commonTestFlag( return ReactNativeFeatureFlags::commonTestFlag(); } +bool NativeReactNativeFeatureFlags::commonTestFlagWithoutNativeImplementation( + jsi::Runtime& /*runtime*/) { + // This flag is configured with `skipNativeAPI: true`. + // TODO(T204838867): Implement support for optional methods in C++ TM codegen and remove the method definition altogether. + return false; +} + bool NativeReactNativeFeatureFlags::allowRecursiveCommitsWithSynchronousMountOnAndroid( jsi::Runtime& /*runtime*/) { return ReactNativeFeatureFlags::allowRecursiveCommitsWithSynchronousMountOnAndroid(); diff --git a/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.h b/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.h index f0db3eed6aa334..024cc0c0912a15 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.h +++ b/packages/react-native/ReactCommon/react/nativemodule/featureflags/NativeReactNativeFeatureFlags.h @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<8260155b1822cf4906660d3ad9b298a4>> + * @generated SignedSource<<68423ab013e1357ec176da0fbef4f7e8>> */ /** @@ -37,6 +37,8 @@ class NativeReactNativeFeatureFlags bool commonTestFlag(jsi::Runtime& runtime); + bool commonTestFlagWithoutNativeImplementation(jsi::Runtime& runtime); + bool allowRecursiveCommitsWithSynchronousMountOnAndroid(jsi::Runtime& runtime); bool batchRenderingUpdatesInEventLoop(jsi::Runtime& runtime); diff --git a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js index 80fa246f9c6f4e..765f7f18613b06 100644 --- a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js +++ b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js @@ -30,6 +30,15 @@ const testDefinitions: FeatureFlagDefinitions = { purpose: 'operational', }, }, + commonTestFlagWithoutNativeImplementation: { + defaultValue: false, + metadata: { + description: + 'Common flag for testing (without native implementation). Do NOT modify.', + purpose: 'operational', + }, + skipNativeAPI: true, + }, }, jsOnly: { jsOnlyTestFlag: { diff --git a/packages/react-native/scripts/featureflags/generateFiles.js b/packages/react-native/scripts/featureflags/generateFiles.js index 8f155b1350aeb5..b466b8cd667dfa 100644 --- a/packages/react-native/scripts/featureflags/generateFiles.js +++ b/packages/react-native/scripts/featureflags/generateFiles.js @@ -28,11 +28,31 @@ export default function generateFiles( const jsModules = generateJavaScriptModules(generatorConfig); - const commonCxxModules = generateCommonCxxModules(generatorConfig); + const generatorConfigWithDefinitionsForNative = { + ...generatorConfig, + featureFlagDefinitions: { + ...generatorConfig.featureFlagDefinitions, + common: Object.fromEntries( + Object.entries(generatorConfig.featureFlagDefinitions.common).filter( + ([_, definition]) => !definition.skipNativeAPI, + ), + ), + }, + }; - const androidModules = generateAndroidModules(generatorConfig); + const commonCxxModules = generateCommonCxxModules( + generatorConfigWithDefinitionsForNative, + ); - const generatedFiles = {...jsModules, ...commonCxxModules, ...androidModules}; + const androidModules = generateAndroidModules( + generatorConfigWithDefinitionsForNative, + ); + + const generatedFiles = { + ...jsModules, + ...commonCxxModules, + ...androidModules, + }; if (generatorOptions.verifyUnchanged) { const existingModules: {[string]: string} = {}; diff --git a/packages/react-native/scripts/featureflags/templates/js/NativeReactNativeFeatureFlags.cpp-template.js b/packages/react-native/scripts/featureflags/templates/js/NativeReactNativeFeatureFlags.cpp-template.js index 74be22c0dc43bd..ade547e49d9353 100644 --- a/packages/react-native/scripts/featureflags/templates/js/NativeReactNativeFeatureFlags.cpp-template.js +++ b/packages/react-native/scripts/featureflags/templates/js/NativeReactNativeFeatureFlags.cpp-template.js @@ -46,11 +46,19 @@ NativeReactNativeFeatureFlags::NativeReactNativeFeatureFlags( : NativeReactNativeFeatureFlagsCxxSpec(std::move(jsInvoker)) {} ${Object.entries(definitions.common) - .map( - ([flagName, flagConfig]) => - `${getCxxTypeFromDefaultValue( - flagConfig.defaultValue, - )} NativeReactNativeFeatureFlags::${flagName}( + .map(([flagName, flagConfig]) => + flagConfig.skipNativeAPI + ? `${getCxxTypeFromDefaultValue( + flagConfig.defaultValue, + )} NativeReactNativeFeatureFlags::${flagName}( + jsi::Runtime& /*runtime*/) { + // This flag is configured with \`skipNativeAPI: true\`. + // TODO(T204838867): Implement support for optional methods in C++ TM codegen and remove the method definition altogether. + return ${JSON.stringify(flagConfig.defaultValue)}; +}` + : `${getCxxTypeFromDefaultValue( + flagConfig.defaultValue, + )} NativeReactNativeFeatureFlags::${flagName}( jsi::Runtime& /*runtime*/) { return ReactNativeFeatureFlags::${flagName}(); }`, diff --git a/packages/react-native/scripts/featureflags/types.js b/packages/react-native/scripts/featureflags/types.js index 3c5af96fb1305c..1e328087159714 100644 --- a/packages/react-native/scripts/featureflags/types.js +++ b/packages/react-native/scripts/featureflags/types.js @@ -11,30 +11,42 @@ export type FeatureFlagValue = boolean | number | string; export type FeatureFlagDefinitions = { - common: FeatureFlagList, - jsOnly: FeatureFlagList, + common: CommonFeatureFlagList, + jsOnly: JsOnlyFeatureFlagList, }; -type FeatureFlagList = { +type CommonFeatureFlagList = { [flagName: string]: { defaultValue: FeatureFlagValue, - metadata: - | { - purpose: 'experimentation', - /** - * Aproximate date when the flag was added. - * Used to help prioritize feature flags that need to be cleaned up. - */ - dateAdded: string, - description: string, - } - | { - purpose: 'operational' | 'release', - description: string, - }, + metadata: FeatureFlagMetadata, + // Indicates if this API should only be defined in JavaScript, only to + // preserve backwards compatibility with existing native code temporarily. + skipNativeAPI?: true, }, }; +type JsOnlyFeatureFlagList = { + [flagName: string]: { + defaultValue: FeatureFlagValue, + metadata: FeatureFlagMetadata, + }, +}; + +type FeatureFlagMetadata = + | { + purpose: 'experimentation', + /** + * Aproximate date when the flag was added. + * Used to help prioritize feature flags that need to be cleaned up. + */ + dateAdded: string, + description: string, + } + | { + purpose: 'operational' | 'release', + description: string, + }; + export type GeneratorConfig = { featureFlagDefinitions: FeatureFlagDefinitions, jsPath: string, diff --git a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js index 05ad73e441c09a..22521f5378dd4d 100644 --- a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js +++ b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<5f8a62b797980987ce7f415fa00c0965>> + * @generated SignedSource<> * @flow strict */ @@ -51,6 +51,7 @@ export type ReactNativeFeatureFlagsJsOnlyOverrides = OverridesFor, + commonTestFlagWithoutNativeImplementation: Getter, allowRecursiveCommitsWithSynchronousMountOnAndroid: Getter, batchRenderingUpdatesInEventLoop: Getter, completeReactInstanceCreationOnBgThreadOnAndroid: Getter, @@ -188,6 +189,10 @@ export const useRefsForTextInputState: Getter = createJavaScriptFlagGet * Common flag for testing. Do NOT modify. */ export const commonTestFlag: Getter = createNativeFlagGetter('commonTestFlag', false); +/** + * Common flag for testing (without native implementation). Do NOT modify. + */ +export const commonTestFlagWithoutNativeImplementation: Getter = createNativeFlagGetter('commonTestFlagWithoutNativeImplementation', false); /** * Adds support for recursively processing commits that mount synchronously (Android only). */ diff --git a/packages/react-native/src/private/featureflags/specs/NativeReactNativeFeatureFlags.js b/packages/react-native/src/private/featureflags/specs/NativeReactNativeFeatureFlags.js index 4f8dac8cbc202f..d908c98b408546 100644 --- a/packages/react-native/src/private/featureflags/specs/NativeReactNativeFeatureFlags.js +++ b/packages/react-native/src/private/featureflags/specs/NativeReactNativeFeatureFlags.js @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<2dea79097b1952f97d9e5dda86a38e58>> + * @generated SignedSource<> * @flow strict */ @@ -24,6 +24,7 @@ import * as TurboModuleRegistry from '../../../../Libraries/TurboModule/TurboMod export interface Spec extends TurboModule { +commonTestFlag?: () => boolean; + +commonTestFlagWithoutNativeImplementation?: () => boolean; +allowRecursiveCommitsWithSynchronousMountOnAndroid?: () => boolean; +batchRenderingUpdatesInEventLoop?: () => boolean; +completeReactInstanceCreationOnBgThreadOnAndroid?: () => boolean;