Skip to content

Commit

Permalink
fix: Make string literals throw again, and test optional enums (#135)
Browse files Browse the repository at this point in the history
* fix: Make string literals throw again, and test optional enums

* Update Podfile.lock

* fix tests

* hm compiler doesnt work

* fix: Fix nitrogen for optional enums

* fix: Optional enums or ArrayBuffers can be bridged directly

* fix: Disable codegen for Nitro Modules

* fix: Rename `Spec` to avoid running codegen

* fix: Disable autolinking by renaming the file so codegen doesnt find it

* fix: Format

* Update HybridObjectRegistry.cpp

* Update HybridObjectRegistry.cpp
  • Loading branch information
mrousavy authored Sep 16, 2024
1 parent f43e06f commit 46c9ca0
Show file tree
Hide file tree
Showing 25 changed files with 128 additions and 39 deletions.
4 changes: 2 additions & 2 deletions example/ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ PODS:
- ReactCommon/turbomodule/bridging
- ReactCommon/turbomodule/core
- Yoga
- NitroModules (0.8.0):
- NitroModules (0.9.1):
- DoubleConversion
- glog
- hermes-engine
Expand Down Expand Up @@ -1828,7 +1828,7 @@ SPEC CHECKSUMS:
glog: fdfdfe5479092de0c4bdbebedd9056951f092c4f
hermes-engine: 3b6e0717ca847e2fc90a201e59db36caf04dee88
NitroImage: 0cffeee137c14b8c8df97649626646528cb89b28
NitroModules: 7184d6a9527f2d015c7e4865ee6615b3e26d9cfb
NitroModules: a3e3619f2c74dd9a706d3597d7c359d56e94ef22
RCT-Folly: 02617c592a293bd6d418e0a88ff4ee1f88329b47
RCTDeprecation: 34cbf122b623037ea9facad2e92e53434c5c7422
RCTRequired: 24c446d7bcd0f517d516b6265d8df04dc3eb1219
Expand Down
10 changes: 10 additions & 0 deletions example/src/getTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,16 @@ export function getTests(
.didNotThrow()
.equals('passed')
),
createTest('tryOptionalEnum(...)', () =>
it(() => testObject.tryOptionalEnum('gas'))
.didNotThrow()
.equals('gas')
),
createTest('tryOptionalEnum(...)', () =>
it(() => testObject.tryOptionalEnum(undefined))
.didNotThrow()
.equals(undefined)
),

// Variants tests
...('someVariant' in testObject
Expand Down
4 changes: 3 additions & 1 deletion packages/nitrogen/src/syntax/createType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,9 @@ export function createType(type: TSMorphType, isOptional: boolean): Type {
`Anonymous objects cannot be represented in C++! Extract "${type.getText()}" to a separate interface/type declaration.`
)
} else if (type.isStringLiteral()) {
return new StringType()
throw new Error(
`String literal ${type.getText()} cannot be represented in C++ because it is ambiguous between a string and a discriminating union enum.`
)
} else {
throw new Error(
`The TypeScript type "${type.getText()}" cannot be represented in C++!`
Expand Down
14 changes: 12 additions & 2 deletions packages/nitrogen/src/syntax/swift/SwiftCxxBridgedType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ export class SwiftCxxBridgedType implements BridgedType<'swift', 'c++'> {
case 'enum':
// Enums cannot be referenced from C++ <-> Swift bi-directionally,
// so we just pass the underlying raw value (int32), and cast from Int <-> Enum.
if (this.isBridgingToDirectCppTarget) {
// ...unless we bridge directly to a C++ target. Then we don't need special conversion.
return false
}
return true
case 'hybrid-object':
// Swift HybridObjects need to be wrapped in our own *Cxx Swift classes.
Expand Down Expand Up @@ -83,6 +87,9 @@ export class SwiftCxxBridgedType implements BridgedType<'swift', 'c++'> {
return true
case 'array-buffer':
// ArrayBufferHolder <> std::shared_ptr<ArrayBuffer>
if (this.isBridgingToDirectCppTarget) {
return false
}
return true
case 'promise':
// PromiseHolder<T> <> std::shared_ptr<std::promise<T>>
Expand Down Expand Up @@ -348,9 +355,12 @@ export class SwiftCxxBridgedType implements BridgedType<'swift', 'c++'> {
}
case 'optional': {
const optional = getTypeAs(this.type, OptionalType)
const wrapping = new SwiftCxxBridgedType(optional.wrappingType)
const wrapping = new SwiftCxxBridgedType(optional.wrappingType, true)
switch (language) {
case 'swift':
if (!wrapping.needsSpecialHandling) {
return `${cppParameterName}.value`
}
return `
{ () -> ${optional.getCode('swift')} in
if let actualValue = ${cppParameterName}.value {
Expand Down Expand Up @@ -525,7 +535,7 @@ case ${i}:
}
case 'optional': {
const optional = getTypeAs(this.type, OptionalType)
const wrapping = new SwiftCxxBridgedType(optional.wrappingType)
const wrapping = new SwiftCxxBridgedType(optional.wrappingType, true)
const bridge = this.getBridgeOrThrow()
const makeFunc = `bridge.${bridge.funcName}`
switch (language) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ class HybridTestObjectKotlin: HybridTestObjectSwiftKotlinSpec() {
return str
}


override fun tryOptionalEnum(value: Powertrain?): Powertrain? {
return value
}

override fun calculateFibonacciSync(value: Double): Long {
val n = value.toInt()
if (n == 0) return 0L
Expand Down
4 changes: 4 additions & 0 deletions packages/react-native-nitro-image/cpp/HybridTestObjectCpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,10 @@ std::string HybridTestObjectCpp::tryMiddleParam(double num, std::optional<bool>
return str;
}

std::optional<Powertrain> HybridTestObjectCpp::tryOptionalEnum(std::optional<Powertrain> value) {
return value;
}

std::variant<std::string, double>
HybridTestObjectCpp::passVariant(const std::variant<std::string, double, bool, std::vector<double>, std::vector<std::string>>& either) {
if (std::holds_alternative<std::string>(either)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class HybridTestObjectCpp : public HybridTestObjectCppSpec {
double funcThatThrows() override;
std::string tryOptionalParams(double num, bool boo, const std::optional<std::string>& str) override;
std::string tryMiddleParam(double num, std::optional<bool> boo, const std::string& str) override;
std::optional<Powertrain> tryOptionalEnum(std::optional<Powertrain> value) override;
std::variant<std::string, double>
passVariant(const std::variant<std::string, double, bool, std::vector<double>, std::vector<std::string>>& either) override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ class HybridTestObjectSwift : HybridTestObjectSwiftKotlinSpec {
return str
}

func tryOptionalEnum(value: Powertrain?) throws -> Powertrain? {
return value
}

func calculateFibonacciSync(value: Double) throws -> Int64 {
let n = Int64(value)
if n <= 1 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
namespace margelo::nitro::image { class HybridTestObjectSwiftKotlinSpec; }
// Forward declaration of `AnyMap` to properly resolve imports.
namespace NitroModules { class AnyMap; }
// Forward declaration of `Car` to properly resolve imports.
namespace margelo::nitro::image { struct Car; }
// Forward declaration of `Powertrain` to properly resolve imports.
namespace margelo::nitro::image { enum class Powertrain; }
// Forward declaration of `Car` to properly resolve imports.
namespace margelo::nitro::image { struct Car; }
// Forward declaration of `Person` to properly resolve imports.
namespace margelo::nitro::image { struct Person; }
// Forward declaration of `ArrayBuffer` to properly resolve imports.
Expand All @@ -28,12 +28,12 @@ namespace NitroModules { class ArrayBuffer; }
#include <NitroModules/JNISharedPtr.hpp>
#include <NitroModules/AnyMap.hpp>
#include <NitroModules/JAnyMap.hpp>
#include "Powertrain.hpp"
#include "JPowertrain.hpp"
#include <future>
#include <NitroModules/JPromise.hpp>
#include "Car.hpp"
#include "JCar.hpp"
#include "Powertrain.hpp"
#include "JPowertrain.hpp"
#include "Person.hpp"
#include "JPerson.hpp"
#include <NitroModules/ArrayBuffer.hpp>
Expand Down Expand Up @@ -172,6 +172,11 @@ namespace margelo::nitro::image {
auto result = method(_javaPart, num, boo.has_value() ? jni::JBoolean::valueOf(boo.value()) : nullptr, jni::make_jstring(str));
return result->toStdString();
}
std::optional<Powertrain> JHybridTestObjectSwiftKotlinSpec::tryOptionalEnum(std::optional<Powertrain> value) {
static const auto method = _javaPart->getClass()->getMethod<jni::local_ref<JPowertrain>(jni::alias_ref<JPowertrain> /* value */)>("tryOptionalEnum");
auto result = method(_javaPart, value.has_value() ? JPowertrain::fromCpp(value.value()) : nullptr);
return result != nullptr ? std::make_optional(result->toCpp()) : std::nullopt;
}
int64_t JHybridTestObjectSwiftKotlinSpec::calculateFibonacciSync(double value) {
static const auto method = _javaPart->getClass()->getMethod<int64_t(double /* value */)>("calculateFibonacciSync");
auto result = method(_javaPart, value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ namespace margelo::nitro::image {
double funcThatThrows() override;
std::string tryOptionalParams(double num, bool boo, const std::optional<std::string>& str) override;
std::string tryMiddleParam(double num, std::optional<bool> boo, const std::string& str) override;
std::optional<Powertrain> tryOptionalEnum(std::optional<Powertrain> value) override;
int64_t calculateFibonacciSync(double value) override;
std::future<int64_t> calculateFibonacciAsync(double value) override;
std::future<void> wait(double seconds) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ abstract class HybridTestObjectSwiftKotlinSpec: HybridObject() {
@Keep
abstract fun tryMiddleParam(num: Double, boo: Boolean?, str: String): String

@DoNotStrip
@Keep
abstract fun tryOptionalEnum(value: Powertrain?): Powertrain?

@DoNotStrip
@Keep
abstract fun calculateFibonacciSync(value: Double): Long
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,14 @@ namespace margelo::nitro::image::bridge::swift {
return std::optional<bool>(value);
}

/**
* Specialized version of `std::optional<Powertrain>`.
*/
using std__optional_Powertrain_ = std::optional<Powertrain>;
inline std::optional<Powertrain> create_std__optional_Powertrain_(const Powertrain& value) {
return std::optional<Powertrain>(value);
}

/**
* Specialized version of `std::vector<double>`.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ namespace margelo::nitro::image { class HybridTestObjectSwiftKotlinSpec; }
namespace margelo::nitro::image { class HybridTestObjectSwiftKotlinSpecSwift; }
// Forward declaration of `AnyMap` to properly resolve imports.
namespace NitroModules { class AnyMap; }
// Forward declaration of `Car` to properly resolve imports.
namespace margelo::nitro::image { struct Car; }
// Forward declaration of `Powertrain` to properly resolve imports.
namespace margelo::nitro::image { enum class Powertrain; }
// Forward declaration of `Car` to properly resolve imports.
namespace margelo::nitro::image { struct Car; }
// Forward declaration of `Person` to properly resolve imports.
namespace margelo::nitro::image { struct Person; }
// Forward declaration of `ArrayBuffer` to properly resolve imports.
Expand All @@ -35,11 +35,11 @@ namespace NitroModules { class ArrayBufferHolder; }
#include "HybridTestObjectSwiftKotlinSpec.hpp"
#include "HybridTestObjectSwiftKotlinSpecSwift.hpp"
#include <NitroModules/AnyMap.hpp>
#include "Powertrain.hpp"
#include <future>
#include <NitroModules/PromiseHolder.hpp>
#include <functional>
#include "Car.hpp"
#include "Powertrain.hpp"
#include "Person.hpp"
#include <NitroModules/ArrayBuffer.hpp>
#include <NitroModules/ArrayBufferHolder.hpp>
Expand Down Expand Up @@ -170,6 +170,10 @@ namespace margelo::nitro::image {
auto __result = _swiftPart.tryMiddleParam(std::forward<decltype(num)>(num), boo, str);
return __result;
}
inline std::optional<Powertrain> tryOptionalEnum(std::optional<Powertrain> value) override {
auto __result = _swiftPart.tryOptionalEnum(value);
return __result;
}
inline int64_t calculateFibonacciSync(double value) override {
auto __result = _swiftPart.calculateFibonacciSync(std::forward<decltype(value)>(value));
return __result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public protocol HybridTestObjectSwiftKotlinSpec: HybridObjectSpec {
func funcThatThrows() throws -> Double
func tryOptionalParams(num: Double, boo: Bool, str: String?) throws -> String
func tryMiddleParam(num: Double, boo: Bool?, str: String) throws -> String
func tryOptionalEnum(value: Powertrain?) throws -> Powertrain?
func calculateFibonacciSync(value: Double) throws -> Int64
func calculateFibonacciAsync(value: Double) throws -> Promise<Int64>
func wait(seconds: Double) throws -> Promise<Void>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,14 +277,25 @@ public final class HybridTestObjectSwiftKotlinSpecCxx {
@inline(__always)
public func tryMiddleParam(num: Double, boo: bridge.std__optional_bool_, str: std.string) -> std.string {
do {
let result = try self.implementation.tryMiddleParam(num: num, boo: { () -> Bool? in
if let actualValue = boo.value {
return actualValue
let result = try self.implementation.tryMiddleParam(num: num, boo: boo.value, str: String(str))
return std.string(result)
} catch {
let message = "\(error.localizedDescription)"
fatalError("Swift errors can currently not be propagated to C++! See https://github.com/swiftlang/swift/issues/75290 (Error: \(message))")
}
}

@inline(__always)
public func tryOptionalEnum(value: bridge.std__optional_Powertrain_) -> bridge.std__optional_Powertrain_ {
do {
let result = try self.implementation.tryOptionalEnum(value: value.value)
return { () -> bridge.std__optional_Powertrain_ in
if let actualValue = result {
return bridge.create_std__optional_Powertrain_(actualValue)
} else {
return nil
return .init()
}
}(), str: String(str))
return std.string(result)
}()
} catch {
let message = "\(error.localizedDescription)"
fatalError("Swift errors can currently not be propagated to C++! See https://github.com/swiftlang/swift/issues/75290 (Error: \(message))")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ namespace margelo::nitro::image {
prototype.registerHybridMethod("funcThatThrows", &HybridTestObjectCppSpec::funcThatThrows);
prototype.registerHybridMethod("tryOptionalParams", &HybridTestObjectCppSpec::tryOptionalParams);
prototype.registerHybridMethod("tryMiddleParam", &HybridTestObjectCppSpec::tryMiddleParam);
prototype.registerHybridMethod("tryOptionalEnum", &HybridTestObjectCppSpec::tryOptionalEnum);
prototype.registerHybridMethod("passVariant", &HybridTestObjectCppSpec::passVariant);
prototype.registerHybridMethod("getVariantEnum", &HybridTestObjectCppSpec::getVariantEnum);
prototype.registerHybridMethod("getVariantObjects", &HybridTestObjectCppSpec::getVariantObjects);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
namespace margelo::nitro::image { class HybridTestObjectCppSpec; }
// Forward declaration of `AnyMap` to properly resolve imports.
namespace NitroModules { class AnyMap; }
// Forward declaration of `Powertrain` to properly resolve imports.
namespace margelo::nitro::image { enum class Powertrain; }
// Forward declaration of `OldEnum` to properly resolve imports.
namespace margelo::nitro::image { enum class OldEnum; }
// Forward declaration of `Person` to properly resolve imports.
Expand All @@ -33,6 +35,7 @@ namespace NitroModules { class ArrayBuffer; }
#include <memory>
#include "HybridTestObjectCppSpec.hpp"
#include <NitroModules/AnyMap.hpp>
#include "Powertrain.hpp"
#include <vector>
#include "OldEnum.hpp"
#include "Person.hpp"
Expand Down Expand Up @@ -96,6 +99,7 @@ namespace margelo::nitro::image {
virtual double funcThatThrows() = 0;
virtual std::string tryOptionalParams(double num, bool boo, const std::optional<std::string>& str) = 0;
virtual std::string tryMiddleParam(double num, std::optional<bool> boo, const std::string& str) = 0;
virtual std::optional<Powertrain> tryOptionalEnum(std::optional<Powertrain> value) = 0;
virtual std::variant<std::string, double> passVariant(const std::variant<std::string, double, bool, std::vector<double>, std::vector<std::string>>& either) = 0;
virtual std::variant<bool, OldEnum> getVariantEnum(const std::variant<bool, OldEnum>& variant) = 0;
virtual std::variant<Person, Car> getVariantObjects(const std::variant<Person, Car>& variant) = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ namespace margelo::nitro::image {
prototype.registerHybridMethod("funcThatThrows", &HybridTestObjectSwiftKotlinSpec::funcThatThrows);
prototype.registerHybridMethod("tryOptionalParams", &HybridTestObjectSwiftKotlinSpec::tryOptionalParams);
prototype.registerHybridMethod("tryMiddleParam", &HybridTestObjectSwiftKotlinSpec::tryMiddleParam);
prototype.registerHybridMethod("tryOptionalEnum", &HybridTestObjectSwiftKotlinSpec::tryOptionalEnum);
prototype.registerHybridMethod("calculateFibonacciSync", &HybridTestObjectSwiftKotlinSpec::calculateFibonacciSync);
prototype.registerHybridMethod("calculateFibonacciAsync", &HybridTestObjectSwiftKotlinSpec::calculateFibonacciAsync);
prototype.registerHybridMethod("wait", &HybridTestObjectSwiftKotlinSpec::wait);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
namespace margelo::nitro::image { class HybridTestObjectSwiftKotlinSpec; }
// Forward declaration of `AnyMap` to properly resolve imports.
namespace NitroModules { class AnyMap; }
// Forward declaration of `Powertrain` to properly resolve imports.
namespace margelo::nitro::image { enum class Powertrain; }
// Forward declaration of `Car` to properly resolve imports.
namespace margelo::nitro::image { struct Car; }
// Forward declaration of `Person` to properly resolve imports.
Expand All @@ -29,6 +31,7 @@ namespace NitroModules { class ArrayBuffer; }
#include <memory>
#include "HybridTestObjectSwiftKotlinSpec.hpp"
#include <NitroModules/AnyMap.hpp>
#include "Powertrain.hpp"
#include <future>
#include <functional>
#include "Car.hpp"
Expand Down Expand Up @@ -86,6 +89,7 @@ namespace margelo::nitro::image {
virtual double funcThatThrows() = 0;
virtual std::string tryOptionalParams(double num, bool boo, const std::optional<std::string>& str) = 0;
virtual std::string tryMiddleParam(double num, std::optional<bool> boo, const std::string& str) = 0;
virtual std::optional<Powertrain> tryOptionalEnum(std::optional<Powertrain> value) = 0;
virtual int64_t calculateFibonacciSync(double value) = 0;
virtual std::future<int64_t> calculateFibonacciAsync(double value) = 0;
virtual std::future<void> wait(double seconds) = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export interface TestObjectCpp extends HybridObject<{ ios: 'c++' }> {
// Optional parameters
tryOptionalParams(num: number, boo: boolean, str?: string): string
tryMiddleParam(num: number, boo: boolean | undefined, str: string): string
tryOptionalEnum(value?: Powertrain): Powertrain | undefined

// Variants
someVariant: number | string
Expand Down Expand Up @@ -127,6 +128,7 @@ export interface TestObjectSwiftKotlin
// Optional parameters
tryOptionalParams(num: number, boo: boolean, str?: string): string
tryMiddleParam(num: number, boo: boolean | undefined, str: string): string
tryOptionalEnum(value?: Powertrain): Powertrain | undefined

// TODO: Variants are not yet supported in Swift/Kotlin!
// Variants
Expand Down
Loading

0 comments on commit 46c9ca0

Please sign in to comment.