fix: Fix NITRO_DEBUG debug/release preprocessor directive (#132)

* fix: Use `NITRO_DEBUG` flag to predictably run debug/release code * feat: Add `NitroModules.buildType` * fix: Style `debug` label
mrousavy · Sep 16, 2024 · 03e1a71 · 03e1a71
1 parent d291f63
commit 03e1a71
Show file tree

Hide file tree

Showing 25 changed files with 110 additions and 41 deletions.
diff --git a/example/src/screens/HybridObjectTestsScreen.tsx b/example/src/screens/HybridObjectTestsScreen.tsx
@@ -1,6 +1,13 @@
 import * as React from 'react'
 
-import { StyleSheet, View, Text, ScrollView, Button } from 'react-native'
+import {
+  StyleSheet,
+  View,
+  Text,
+  ScrollView,
+  Button,
+  Platform,
+} from 'react-native'
 import {
   HybridTestObjectCpp,
   HybridTestObjectSwiftKotlin,
@@ -9,6 +16,7 @@ import { getTests, type TestRunner } from '../getTests'
 import { SafeAreaView } from 'react-native-safe-area-context'
 import { logPrototypeChain } from '../logPrototypeChain'
 import SegmentedControl from '@react-native-segmented-control/segmented-control'
+import { NitroModules } from 'react-native-nitro-modules'
 
 logPrototypeChain(HybridTestObjectCpp)
 
@@ -128,14 +136,18 @@ export function HybridObjectTestsScreen() {
   return (
     <SafeAreaView style={styles.container}>
       <Text style={styles.header}>HybridObject Tests</Text>
-      <SegmentedControl
-        style={styles.segmentedControl}
-        values={['C++', 'Swift/Kotlin']}
-        selectedIndex={selectedIndex}
-        onChange={({ nativeEvent: { selectedSegmentIndex } }) => {
-          setSelectedIndex(selectedSegmentIndex)
-        }}
-      />
+      <View style={styles.topControls}>
+        <SegmentedControl
+          style={styles.segmentedControl}
+          values={['C++', 'Swift/Kotlin']}
+          selectedIndex={selectedIndex}
+          onChange={({ nativeEvent: { selectedSegmentIndex } }) => {
+            setSelectedIndex(selectedSegmentIndex)
+          }}
+        />
+        <View style={styles.flex} />
+        <Text style={styles.buildTypeText}>{NitroModules.buildType}</Text>
+      </View>
 
       <ScrollView contentContainerStyle={styles.scrollContent}>
         {tests.map((t, i) => (
@@ -170,11 +182,22 @@ const styles = StyleSheet.create({
   scrollContent: {
     paddingHorizontal: 15,
   },
+  topControls: {
+    marginHorizontal: 15,
+    marginBottom: 10,
+    flexDirection: 'row',
+    alignItems: 'center',
+  },
+  buildTypeText: {
+    fontFamily: Platform.select({
+      ios: 'Menlo',
+      macos: 'Menlo',
+      android: 'monospace',
+    }),
+    fontWeight: 'bold',
+  },
   segmentedControl: {
-    alignSelf: 'flex-start',
     minWidth: 180,
-    marginLeft: 15,
-    marginBottom: 10,
   },
   box: {
     width: 60,

diff --git a/packages/nitrogen/src/autolinking/android/createHybridObjectInitializer.ts b/packages/nitrogen/src/autolinking/android/createHybridObjectInitializer.ts
@@ -56,6 +56,7 @@ export function createHybridObjectIntializer(): SourceFile[] {
 ${createFileMetadataString(`${autolinkingClassName}.hpp`)}
 
 #include <jni.h>
+#include <NitroModules/NitroDefines.hpp>
 
 namespace ${cxxNamespace} {
 

diff --git a/packages/nitrogen/src/syntax/kotlin/KotlinHybridObjectRegistration.ts b/packages/nitrogen/src/syntax/kotlin/KotlinHybridObjectRegistration.ts
@@ -42,7 +42,7 @@ HybridObjectRegistry::registerHybridObjectConstructor(
     static auto defaultConstructor = javaClass->getConstructor<${JHybridTSpec}::javaobject()>();
 
     auto instance = javaClass->newObject(defaultConstructor);
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
     if (instance == nullptr) [[unlikely]] {
       throw std::runtime_error("Failed to create an instance of \\"${JHybridTSpec}\\" - the constructor returned null!");
     }

diff --git a/packages/react-native-nitro-image/nitrogen/generated/android/NitroImageOnLoad.cpp b/packages/react-native-nitro-image/nitrogen/generated/android/NitroImageOnLoad.cpp
@@ -45,7 +45,7 @@ int initialize(JavaVM* vm) {
         static auto defaultConstructor = javaClass->getConstructor<JHybridImageFactorySpec::javaobject()>();
 
         auto instance = javaClass->newObject(defaultConstructor);
-    #ifndef NDEBUG
+    #ifdef NITRO_DEBUG
         if (instance == nullptr) [[unlikely]] {
           throw std::runtime_error("Failed to create an instance of \"JHybridImageFactorySpec\" - the constructor returned null!");
         }
@@ -70,7 +70,7 @@ int initialize(JavaVM* vm) {
         static auto defaultConstructor = javaClass->getConstructor<JHybridTestObjectSwiftKotlinSpec::javaobject()>();
 
         auto instance = javaClass->newObject(defaultConstructor);
-    #ifndef NDEBUG
+    #ifdef NITRO_DEBUG
         if (instance == nullptr) [[unlikely]] {
           throw std::runtime_error("Failed to create an instance of \"JHybridTestObjectSwiftKotlinSpec\" - the constructor returned null!");
         }

diff --git a/packages/react-native-nitro-image/nitrogen/generated/android/NitroImageOnLoad.hpp b/packages/react-native-nitro-image/nitrogen/generated/android/NitroImageOnLoad.hpp
@@ -6,6 +6,7 @@
 ///
 
 #include <jni.h>
+#include <NitroModules/NitroDefines.hpp>
 
 namespace margelo::nitro::image {
 

diff --git a/packages/react-native-nitro-modules/android/src/main/cpp/platform/NitroLogger.cpp b/packages/react-native-nitro-modules/android/src/main/cpp/platform/NitroLogger.cpp
@@ -6,6 +6,7 @@
 //
 
 #include "NitroLogger.hpp"
+#include "NitroDefines.hpp"
 #include <android/log.h>
 
 namespace margelo::nitro {
@@ -26,7 +27,7 @@ int levelToAndroidLevel(LogLevel level) {
 }
 
 void Logger::nativeLog(LogLevel level, const char* tag, const std::string& message) {
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
   int logLevel = levelToAndroidLevel(level);
   std::string combinedTag = "Nitro." + std::string(tag);
   __android_log_print(logLevel, combinedTag.c_str(), "%s", message.c_str());

diff --git a/packages/react-native-nitro-modules/android/src/main/cpp/registry/JHybridObjectRegistry.cpp b/packages/react-native-nitro-modules/android/src/main/cpp/registry/JHybridObjectRegistry.cpp
@@ -8,6 +8,7 @@
 #include "JHybridObjectRegistry.hpp"
 #include "HybridObjectRegistry.hpp"
 #include "JNISharedPtr.hpp"
+#include "NitroDefines.hpp"
 
 namespace margelo::nitro {
 
@@ -17,7 +18,7 @@ void JHybridObjectRegistry::registerHybridObjectConstructor(jni::alias_ref<jni::
   HybridObjectRegistry::registerHybridObjectConstructor(hybridObjectName, [=]() -> std::shared_ptr<HybridObject> {
     // 1. Call the Java initializer function
     jni::local_ref<JHybridObject::javaobject> hybridObject = sharedInitializer->call();
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
     if (hybridObject == nullptr) [[unlikely]] {
       throw std::runtime_error("Failed to create HybridObject \"" + hybridObjectName + "\" - the constructor returned null!");
     }

diff --git a/packages/react-native-nitro-modules/cpp/core/HybridFunction.hpp b/packages/react-native-nitro-modules/cpp/core/HybridFunction.hpp
@@ -14,6 +14,7 @@ struct JSIConverter;
 
 #include "CountTrailingOptionals.hpp"
 #include "JSIConverter.hpp"
+#include "NitroDefines.hpp"
 #include "TypeInfo.hpp"
 #include <functional>
 #include <jsi/jsi.h>
@@ -170,15 +171,15 @@ class HybridFunction final {
   static inline std::shared_ptr<THybrid> getHybridObjectNativeState(jsi::Runtime& runtime, const jsi::Value& value, FunctionKind funcKind,
                                                                     const std::string& funcName) {
     // 1. Convert jsi::Value to jsi::Object
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
     if (!value.isObject()) [[unlikely]] {
       throw jsi::JSError(runtime, "Cannot " + getHybridFuncDebugInfo<THybrid>(funcKind, funcName) + " - `this` is not bound!");
     }
 #endif
     jsi::Object object = value.getObject(runtime);
 
     // 2. Check if it even has any kind of `NativeState`
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
     if (!object.hasNativeState(runtime)) [[unlikely]] {
       throw jsi::JSError(runtime, "Cannot " + getHybridFuncDebugInfo<THybrid>(funcKind, funcName) +
                                       " - `this` does not have a NativeState! Suggestions:\n"
@@ -191,7 +192,7 @@ class HybridFunction final {
 
     // 3. Get `NativeState` from the jsi::Object and check if it is non-null
     std::shared_ptr<jsi::NativeState> nativeState = object.getNativeState(runtime);
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
     if (nativeState == nullptr) [[unlikely]] {
       throw jsi::JSError(runtime, "Cannot " + getHybridFuncDebugInfo<THybrid>(funcKind, funcName) +
                                       " - `this`'s `NativeState` is `nullptr`, "
@@ -201,7 +202,7 @@ class HybridFunction final {
 
     // 4. Try casting it to our desired target type.
     std::shared_ptr<THybrid> hybridInstance = std::dynamic_pointer_cast<THybrid>(nativeState);
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
     if (hybridInstance == nullptr) [[unlikely]] {
       throw jsi::JSError(runtime, "Cannot " + getHybridFuncDebugInfo<THybrid>(funcKind, funcName) +
                                       " - `this` has a NativeState, but it's the wrong type!");

diff --git a/packages/react-native-nitro-modules/cpp/core/HybridObject.cpp b/packages/react-native-nitro-modules/cpp/core/HybridObject.cpp
@@ -4,6 +4,7 @@
 
 #include "HybridObject.hpp"
 #include "JSIConverter.hpp"
+#include "NitroDefines.hpp"
 
 namespace margelo::nitro {
 
@@ -70,7 +71,7 @@ jsi::Value HybridObject::toObject(jsi::Runtime& runtime) {
   // 6. Set memory size so Hermes GC knows about actual memory
   object.setExternalMemoryPressure(runtime, getExternalMemorySize());
 
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
   // 7. Assign a private __type property for debugging - this will be used so users know it's not just an empty object.
   object.setProperty(runtime, "__type", jsi::String::createFromUtf8(runtime, "NativeState<" + std::string(_name) + ">"));
 #endif

diff --git a/packages/react-native-nitro-modules/cpp/jsi/JSICache.cpp b/packages/react-native-nitro-modules/cpp/jsi/JSICache.cpp
@@ -7,6 +7,7 @@
 
 #include "JSICache.hpp"
 #include "JSIHelpers.hpp"
+#include "NitroDefines.hpp"
 
 namespace margelo::nitro {
 
@@ -46,7 +47,7 @@ JSICacheReference JSICache::getOrCreateCache(jsi::Runtime& runtime) {
     Logger::log(LogLevel::Warning, TAG, "JSICache was created, but it is no longer strong!");
   }
 
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
   if (runtime.global().hasProperty(runtime, CACHE_PROP_NAME)) [[unlikely]] {
     throw std::runtime_error("The Runtime \"" + getRuntimeId(runtime) + "\" already has a global cache! (\"" + CACHE_PROP_NAME + "\")");
   }

diff --git a/packages/react-native-nitro-modules/cpp/jsi/JSIConverter+ArrayBuffer.hpp b/packages/react-native-nitro-modules/cpp/jsi/JSIConverter+ArrayBuffer.hpp
@@ -18,6 +18,7 @@ struct JSIConverter;
 #include "ArrayBuffer.hpp"
 #include "IsSharedPtrTo.hpp"
 #include "JSICache.hpp"
+#include "NitroDefines.hpp"
 #include <jsi/jsi.h>
 #include <memory>
 #include <type_traits>
@@ -30,9 +31,16 @@ using namespace facebook;
 template <typename T>
 struct JSIConverter<T, std::enable_if_t<is_shared_ptr_to_v<T, jsi::MutableBuffer>>> final {
   static inline std::shared_ptr<ArrayBuffer> fromJSI(jsi::Runtime& runtime, const jsi::Value& arg) {
-    jsi::Object object = arg.asObject(runtime);
+#ifdef NITRO_DEBUG
+    if (!arg.isObject()) [[unlikely]] {
+      throw std::runtime_error("Value \"" + arg.toString(runtime).utf8(runtime) +
+                               "\" is not an ArrayBuffer - "
+                               "in fact, it's not even an object!");
+    }
+#endif
 
-#ifndef NDEBUG
+    jsi::Object object = arg.asObject(runtime);
+#ifdef NITRO_DEBUG
     if (!object.isArrayBuffer(runtime)) [[unlikely]] {
       throw std::runtime_error("Object \"" + arg.toString(runtime).utf8(runtime) +
                                "\" is not an ArrayBuffer! "

diff --git a/packages/react-native-nitro-modules/cpp/jsi/JSIConverter+HybridObject.hpp b/packages/react-native-nitro-modules/cpp/jsi/JSIConverter+HybridObject.hpp
@@ -10,6 +10,7 @@ class HybridObject;
 } // namespace margelo::nitro
 
 #include "IsSharedPtrTo.hpp"
+#include "NitroDefines.hpp"
 #include "TypeInfo.hpp"
 #include <jsi/jsi.h>
 #include <type_traits>
@@ -24,7 +25,7 @@ struct JSIConverter<T, std::enable_if_t<is_shared_ptr_to_v<T, jsi::NativeState>>
   using TPointee = typename T::element_type;
 
   static inline T fromJSI(jsi::Runtime& runtime, const jsi::Value& arg) {
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
     if (!arg.isObject()) [[unlikely]] {
       if (arg.isUndefined()) [[unlikely]] {
         throw jsi::JSError(runtime, invalidTypeErrorMessage("undefined", "It is undefined!"));
@@ -36,7 +37,7 @@ struct JSIConverter<T, std::enable_if_t<is_shared_ptr_to_v<T, jsi::NativeState>>
 #endif
     jsi::Object object = arg.asObject(runtime);
 
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
     if (!object.hasNativeState<TPointee>(runtime)) [[unlikely]] {
       if (!object.hasNativeState(runtime)) [[unlikely]] {
         std::string stringRepresentation = arg.toString(runtime).utf8(runtime);

diff --git a/packages/react-native-nitro-modules/cpp/jsi/JSIHelpers.hpp b/packages/react-native-nitro-modules/cpp/jsi/JSIHelpers.hpp
@@ -1,6 +1,6 @@
 //
 //  JSIHelpers.hpp
-//  Pods
+//  Nitro
 //
 //  Created by Marc Rousavy on 07.08.24.
 //

diff --git a/packages/react-native-nitro-modules/cpp/platform/NitroLogger.hpp b/packages/react-native-nitro-modules/cpp/platform/NitroLogger.hpp
@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include "NitroDefines.hpp"
 #include <cstdarg>
 #include <cstdio>
 #include <iostream>
@@ -22,7 +23,7 @@ class Logger final {
 public:
   template <typename... Args>
   static void log(LogLevel level, const char* tag, const char* format, Args... args) {
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
     // 1. Make sure args can be passed to sprintf(..)
     static_assert(all_are_trivially_copyable<Args...>(), "All arguments passed to Logger::log(..) must be trivially copyable! "
                                                          "Did you try to pass a complex type, like std::string?");

diff --git a/packages/react-native-nitro-modules/cpp/prototype/HybridObjectPrototype.cpp b/packages/react-native-nitro-modules/cpp/prototype/HybridObjectPrototype.cpp
@@ -6,6 +6,7 @@
 //
 
 #include "HybridObjectPrototype.hpp"
+#include "NitroDefines.hpp"
 #include "NitroLogger.hpp"
 
 namespace margelo::nitro {
@@ -69,7 +70,7 @@ jsi::Value HybridObjectPrototype::createPrototype(jsi::Runtime& runtime, const s
   prototypeCache.emplace(prototype->getNativeInstanceId(), cachedObject);
 
   // 7. In DEBUG, add a __type info to the prototype object.
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
   auto typeName = "Prototype<" + std::string(prototype->getNativeInstanceId().name()) + ">";
   cachedObject->setProperty(runtime, "__type", jsi::String::createFromUtf8(runtime, typeName));
 #endif

diff --git a/packages/react-native-nitro-modules/cpp/registry/HybridObjectRegistry.cpp b/packages/react-native-nitro-modules/cpp/registry/HybridObjectRegistry.cpp
@@ -6,6 +6,7 @@
 //
 
 #include "HybridObjectRegistry.hpp"
+#include "NitroDefines.hpp"
 #include "NitroLogger.hpp"
 
 namespace margelo::nitro {
@@ -31,7 +32,7 @@ std::vector<std::string> HybridObjectRegistry::getAllHybridObjectNames() {
 void HybridObjectRegistry::registerHybridObjectConstructor(const std::string& hybridObjectName, HybridObjectConstructorFn&& constructorFn) {
   Logger::log(LogLevel::Info, TAG, "Registering HybridObject \"%s\"...", hybridObjectName.c_str());
   auto& map = HybridObjectRegistry::getRegistry();
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
   if (map.contains(hybridObjectName)) [[unlikely]] {
     auto message =
         "HybridObject \"" + std::string(hybridObjectName) +
@@ -63,7 +64,7 @@ std::shared_ptr<HybridObject> HybridObjectRegistry::createHybridObject(const std
   }
   std::shared_ptr<HybridObject> instance = fn->second();
 
-#ifndef NDEBUG
+#ifdef NITRO_DEBUG
   if (instance == nullptr) [[unlikely]] {
     throw std::runtime_error("Failed to create HybridObject \"" + hybridObjectName +
                              "\" - "

diff --git a/packages/react-native-nitro-modules/cpp/threading/Dispatcher.cpp b/packages/react-native-nitro-modules/cpp/threading/Dispatcher.cpp
@@ -1,5 +1,6 @@
 #include "Dispatcher.hpp"
 #include "JSIHelpers.hpp"
+#include "NitroDefines.hpp"
 #include "NitroLogger.hpp"
 
 namespace margelo::nitro {
@@ -41,8 +42,8 @@ std::shared_ptr<Dispatcher> Dispatcher::getRuntimeGlobalDispatcher(jsi::Runtime&
 }
 
 jsi::Value Dispatcher::getRuntimeGlobalDispatcherHolder(jsi::Runtime& runtime) {
-#ifndef NDEBUG
-  if (!runtime.global().hasProperty(runtime, GLOBAL_DISPATCHER_HOLDER_NAME)) {
+#ifdef NITRO_DEBUG
+  if (!runtime.global().hasProperty(runtime, GLOBAL_DISPATCHER_HOLDER_NAME)) [[unlikely]] {
     throw std::runtime_error("Failed to get current Dispatcher - the global Dispatcher "
                              "holder (global." +
                              std::string(GLOBAL_DISPATCHER_HOLDER_NAME) +
-Original file line number
+Diff line change
@@ Expand Up / @@ -6,6 +6,7 @@ @@
     ///
     #include <jni.h>
+    #include <NitroModules/NitroDefines.hpp>
     namespace margelo::nitro::image {
@@ Expand Down @@