diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt index e0d4456b4b522d..387eff81aa0cba 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt @@ -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<<405d53cd6aba78616b8690c26a0accad>> + * @generated SignedSource<> */ /** @@ -358,6 +358,32 @@ public object ReactNativeFeatureFlags { accessor = accessorProvider() } + /** + * This is a combination of `dangerouslyReset` and `override` that reduces + * the likeliness of a race condition between the two calls. + * + * This is **dangerous** because it can introduce consistency issues that will + * be much harder to debug. For example, it could hide the fact that feature + * flags are read before you set the values you want to use everywhere. It + * could also cause a workflow to suddently have different feature flags for + * behaviors that were configured with different values before. + * + * It returns a string that contains the feature flags that were accessed + * before this call (or between the last call to `dangerouslyReset` and this + * call). If you are using this method, you do not want the hard crash that + * you would get from using `dangerouslyReset` and `override` separately, + * but you should still log this somehow. + * + * Please see the documentation of `dangerouslyReset` for additional details. + */ + @JvmStatic + public fun dangerouslyForceOverride(provider: ReactNativeFeatureFlagsProvider): String? { + val newAccessor = accessorProvider() + val previouslyAccessedFlags = newAccessor.dangerouslyForceOverride(provider) + accessor = newAccessor + return previouslyAccessedFlags + } + /** * This is just used to replace the default ReactNativeFeatureFlagsCxxAccessor * that uses JNI with a version that doesn't, to simplify testing. diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsAccessor.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsAccessor.kt index b20cafdc4c215e..79c04da9c13b86 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsAccessor.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsAccessor.kt @@ -11,4 +11,6 @@ public interface ReactNativeFeatureFlagsAccessor : ReactNativeFeatureFlagsProvid public fun override(provider: ReactNativeFeatureFlagsProvider) public fun dangerouslyReset() + + public fun dangerouslyForceOverride(provider: ReactNativeFeatureFlagsProvider): String? } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt index dc7173e7c68153..a500c4806477a4 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt @@ -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<<84ee754f916b48a7c55ea94e166510c7>> + * @generated SignedSource<> */ /** @@ -515,4 +515,7 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso ReactNativeFeatureFlagsCxxInterop.override(provider as Any) override fun dangerouslyReset(): Unit = ReactNativeFeatureFlagsCxxInterop.dangerouslyReset() + + override fun dangerouslyForceOverride(provider: ReactNativeFeatureFlagsProvider): String? = + ReactNativeFeatureFlagsCxxInterop.dangerouslyForceOverride(provider as Any) } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt index a1f0cc56e3fb03..2769fbf29a9b9b 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt @@ -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<> + * @generated SignedSource<<3cf8639dfd8a92a954a055c3ebdc749c>> */ /** @@ -129,4 +129,6 @@ public object ReactNativeFeatureFlagsCxxInterop { @DoNotStrip @JvmStatic public external fun override(provider: Any) @DoNotStrip @JvmStatic public external fun dangerouslyReset() + + @DoNotStrip @JvmStatic public external fun dangerouslyForceOverride(provider: Any): String? } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt index ad78217a0b1d7d..6e5cf4e613a6a8 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt @@ -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<<4dc2364f5bcd765d7b61dbcab0d9533d>> + * @generated SignedSource<<91c4268e7e88e2e3c1f3d50d149c0d2b>> */ /** @@ -574,7 +574,21 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces } override fun dangerouslyReset() { - // We don't need to do anything here because `ReactNativeFeatureFlags` will - // just create a new instance of this class. + // We don't need to do anything else here because `ReactNativeFeatureFlags` will just create a + // new instance of this class. + } + + override fun dangerouslyForceOverride(provider: ReactNativeFeatureFlagsProvider): String? { + val accessedFeatureFlags = getAccessedFeatureFlags() + currentProvider = provider + return accessedFeatureFlags + } + + internal fun getAccessedFeatureFlags(): String? { + if (accessedFeatureFlags.isEmpty()) { + return null + } + + return accessedFeatureFlags.joinToString(separator = ", ") { it } } } diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp index 7d4a2d6de2056c..d0d647091210f3 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.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<<0bb021affcef5162b578ad3f1e45508f>> + * @generated SignedSource<> */ /** @@ -594,11 +594,25 @@ void JReactNativeFeatureFlagsCxxInterop::dangerouslyReset( ReactNativeFeatureFlags::dangerouslyReset(); } +jni::local_ref JReactNativeFeatureFlagsCxxInterop::dangerouslyForceOverride( + facebook::jni::alias_ref /*unused*/, + jni::alias_ref provider) { + auto accessedFlags = ReactNativeFeatureFlags::dangerouslyForceOverride( + std::make_unique(provider)); + if (accessedFlags.has_value()) { + return jni::make_jstring(accessedFlags.value()); + } + return nullptr; +} + void JReactNativeFeatureFlagsCxxInterop::registerNatives() { javaClassLocal()->registerNatives({ makeNativeMethod( "override", JReactNativeFeatureFlagsCxxInterop::override), makeNativeMethod("dangerouslyReset", JReactNativeFeatureFlagsCxxInterop::dangerouslyReset), + makeNativeMethod( + "dangerouslyForceOverride", + JReactNativeFeatureFlagsCxxInterop::dangerouslyForceOverride), makeNativeMethod( "commonTestFlag", JReactNativeFeatureFlagsCxxInterop::commonTestFlag), diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.h b/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.h index 74bffbfe9023eb..df77efd59d7c01 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.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<> + * @generated SignedSource<<9b325f01aea83bc93db31c9a63bea3f7>> */ /** @@ -184,6 +184,10 @@ class JReactNativeFeatureFlagsCxxInterop static void dangerouslyReset( facebook::jni::alias_ref); + static jni::local_ref dangerouslyForceOverride( + facebook::jni::alias_ref, + jni::alias_ref provider); + static void registerNatives(); }; diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp index 3fb02c5f0f43e2..582209ed57dfd5 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.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<<013c1fa01cf029635c04b50f83cc80ef>> + * @generated SignedSource<<14957b53f50e3ea943f0e3631475f336>> */ /** @@ -21,6 +21,11 @@ namespace facebook::react { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wglobal-constructors" +std::unique_ptr accessor_; +#pragma GCC diagnostic pop + bool ReactNativeFeatureFlags::commonTestFlag() { return getAccessor().commonTestFlag(); } @@ -223,16 +228,26 @@ void ReactNativeFeatureFlags::override( } void ReactNativeFeatureFlags::dangerouslyReset() { - getAccessor(true); + accessor_ = std::make_unique(); +} + +std::optional ReactNativeFeatureFlags::dangerouslyForceOverride( + std::unique_ptr provider) { + auto accessor = std::make_unique(); + accessor->override(std::move(provider)); + + std::swap(accessor_, accessor); + + // Now accessor is the old accessor + return accessor == nullptr ? std::nullopt + : accessor->getAccessedFeatureFlagNames(); } -ReactNativeFeatureFlagsAccessor& ReactNativeFeatureFlags::getAccessor( - bool reset) { - static std::unique_ptr accessor; - if (accessor == nullptr || reset) { - accessor = std::make_unique(); +ReactNativeFeatureFlagsAccessor& ReactNativeFeatureFlags::getAccessor() { + if (accessor_ == nullptr) { + accessor_ = std::make_unique(); } - return *accessor; + return *accessor_; } } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.h b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.h index dead79d77ae1f5..f93a2ab227085b 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.h +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.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<<3bbe3eb333030be7f32c0965c9db4a5c>> + * @generated SignedSource<> */ /** @@ -22,6 +22,8 @@ #include #include #include +#include +#include #ifndef RN_EXPORT #define RN_EXPORT __attribute__((visibility("default"))) @@ -317,9 +319,24 @@ class ReactNativeFeatureFlags { */ RN_EXPORT static void dangerouslyReset(); + /** + * This is a combination of `dangerouslyReset` and `override` that reduces + * the likeliness of a race condition between the two calls. + * + * This is **dangerous** because it can introduce consistency issues that will + * be much harder to debug. For example, it could hide the fact that feature + * flags are read before you set the values you want to use everywhere. It + * could also cause a workflow to suddently have different feature flags for + * behaviors that were configured with different values before. + * + * Please see the documentation of `dangerouslyReset` for additional details. + */ + RN_EXPORT static std::optional dangerouslyForceOverride( + std::unique_ptr provider); + private: ReactNativeFeatureFlags() = delete; - static ReactNativeFeatureFlagsAccessor& getAccessor(bool reset = false); + static ReactNativeFeatureFlagsAccessor& getAccessor(); }; } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp index 2c0c6d649c4b69..b90049734e5892 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.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<<087de432d750fea45c0c564e626ce1d9>> + * @generated SignedSource<> */ /** @@ -923,13 +923,8 @@ void ReactNativeFeatureFlagsAccessor::override( currentProvider_ = std::move(provider); } -void ReactNativeFeatureFlagsAccessor::markFlagAsAccessed( - int position, - const char* flagName) { - accessedFeatureFlags_[position] = flagName; -} - -void ReactNativeFeatureFlagsAccessor::ensureFlagsNotAccessed() { +std::optional +ReactNativeFeatureFlagsAccessor::getAccessedFeatureFlagNames() const { std::ostringstream featureFlagListBuilder; for (const auto& featureFlagName : accessedFeatureFlags_) { if (featureFlagName != nullptr) { @@ -943,10 +938,24 @@ void ReactNativeFeatureFlagsAccessor::ensureFlagsNotAccessed() { accessedFeatureFlagNames.substr(0, accessedFeatureFlagNames.size() - 2); } - if (!accessedFeatureFlagNames.empty()) { + return accessedFeatureFlagNames.empty() + ? std::nullopt + : std::optional{accessedFeatureFlagNames}; +} + +void ReactNativeFeatureFlagsAccessor::markFlagAsAccessed( + int position, + const char* flagName) { + accessedFeatureFlags_[position] = flagName; +} + +void ReactNativeFeatureFlagsAccessor::ensureFlagsNotAccessed() { + auto accessedFeatureFlagNames = getAccessedFeatureFlagNames(); + + if (accessedFeatureFlagNames.has_value()) { throw std::runtime_error( "Feature flags were accessed before being overridden: " + - accessedFeatureFlagNames); + accessedFeatureFlagNames.value()); } } diff --git a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.h b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.h index 1b1c0b6c72b499..21b006acb711ce 100644 --- a/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.h +++ b/packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.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<<447de2c56cd16e313210f14d0ee3e9e9>> + * @generated SignedSource<<0c9cfdad6a33dd4044a5945befb45522>> */ /** @@ -24,6 +24,7 @@ #include #include #include +#include namespace facebook::react { @@ -82,6 +83,7 @@ class ReactNativeFeatureFlagsAccessor { bool useTurboModules(); void override(std::unique_ptr provider); + std::optional getAccessedFeatureFlagNames() const; private: void markFlagAsAccessed(int position, const char* flagName); diff --git a/packages/react-native/ReactCommon/react/featureflags/tests/ReactNativeFeatureFlagsTest.cpp b/packages/react-native/ReactCommon/react/featureflags/tests/ReactNativeFeatureFlagsTest.cpp index 2047767dbdef39..dad5df2afa7719 100644 --- a/packages/react-native/ReactCommon/react/featureflags/tests/ReactNativeFeatureFlagsTest.cpp +++ b/packages/react-native/ReactCommon/react/featureflags/tests/ReactNativeFeatureFlagsTest.cpp @@ -121,4 +121,27 @@ TEST_F(ReactNativeFeatureFlagsTest, allowsOverridingAgainAfterReset) { EXPECT_EQ(ReactNativeFeatureFlags::commonTestFlag(), true); } +TEST_F( + ReactNativeFeatureFlagsTest, + allowsDangerouslyForcingOverridesWhenValuesHaveNotBeenAccessed) { + auto accessedFlags = ReactNativeFeatureFlags::dangerouslyForceOverride( + std::make_unique()); + + EXPECT_EQ(ReactNativeFeatureFlags::commonTestFlag(), true); + EXPECT_EQ(accessedFlags.has_value(), false); +} + +TEST_F( + ReactNativeFeatureFlagsTest, + allowsDangerouslyForcingOverridesWhenValuesHaveBeenAccessed) { + EXPECT_EQ(ReactNativeFeatureFlags::commonTestFlag(), false); + + auto accessedFlags = ReactNativeFeatureFlags::dangerouslyForceOverride( + std::make_unique()); + + EXPECT_EQ(ReactNativeFeatureFlags::commonTestFlag(), true); + EXPECT_EQ(accessedFlags.has_value(), true); + EXPECT_EQ(accessedFlags.value(), "commonTestFlag"); +} + } // namespace facebook::react diff --git a/packages/react-native/scripts/featureflags/templates/android/JReactNativeFeatureFlagsCxxInterop.cpp-template.js b/packages/react-native/scripts/featureflags/templates/android/JReactNativeFeatureFlagsCxxInterop.cpp-template.js index 223a0e272153e5..8256b7c35b4eca 100644 --- a/packages/react-native/scripts/featureflags/templates/android/JReactNativeFeatureFlagsCxxInterop.cpp-template.js +++ b/packages/react-native/scripts/featureflags/templates/android/JReactNativeFeatureFlagsCxxInterop.cpp-template.js @@ -88,11 +88,25 @@ void JReactNativeFeatureFlagsCxxInterop::dangerouslyReset( ReactNativeFeatureFlags::dangerouslyReset(); } +jni::local_ref JReactNativeFeatureFlagsCxxInterop::dangerouslyForceOverride( + facebook::jni::alias_ref /*unused*/, + jni::alias_ref provider) { + auto accessedFlags = ReactNativeFeatureFlags::dangerouslyForceOverride( + std::make_unique(provider)); + if (accessedFlags.has_value()) { + return jni::make_jstring(accessedFlags.value()); + } + return nullptr; +} + void JReactNativeFeatureFlagsCxxInterop::registerNatives() { javaClassLocal()->registerNatives({ makeNativeMethod( "override", JReactNativeFeatureFlagsCxxInterop::override), makeNativeMethod("dangerouslyReset", JReactNativeFeatureFlagsCxxInterop::dangerouslyReset), + makeNativeMethod( + "dangerouslyForceOverride", + JReactNativeFeatureFlagsCxxInterop::dangerouslyForceOverride), ${Object.entries(definitions.common) .map( ([flagName, flagConfig]) => diff --git a/packages/react-native/scripts/featureflags/templates/android/JReactNativeFeatureFlagsCxxInterop.h-template.js b/packages/react-native/scripts/featureflags/templates/android/JReactNativeFeatureFlagsCxxInterop.h-template.js index 72995b1f714ab1..763d1a805cf76a 100644 --- a/packages/react-native/scripts/featureflags/templates/android/JReactNativeFeatureFlagsCxxInterop.h-template.js +++ b/packages/react-native/scripts/featureflags/templates/android/JReactNativeFeatureFlagsCxxInterop.h-template.js @@ -55,6 +55,10 @@ ${Object.entries(definitions.common) static void dangerouslyReset( facebook::jni::alias_ref); + static jni::local_ref dangerouslyForceOverride( + facebook::jni::alias_ref, + jni::alias_ref provider); + static void registerNatives(); }; diff --git a/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlags.kt-template.js b/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlags.kt-template.js index 9d7ba68a0e196a..68b21fa53b0cea 100644 --- a/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlags.kt-template.js +++ b/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlags.kt-template.js @@ -93,6 +93,32 @@ ${Object.entries(definitions.common) accessor = accessorProvider() } + /** + * This is a combination of \`dangerouslyReset\` and \`override\` that reduces + * the likeliness of a race condition between the two calls. + * + * This is **dangerous** because it can introduce consistency issues that will + * be much harder to debug. For example, it could hide the fact that feature + * flags are read before you set the values you want to use everywhere. It + * could also cause a workflow to suddently have different feature flags for + * behaviors that were configured with different values before. + * + * It returns a string that contains the feature flags that were accessed + * before this call (or between the last call to \`dangerouslyReset\` and this + * call). If you are using this method, you do not want the hard crash that + * you would get from using \`dangerouslyReset\` and \`override\` separately, + * but you should still log this somehow. + * + * Please see the documentation of \`dangerouslyReset\` for additional details. + */ + @JvmStatic + public fun dangerouslyForceOverride(provider: ReactNativeFeatureFlagsProvider): String? { + val newAccessor = accessorProvider() + val previouslyAccessedFlags = newAccessor.dangerouslyForceOverride(provider) + accessor = newAccessor + return previouslyAccessedFlags + } + /** * This is just used to replace the default ReactNativeFeatureFlagsCxxAccessor * that uses JNI with a version that doesn't, to simplify testing. diff --git a/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsCxxAccessor.kt-template.js b/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsCxxAccessor.kt-template.js index 9b2b6894e759ab..71b7ce3ce251a9 100644 --- a/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsCxxAccessor.kt-template.js +++ b/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsCxxAccessor.kt-template.js @@ -57,6 +57,9 @@ ${Object.entries(definitions.common) ReactNativeFeatureFlagsCxxInterop.override(provider as Any) override fun dangerouslyReset(): Unit = ReactNativeFeatureFlagsCxxInterop.dangerouslyReset() + + override fun dangerouslyForceOverride(provider: ReactNativeFeatureFlagsProvider): String? = + ReactNativeFeatureFlagsCxxInterop.dangerouslyForceOverride(provider as Any) } `); } diff --git a/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsCxxInterop.kt-template.js b/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsCxxInterop.kt-template.js index d2e43130ee0756..fa0fc27fdddb50 100644 --- a/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsCxxInterop.kt-template.js +++ b/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsCxxInterop.kt-template.js @@ -51,6 +51,8 @@ ${Object.entries(definitions.common) @DoNotStrip @JvmStatic public external fun override(provider: Any) @DoNotStrip @JvmStatic public external fun dangerouslyReset() + + @DoNotStrip @JvmStatic public external fun dangerouslyForceOverride(provider: Any): String? } `); } diff --git a/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsLocalAccessor.kt-template.js b/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsLocalAccessor.kt-template.js index c1606e41e0b856..b731aee4e84fcf 100644 --- a/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsLocalAccessor.kt-template.js +++ b/packages/react-native/scripts/featureflags/templates/android/ReactNativeFeatureFlagsLocalAccessor.kt-template.js @@ -68,8 +68,22 @@ ${Object.entries(definitions.common) } override fun dangerouslyReset() { - // We don't need to do anything here because \`ReactNativeFeatureFlags\` will - // just create a new instance of this class. + // We don't need to do anything else here because \`ReactNativeFeatureFlags\` will just create a + // new instance of this class. + } + + override fun dangerouslyForceOverride(provider: ReactNativeFeatureFlagsProvider): String? { + val accessedFeatureFlags = getAccessedFeatureFlags() + currentProvider = provider + return accessedFeatureFlags + } + + internal fun getAccessedFeatureFlags(): String? { + if (accessedFeatureFlags.isEmpty()) { + return null + } + + return accessedFeatureFlags.joinToString(separator = ", ") { it } } } `); diff --git a/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlags.cpp-template.js b/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlags.cpp-template.js index bef6fbb8c20e83..32d4e178858241 100644 --- a/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlags.cpp-template.js +++ b/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlags.cpp-template.js @@ -29,6 +29,11 @@ ${DO_NOT_MODIFY_COMMENT} namespace facebook::react { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wglobal-constructors" +std::unique_ptr accessor_; +#pragma GCC diagnostic pop + ${Object.entries(definitions.common) .map( ([flagName, flagConfig]) => @@ -46,16 +51,26 @@ void ReactNativeFeatureFlags::override( } void ReactNativeFeatureFlags::dangerouslyReset() { - getAccessor(true); + accessor_ = std::make_unique(); +} + +std::optional ReactNativeFeatureFlags::dangerouslyForceOverride( + std::unique_ptr provider) { + auto accessor = std::make_unique(); + accessor->override(std::move(provider)); + + std::swap(accessor_, accessor); + + // Now accessor is the old accessor + return accessor == nullptr ? std::nullopt + : accessor->getAccessedFeatureFlagNames(); } -ReactNativeFeatureFlagsAccessor& ReactNativeFeatureFlags::getAccessor( - bool reset) { - static std::unique_ptr accessor; - if (accessor == nullptr || reset) { - accessor = std::make_unique(); +ReactNativeFeatureFlagsAccessor& ReactNativeFeatureFlags::getAccessor() { + if (accessor_ == nullptr) { + accessor_ = std::make_unique(); } - return *accessor; + return *accessor_; } } // namespace facebook::react diff --git a/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlags.h-template.js b/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlags.h-template.js index 3cfeb19e9b62bc..d6a59223341c17 100644 --- a/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlags.h-template.js +++ b/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlags.h-template.js @@ -30,6 +30,8 @@ ${DO_NOT_MODIFY_COMMENT} #include #include #include +#include +#include #ifndef RN_EXPORT #define RN_EXPORT __attribute__((visibility("default"))) @@ -90,9 +92,24 @@ ${Object.entries(definitions.common) */ RN_EXPORT static void dangerouslyReset(); + /** + * This is a combination of \`dangerouslyReset\` and \`override\` that reduces + * the likeliness of a race condition between the two calls. + * + * This is **dangerous** because it can introduce consistency issues that will + * be much harder to debug. For example, it could hide the fact that feature + * flags are read before you set the values you want to use everywhere. It + * could also cause a workflow to suddently have different feature flags for + * behaviors that were configured with different values before. + * + * Please see the documentation of \`dangerouslyReset\` for additional details. + */ + RN_EXPORT static std::optional dangerouslyForceOverride( + std::unique_ptr provider); + private: ReactNativeFeatureFlags() = delete; - static ReactNativeFeatureFlagsAccessor& getAccessor(bool reset = false); + static ReactNativeFeatureFlagsAccessor& getAccessor(); }; } // namespace facebook::react diff --git a/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlagsAccessor.cpp-template.js b/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlagsAccessor.cpp-template.js index 26b9df54910eeb..1d1dc5f249f3fe 100644 --- a/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlagsAccessor.cpp-template.js +++ b/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlagsAccessor.cpp-template.js @@ -74,13 +74,8 @@ void ReactNativeFeatureFlagsAccessor::override( currentProvider_ = std::move(provider); } -void ReactNativeFeatureFlagsAccessor::markFlagAsAccessed( - int position, - const char* flagName) { - accessedFeatureFlags_[position] = flagName; -} - -void ReactNativeFeatureFlagsAccessor::ensureFlagsNotAccessed() { +std::optional +ReactNativeFeatureFlagsAccessor::getAccessedFeatureFlagNames() const { std::ostringstream featureFlagListBuilder; for (const auto& featureFlagName : accessedFeatureFlags_) { if (featureFlagName != nullptr) { @@ -94,10 +89,24 @@ void ReactNativeFeatureFlagsAccessor::ensureFlagsNotAccessed() { accessedFeatureFlagNames.substr(0, accessedFeatureFlagNames.size() - 2); } - if (!accessedFeatureFlagNames.empty()) { + return accessedFeatureFlagNames.empty() + ? std::nullopt + : std::optional{accessedFeatureFlagNames}; +} + +void ReactNativeFeatureFlagsAccessor::markFlagAsAccessed( + int position, + const char* flagName) { + accessedFeatureFlags_[position] = flagName; +} + +void ReactNativeFeatureFlagsAccessor::ensureFlagsNotAccessed() { + auto accessedFeatureFlagNames = getAccessedFeatureFlagNames(); + + if (accessedFeatureFlagNames.has_value()) { throw std::runtime_error( "Feature flags were accessed before being overridden: " + - accessedFeatureFlagNames); + accessedFeatureFlagNames.value()); } } diff --git a/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlagsAccessor.h-template.js b/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlagsAccessor.h-template.js index 07ad24bc0fa92d..86c6cdd7caff45 100644 --- a/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlagsAccessor.h-template.js +++ b/packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlagsAccessor.h-template.js @@ -32,6 +32,7 @@ ${DO_NOT_MODIFY_COMMENT} #include #include #include +#include namespace facebook::react { @@ -47,6 +48,7 @@ ${Object.entries(definitions.common) .join('\n')} void override(std::unique_ptr provider); + std::optional getAccessedFeatureFlagNames() const; private: void markFlagAsAccessed(int position, const char* flagName);