Add runtime check to ensure feature flags are only overridden once (f…

…acebook#42825)

Summary:
Pull Request resolved: facebook#42825

This makes overrides for feature flags more predictable by only allowing a single point of overrides per app.

The previous behavior was:
* Common flags: the last override would win.
* JS flags: overrides would be combined and the last definition for each flag would win.

The new behavior, both for common flags and for JS flags, is to only have a single override.

Changelog: [internal]

Reviewed By: mdvacca

Differential Revision: D53360609

fbshipit-source-id: 03d58eb43b5b9988b68337ea5828ba284d86cbd1

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<<0f2ef778c97b1928fde0245008015227>>
+ * @generated SignedSource<<b6542bcfd6e42bb1142a3682a15edb80>>
  */
 
 /**
@@ -26,7 +26,8 @@
 namespace facebook::react {
 
 ReactNativeFeatureFlagsAccessor::ReactNativeFeatureFlagsAccessor()
-    : currentProvider_(std::make_unique<ReactNativeFeatureFlagsDefaults>()) {}
+    : currentProvider_(std::make_unique<ReactNativeFeatureFlagsDefaults>()),
+      wasOverridden_(false) {}
 
 bool ReactNativeFeatureFlagsAccessor::commonTestFlag() {
   auto flagValue = commonTestFlag_.load();
@@ -156,7 +157,13 @@ bool ReactNativeFeatureFlagsAccessor::enableFixForClippedSubviewsCrash() {
 
 void ReactNativeFeatureFlagsAccessor::override(
     std::unique_ptr<ReactNativeFeatureFlagsProvider> provider) {
+  if (wasOverridden_) {
+    throw std::runtime_error(
+        "Feature flags cannot be overridden more than once");
+  }
+
   ensureFlagsNotAccessed();
+  wasOverridden_ = true;
   currentProvider_ = std::move(provider);
 }
 

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<<4de1ed8503548f440c1a0e4f903c80c7>>
+ * @generated SignedSource<<3ccda927120cc579c0ccbc42c32b17e0>>
  */
 
 /**
@@ -46,6 +46,8 @@ class ReactNativeFeatureFlagsAccessor {
   void ensureFlagsNotAccessed();
 
   std::unique_ptr<ReactNativeFeatureFlagsProvider> currentProvider_;
+  bool wasOverridden_;
+
   std::array<std::atomic<const char*>, 7> accessedFeatureFlags_;
 
   std::atomic<std::optional<bool>> commonTestFlag_;

packages/react-native/ReactCommon/react/featureflags/tests/ReactNativeFeatureFlagsTest.cpp

@@ -63,6 +63,25 @@ TEST_F(ReactNativeFeatureFlagsTest, preventsOverridingAfterAccess) {
   EXPECT_EQ(ReactNativeFeatureFlags::commonTestFlag(), false);
 }
 
+TEST_F(ReactNativeFeatureFlagsTest, preventsOverridingAfterOverride) {
+  ReactNativeFeatureFlags::override(
+      std::make_unique<ReactNativeFeatureFlagsTestOverrides>());
+
+  EXPECT_EQ(ReactNativeFeatureFlags::commonTestFlag(), true);
+
+  try {
+    ReactNativeFeatureFlags::override(
+        std::make_unique<ReactNativeFeatureFlagsTestOverrides>());
+    FAIL()
+        << "Expected ReactNativeFeatureFlags::override() to throw an exception";
+  } catch (const std::runtime_error& e) {
+    EXPECT_STREQ("Feature flags cannot be overridden more than once", e.what());
+  }
+
+  // Original overrides should still work
+  EXPECT_EQ(ReactNativeFeatureFlags::commonTestFlag(), true);
+}
+
 TEST_F(ReactNativeFeatureFlagsTest, cachesValuesFromOverride) {
   ReactNativeFeatureFlags::override(
       std::make_unique<ReactNativeFeatureFlagsTestOverrides>());

packages/react-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlagsAccessor.cpp-template.js

@@ -34,7 +34,8 @@ ${DO_NOT_MODIFY_COMMENT}
 namespace facebook::react {
 
 ReactNativeFeatureFlagsAccessor::ReactNativeFeatureFlagsAccessor()
-    : currentProvider_(std::make_unique<ReactNativeFeatureFlagsDefaults>()) {}
+    : currentProvider_(std::make_unique<ReactNativeFeatureFlagsDefaults>()),
+      wasOverridden_(false) {}
 
 ${Object.entries(definitions.common)
   .map(
@@ -63,7 +64,13 @@ ${Object.entries(definitions.common)
 
 void ReactNativeFeatureFlagsAccessor::override(
     std::unique_ptr<ReactNativeFeatureFlagsProvider> provider) {
+  if (wasOverridden_) {
+    throw std::runtime_error(
+        "Feature flags cannot be overridden more than once");
+  }
+
   ensureFlagsNotAccessed();
+  wasOverridden_ = true;
   currentProvider_ = std::move(provider);
 }
 

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

@@ -53,6 +53,8 @@ ${Object.entries(definitions.common)
   void ensureFlagsNotAccessed();
 
   std::unique_ptr<ReactNativeFeatureFlagsProvider> currentProvider_;
+  bool wasOverridden_;
+
   std::array<std::atomic<const char*>, ${
     Object.keys(definitions.common).length
   }> accessedFeatureFlags_;

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

@@ -16,7 +16,7 @@ import type {
 import NativeReactNativeFeatureFlags from './NativeReactNativeFeatureFlags';
 
 const accessedFeatureFlags: Set<string> = new Set();
-const overrides: ReactNativeFeatureFlagsJsOnlyOverrides = {};
+let overrides: ?ReactNativeFeatureFlagsJsOnlyOverrides;
 
 export type Getter<T> = () => T;
 
@@ -44,7 +44,7 @@ export function createJavaScriptFlagGetter<
 ): Getter<ReturnType<ReactNativeFeatureFlagsJsOnly[K]>> {
   return createGetter(
     configName,
-    () => overrides[configName]?.(),
+    () => overrides?.[configName]?.(),
     defaultValue,
   );
 }
@@ -69,12 +69,16 @@ export function getOverrides(): ?ReactNativeFeatureFlagsJsOnlyOverrides {
 export function setOverrides(
   newOverrides: ReactNativeFeatureFlagsJsOnlyOverrides,
 ): void {
+  if (overrides != null) {
+    throw new Error('Feature flags cannot be overridden more than once');
+  }
+
   if (accessedFeatureFlags.size > 0) {
     const accessedFeatureFlagsStr = Array.from(accessedFeatureFlags).join(', ');
     throw new Error(
       `Feature flags were accessed before being overridden: ${accessedFeatureFlagsStr}`,
     );
   }
 
-  Object.assign(overrides, newOverrides);
+  overrides = newOverrides;
 }

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

@@ -75,4 +75,18 @@ describe('ReactNativeFeatureFlags', () => {
       'Feature flags were accessed before being overridden: commonTestFlag',
     );
   });
+
+  it('should throw an error when trying to set overrides twice', () => {
+    const ReactNativeFeatureFlags = require('../ReactNativeFeatureFlags');
+
+    ReactNativeFeatureFlags.override({
+      jsOnlyTestFlag: () => true,
+    });
+
+    expect(() =>
+      ReactNativeFeatureFlags.override({
+        jsOnlyTestFlag: () => false,
+      }),
+    ).toThrow('Feature flags cannot be overridden more than once');
+  });
 });