Skip to content

Commit

Permalink
Remove fuseboxEnabledDebug flag (facebook#48435)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#48435

Cleans up the runtime `fuseboxEnabledDebug` feature flag. The modern CDP backend has been enabled by default in open source since 0.76.

- Updates `ReactInstanceIntegrationTest` to preserve testing under both backend modes (legacy Hermes debugger vs Fusebox).
- Preserves ability to override `ReactNativeFeatureFlags` in tests via `InspectorFlagOverridesGuard` — we anticipate that future CDP features will continue to read from the `ReactNativeFeatureFlags` system (`fuseboxEnabled` was/is a special case).

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D67759600

fbshipit-source-id: 5878dd879bae435e59c48823a9b9faf85561b028
  • Loading branch information
huntie authored and facebook-github-bot committed Jan 3, 2025
1 parent 39757da commit 546d21c
Show file tree
Hide file tree
Showing 28 changed files with 75 additions and 185 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<<90dcb658c018c8007953964b24b84278>>
* @generated SignedSource<<cba7d5b72ece69330b97ba0c615502c5>>
*/

/**
Expand Down Expand Up @@ -220,12 +220,6 @@ public object ReactNativeFeatureFlags {
@JvmStatic
public fun fixMountingCoordinatorReportedPendingTransactionsOnAndroid(): Boolean = accessor.fixMountingCoordinatorReportedPendingTransactionsOnAndroid()

/**
* Flag determining if the React Native DevTools (Fusebox) CDP backend should be enabled in debug builds. This flag is global and should not be changed across React Host lifetimes.
*/
@JvmStatic
public fun fuseboxEnabledDebug(): Boolean = accessor.fuseboxEnabledDebug()

/**
* Flag determining if the React Native DevTools (Fusebox) CDP backend should be enabled in release builds. This flag is global and should not be changed across React Host lifetimes.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<ad976a93c28bf6e46dfd7ed1ba0493ed>>
* @generated SignedSource<<ee6d72d1924bffa3a21845a5daa0effd>>
*/

/**
Expand Down Expand Up @@ -52,7 +52,6 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
private var fixDifferentiatorEmittingUpdatesWithWrongParentTagCache: Boolean? = null
private var fixMappingOfEventPrioritiesBetweenFabricAndReactCache: Boolean? = null
private var fixMountingCoordinatorReportedPendingTransactionsOnAndroidCache: Boolean? = null
private var fuseboxEnabledDebugCache: Boolean? = null
private var fuseboxEnabledReleaseCache: Boolean? = null
private var initEagerTurboModulesOnNativeModulesQueueAndroidCache: Boolean? = null
private var lazyAnimationCallbacksCache: Boolean? = null
Expand Down Expand Up @@ -357,15 +356,6 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
return cached
}

override fun fuseboxEnabledDebug(): Boolean {
var cached = fuseboxEnabledDebugCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.fuseboxEnabledDebug()
fuseboxEnabledDebugCache = cached
}
return cached
}

override fun fuseboxEnabledRelease(): Boolean {
var cached = fuseboxEnabledReleaseCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<2e766bcc58b8d6b0fb605ee71520dd2e>>
* @generated SignedSource<<aa6543cc1bf955b467fc977d2bf475eb>>
*/

/**
Expand Down Expand Up @@ -92,8 +92,6 @@ public object ReactNativeFeatureFlagsCxxInterop {

@DoNotStrip @JvmStatic public external fun fixMountingCoordinatorReportedPendingTransactionsOnAndroid(): Boolean

@DoNotStrip @JvmStatic public external fun fuseboxEnabledDebug(): Boolean

@DoNotStrip @JvmStatic public external fun fuseboxEnabledRelease(): Boolean

@DoNotStrip @JvmStatic public external fun initEagerTurboModulesOnNativeModulesQueueAndroid(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<ad4ecf46a017b05a22046b772504f889>>
* @generated SignedSource<<7075974bc541d91e275ee370f2a3709b>>
*/

/**
Expand Down Expand Up @@ -87,8 +87,6 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi

override fun fixMountingCoordinatorReportedPendingTransactionsOnAndroid(): Boolean = false

override fun fuseboxEnabledDebug(): Boolean = true

override fun fuseboxEnabledRelease(): Boolean = false

override fun initEagerTurboModulesOnNativeModulesQueueAndroid(): Boolean = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<f56022e0738217726e5e7b938666b56e>>
* @generated SignedSource<<0b570bc8931ec25f16ee777f0bfcd04f>>
*/

/**
Expand Down Expand Up @@ -56,7 +56,6 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
private var fixDifferentiatorEmittingUpdatesWithWrongParentTagCache: Boolean? = null
private var fixMappingOfEventPrioritiesBetweenFabricAndReactCache: Boolean? = null
private var fixMountingCoordinatorReportedPendingTransactionsOnAndroidCache: Boolean? = null
private var fuseboxEnabledDebugCache: Boolean? = null
private var fuseboxEnabledReleaseCache: Boolean? = null
private var initEagerTurboModulesOnNativeModulesQueueAndroidCache: Boolean? = null
private var lazyAnimationCallbacksCache: Boolean? = null
Expand Down Expand Up @@ -393,16 +392,6 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
return cached
}

override fun fuseboxEnabledDebug(): Boolean {
var cached = fuseboxEnabledDebugCache
if (cached == null) {
cached = currentProvider.fuseboxEnabledDebug()
accessedFeatureFlags.add("fuseboxEnabledDebug")
fuseboxEnabledDebugCache = cached
}
return cached
}

override fun fuseboxEnabledRelease(): Boolean {
var cached = fuseboxEnabledReleaseCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<94d6ce778ccf52a7f7b2ab574b1c9547>>
* @generated SignedSource<<080245bea6e8535f2e8a91c1559e2fcc>>
*/

/**
Expand Down Expand Up @@ -87,8 +87,6 @@ public interface ReactNativeFeatureFlagsProvider {

@DoNotStrip public fun fixMountingCoordinatorReportedPendingTransactionsOnAndroid(): Boolean

@DoNotStrip public fun fuseboxEnabledDebug(): Boolean

@DoNotStrip public fun fuseboxEnabledRelease(): Boolean

@DoNotStrip public fun initEagerTurboModulesOnNativeModulesQueueAndroid(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<260f2446e2c2d5ad3e4089798bb86ae0>>
* @generated SignedSource<<5b4977559c424312ed8bd791a96da52d>>
*/

/**
Expand Down Expand Up @@ -231,12 +231,6 @@ class ReactNativeFeatureFlagsProviderHolder
return method(javaProvider_);
}

bool fuseboxEnabledDebug() override {
static const auto method =
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("fuseboxEnabledDebug");
return method(javaProvider_);
}

bool fuseboxEnabledRelease() override {
static const auto method =
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("fuseboxEnabledRelease");
Expand Down Expand Up @@ -491,11 +485,6 @@ bool JReactNativeFeatureFlagsCxxInterop::fixMountingCoordinatorReportedPendingTr
return ReactNativeFeatureFlags::fixMountingCoordinatorReportedPendingTransactionsOnAndroid();
}

bool JReactNativeFeatureFlagsCxxInterop::fuseboxEnabledDebug(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
return ReactNativeFeatureFlags::fuseboxEnabledDebug();
}

bool JReactNativeFeatureFlagsCxxInterop::fuseboxEnabledRelease(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
return ReactNativeFeatureFlags::fuseboxEnabledRelease();
Expand Down Expand Up @@ -698,9 +687,6 @@ void JReactNativeFeatureFlagsCxxInterop::registerNatives() {
makeNativeMethod(
"fixMountingCoordinatorReportedPendingTransactionsOnAndroid",
JReactNativeFeatureFlagsCxxInterop::fixMountingCoordinatorReportedPendingTransactionsOnAndroid),
makeNativeMethod(
"fuseboxEnabledDebug",
JReactNativeFeatureFlagsCxxInterop::fuseboxEnabledDebug),
makeNativeMethod(
"fuseboxEnabledRelease",
JReactNativeFeatureFlagsCxxInterop::fuseboxEnabledRelease),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<<bc53fc826ce10cd270fe71ebaefc9ad8>>
* @generated SignedSource<<eead6b4f156908455773230140a7d733>>
*/

/**
Expand Down Expand Up @@ -126,9 +126,6 @@ class JReactNativeFeatureFlagsCxxInterop
static bool fixMountingCoordinatorReportedPendingTransactionsOnAndroid(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

static bool fuseboxEnabledDebug(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

static bool fuseboxEnabledRelease(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ InspectorFlags& InspectorFlags::getInstance() {
}

bool InspectorFlags::getFuseboxEnabled() const {
if (fuseboxDisabledForTest_) {
return false;
}

return loadFlagsAndAssertUnchanged().fuseboxEnabled;
}

Expand All @@ -29,6 +33,10 @@ void InspectorFlags::dangerouslyResetFlags() {
*this = InspectorFlags{};
}

void InspectorFlags::dangerouslyDisableFuseboxForTest() {
fuseboxDisabledForTest_ = true;
}

#if defined(REACT_NATIVE_FORCE_ENABLE_FUSEBOX) && \
defined(REACT_NATIVE_FORCE_DISABLE_FUSEBOX)
#error \
Expand All @@ -46,8 +54,6 @@ const InspectorFlags::Values& InspectorFlags::loadFlagsAndAssertUnchanged()
#elif defined(HERMES_ENABLE_DEBUGGER) && \
defined(REACT_NATIVE_ENABLE_FUSEBOX_DEBUG)
true,
#elif defined(HERMES_ENABLE_DEBUGGER)
ReactNativeFeatureFlags::fuseboxEnabledDebug(),
#elif defined(REACT_NATIVE_ENABLE_FUSEBOX_RELEASE)
true,
#else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ class InspectorFlags {
*/
bool getIsProfilingBuild() const;

/**
* Forcibly disable the main `getFuseboxEnabled()` flag. This should ONLY be
* used by `ReactInstanceIntegrationTest`.
*/
void dangerouslyDisableFuseboxForTest();

/**
* Reset flags to their upstream values. The caller must ensure any resources
* that have read previous flag values have been cleaned up.
Expand All @@ -50,6 +56,7 @@ class InspectorFlags {

mutable std::optional<Values> cachedValues_;
mutable bool inconsistentFlagsStateLogged_{false};
bool fuseboxDisabledForTest_{false};

const Values& loadFlagsAndAssertUnchanged() const;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
#include <folly/json.h>
#include <glog/logging.h>
#include <jsinspector-modern/InspectorFlags.h>
#include <react/featureflags/ReactNativeFeatureFlags.h>
#include <react/featureflags/ReactNativeFeatureFlagsDefaults.h>
#include <react/runtime/hermes/HermesInstance.h>

using namespace ::testing;
Expand All @@ -27,9 +25,14 @@ ReactInstanceIntegrationTest::ReactInstanceIntegrationTest()
: runtime(nullptr),
instance(nullptr),
messageQueueThread(std::make_shared<MockMessageQueueThread>()),
errorHandler(std::make_shared<ErrorUtils>()) {}
errorHandler(std::make_shared<ErrorUtils>()),
testMode_(GetParam()) {}

void ReactInstanceIntegrationTest::SetUp() {
if (testMode_ == ReactInstanceIntegrationTestMode::LEGACY_HERMES) {
InspectorFlags::getInstance().dangerouslyDisableFuseboxForTest();
}

auto mockRegistry = std::make_unique<MockTimerRegistry>();
auto timerManager =
std::make_shared<react::TimerManager>(std::move(mockRegistry));
Expand Down Expand Up @@ -128,7 +131,6 @@ void ReactInstanceIntegrationTest::TearDown() {

// Expect the remote connection to have been destroyed.
EXPECT_EQ(mockRemoteConnections_[0], nullptr);
ReactNativeFeatureFlags::dangerouslyReset();
}

void ReactInstanceIntegrationTest::initializeRuntime(std::string_view script) {
Expand Down Expand Up @@ -191,12 +193,12 @@ bool ReactInstanceIntegrationTest::verbose(bool isVerbose) {

#pragma endregion

TEST_F(ReactInstanceIntegrationTest, RuntimeEvalTest) {
TEST_P(ReactInstanceIntegrationTest, RuntimeEvalTest) {
auto val = run("1 + 2");
EXPECT_EQ(val.asNumber(), 3);
}

TEST_P(ReactInstanceIntegrationTestWithFlags, ConsoleLog) {
TEST_P(ReactInstanceIntegrationTest, ConsoleLog) {
EXPECT_CALL(
getRemoteConnection(),
onMessage(JsonParsed(
Expand All @@ -220,11 +222,10 @@ TEST_P(ReactInstanceIntegrationTestWithFlags, ConsoleLog) {
}

INSTANTIATE_TEST_SUITE_P(
ReactInstanceVaryingInspectorFlags,
ReactInstanceIntegrationTestWithFlags,
ReactInstanceVaryingInspectorBackend,
ReactInstanceIntegrationTest,
::testing::Values(
InspectorFlagOverrides{.fuseboxEnabledDebug = false},
InspectorFlagOverrides{.fuseboxEnabledDebug = false},
InspectorFlagOverrides{.fuseboxEnabledDebug = true}));
ReactInstanceIntegrationTestMode::LEGACY_HERMES,
ReactInstanceIntegrationTestMode::FUSEBOX));

} // namespace facebook::react::jsinspector_modern
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "InspectorMocks.h"
#include "ReactNativeMocks.h"
#include "UniquePtrFactory.h"
#include "utils/InspectorFlagOverridesGuard.h"

#include <folly/executors/QueuedImmediateExecutor.h>
#include <folly/json.h>
Expand All @@ -24,11 +23,14 @@ namespace facebook::react::jsinspector_modern {

using namespace ::testing;

struct FeatureFlags {
const bool fuseboxEnabledDebug = true;
enum ReactInstanceIntegrationTestMode {
LEGACY_HERMES,
FUSEBOX,
};

class ReactInstanceIntegrationTest : public Test {
class ReactInstanceIntegrationTest
: public Test,
public ::testing::WithParamInterface<ReactInstanceIntegrationTestMode> {
protected:
ReactInstanceIntegrationTest();

Expand Down Expand Up @@ -58,6 +60,7 @@ class ReactInstanceIntegrationTest : public Test {
private:
void initializeRuntime(std::string_view script);

ReactInstanceIntegrationTestMode testMode_;
size_t id_ = 1;
bool verbose_ = false;
std::optional<int> pageId_;
Expand All @@ -66,13 +69,4 @@ class ReactInstanceIntegrationTest : public Test {
folly::QueuedImmediateExecutor immediateExecutor_;
};

class ReactInstanceIntegrationTestWithFlags
: public ReactInstanceIntegrationTest,
public ::testing::WithParamInterface<InspectorFlagOverrides> {
protected:
ReactInstanceIntegrationTestWithFlags() : inspectorFlagsGuard_(GetParam()) {}

private:
InspectorFlagOverridesGuard inspectorFlagsGuard_;
};
} // namespace facebook::react::jsinspector_modern
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ JsiIntegrationTestGenericEngineAdapter::JsiIntegrationTestGenericEngineAdapter(

/* static */ InspectorFlagOverrides
JsiIntegrationTestGenericEngineAdapter::getInspectorFlagOverrides() noexcept {
return {.fuseboxEnabledDebug = true};
return {};
}

RuntimeTargetDelegate&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ JsiIntegrationTestHermesEngineAdapter::JsiIntegrationTestHermesEngineAdapter(

/* static */ InspectorFlagOverrides
JsiIntegrationTestHermesEngineAdapter::getInspectorFlagOverrides() noexcept {
return {
.fuseboxEnabledDebug = true,
};
return {};
}

RuntimeTargetDelegate&
Expand Down
Loading

0 comments on commit 546d21c

Please sign in to comment.