Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix NITRO_DEBUG debug/release preprocessor directive #132

Merged
merged 4 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 35 additions & 12 deletions example/src/screens/HybridObjectTestsScreen.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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)

Expand Down Expand Up @@ -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) => (
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export function createHybridObjectIntializer(): SourceFile[] {
${createFileMetadataString(`${autolinkingClassName}.hpp`)}

#include <jni.h>
#include <NitroModules/NitroDefines.hpp>

namespace ${cxxNamespace} {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!");
}
Expand All @@ -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!");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
///

#include <jni.h>
#include <NitroModules/NitroDefines.hpp>

namespace margelo::nitro::image {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//

#include "NitroLogger.hpp"
#include "NitroDefines.hpp"
#include <android/log.h>

namespace margelo::nitro {
Expand All @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "JHybridObjectRegistry.hpp"
#include "HybridObjectRegistry.hpp"
#include "JNISharedPtr.hpp"
#include "NitroDefines.hpp"

namespace margelo::nitro {

Expand All @@ -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!");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ struct JSIConverter;

#include "CountTrailingOptionals.hpp"
#include "JSIConverter.hpp"
#include "NitroDefines.hpp"
#include "TypeInfo.hpp"
#include <functional>
#include <jsi/jsi.h>
Expand Down Expand Up @@ -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"
Expand All @@ -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`, "
Expand All @@ -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!");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "HybridObject.hpp"
#include "JSIConverter.hpp"
#include "NitroDefines.hpp"

namespace margelo::nitro {

Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion packages/react-native-nitro-modules/cpp/jsi/JSICache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "JSICache.hpp"
#include "JSIHelpers.hpp"
#include "NitroDefines.hpp"

namespace margelo::nitro {

Expand Down Expand Up @@ -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 + "\")");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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! "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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!"));
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion packages/react-native-nitro-modules/cpp/jsi/JSIHelpers.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//
// JSIHelpers.hpp
// Pods
// Nitro
//
// Created by Marc Rousavy on 07.08.24.
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#pragma once

#include "NitroDefines.hpp"
#include <cstdarg>
#include <cstdio>
#include <iostream>
Expand All @@ -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?");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//

#include "HybridObjectPrototype.hpp"
#include "NitroDefines.hpp"
#include "NitroLogger.hpp"

namespace margelo::nitro {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//

#include "HybridObjectRegistry.hpp"
#include "NitroDefines.hpp"
#include "NitroLogger.hpp"

namespace margelo::nitro {
Expand All @@ -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) +
Expand Down Expand Up @@ -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 +
"\" - "
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "Dispatcher.hpp"
#include "JSIHelpers.hpp"
#include "NitroDefines.hpp"
#include "NitroLogger.hpp"

namespace margelo::nitro {
Expand Down Expand Up @@ -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) +
Expand Down
Loading
Loading