From 67bb07bb63e734acc519e5a92c60ad495b2ecb15 Mon Sep 17 00:00:00 2001 From: jhugman Date: Wed, 16 Oct 2024 22:15:28 +0100 Subject: [PATCH] Allow turbo-modules with names with @my-org/project-name (#127) Fixes #124 According to [The Big O of Code Reviews](https://www.egorand.dev/the-big-o-of-code-reviews/), this is a O(_n_) change. This adds to the tests introduced in #123 to support names with an org name. --- crates/ubrn_cli/src/codegen/mod.rs | 2 +- .../src/codegen/templates/CMakeLists.txt | 5 +- .../codegen/templates/module-template.podspec | 2 +- crates/ubrn_cli/src/config/mod.rs | 35 ++- crates/ubrn_cli/src/config/npm.rs | 21 +- crates/ubrn_cli/src/ios.rs | 15 +- .../fixtures/turbo-module-testing/App.tsx | 56 +++++ scripts/test-turbo-modules.sh | 206 ++++++++++++------ 8 files changed, 257 insertions(+), 85 deletions(-) create mode 100644 integration/fixtures/turbo-module-testing/App.tsx diff --git a/crates/ubrn_cli/src/codegen/mod.rs b/crates/ubrn_cli/src/codegen/mod.rs index 7b15b744..4071e183 100644 --- a/crates/ubrn_cli/src/codegen/mod.rs +++ b/crates/ubrn_cli/src/codegen/mod.rs @@ -327,7 +327,7 @@ mod files { templated_file!(PodspecTemplate, "module-template.podspec"); impl RenderedFile for PodspecTemplate { fn path(&self, project_root: &Utf8Path) -> Utf8PathBuf { - let name = self.config.project.raw_name(); + let name = self.config.project.podspec_filename(); let filename = format!("{name}.podspec"); project_root.join(filename) } diff --git a/crates/ubrn_cli/src/codegen/templates/CMakeLists.txt b/crates/ubrn_cli/src/codegen/templates/CMakeLists.txt index d976034f..17419703 100644 --- a/crates/ubrn_cli/src/codegen/templates/CMakeLists.txt +++ b/crates/ubrn_cli/src/codegen/templates/CMakeLists.txt @@ -30,8 +30,7 @@ include_directories( ${UNIFFI_BINDGEN_PATH}/cpp/includes ) -add_library( - ${CMAKE_PROJECT_NAME} SHARED +add_library({{ self.config.project.cpp_filename() }} SHARED {{ tm_dir }}/{{ self.config.project.cpp_filename() }}.cpp {%- for m in self.config.modules %} {{ bindings_dir }}/{{ m.cpp_filename() }} @@ -63,7 +62,7 @@ find_package(fbjni REQUIRED CONFIG) find_library(LOGCAT log) target_link_libraries( - ${CMAKE_PROJECT_NAME} + {{ self.config.project.cpp_filename() }} fbjni::fbjni ReactAndroid::jsi ReactAndroid::turbomodulejsijni diff --git a/crates/ubrn_cli/src/codegen/templates/module-template.podspec b/crates/ubrn_cli/src/codegen/templates/module-template.podspec index d37bec0b..14f4f9cc 100644 --- a/crates/ubrn_cli/src/codegen/templates/module-template.podspec +++ b/crates/ubrn_cli/src/codegen/templates/module-template.podspec @@ -5,7 +5,7 @@ package = JSON.parse(File.read(File.join(__dir__, "package.json"))) folly_compiler_flags = '-DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1 -Wno-comma -Wno-shorten-64-to-32' Pod::Spec.new do |s| - s.name = "{{ self.config.project.raw_name() }}" + s.name = "{{ self.config.project.cpp_filename() }}" s.version = package["version"] s.summary = package["description"] s.homepage = package["homepage"] diff --git a/crates/ubrn_cli/src/config/mod.rs b/crates/ubrn_cli/src/config/mod.rs index 3fda1216..4e238117 100644 --- a/crates/ubrn_cli/src/config/mod.rs +++ b/crates/ubrn_cli/src/config/mod.rs @@ -76,13 +76,31 @@ pub(crate) fn trim_react_native(name: &str) -> String { .to_string() } +pub(crate) fn org_and_name(name: &str) -> (Option<&str>, &str) { + if let Some((left, right)) = name.split_once('/') { + let org = left.strip_prefix('@').unwrap_or(left); + (Some(org), right) + } else { + (None, name) + } +} + +pub(crate) fn lower(s: &str) -> String { + s.to_upper_camel_case().to_lowercase() +} + impl ProjectConfig { pub(crate) fn project_root(&self) -> &Utf8Path { &self.crate_.project_root } pub(crate) fn module_cpp(&self) -> String { - trim_react_native(&self.name).to_upper_camel_case() + let (org, name) = org_and_name(&self.name); + if org.is_some() { + name.to_upper_camel_case() + } else { + trim_react_native(name).to_upper_camel_case() + } } } @@ -96,9 +114,12 @@ impl ProjectConfig { } pub(crate) fn cpp_namespace(&self) -> String { - trim_react_native(&self.name) - .to_upper_camel_case() - .to_lowercase() + let (org, name) = org_and_name(&self.name); + if let Some(org) = org { + format!("{}_{}", lower(org), lower(name)) + } else { + lower(&trim_react_native(name)) + } } pub(crate) fn cpp_filename(&self) -> String { @@ -106,12 +127,16 @@ impl ProjectConfig { self.raw_name().to_kebab_case() } + pub(crate) fn podspec_filename(&self) -> String { + self.cpp_filename() + } + pub(crate) fn codegen_filename(&self) -> String { format!("Native{}", self.spec_name()) } pub(crate) fn spec_name(&self) -> String { - trim_react_native(&self.name).to_upper_camel_case() + self.module_cpp() } pub(crate) fn exclude_files(&self) -> &GlobSet { diff --git a/crates/ubrn_cli/src/config/npm.rs b/crates/ubrn_cli/src/config/npm.rs index 3a64056f..956b193f 100644 --- a/crates/ubrn_cli/src/config/npm.rs +++ b/crates/ubrn_cli/src/config/npm.rs @@ -4,9 +4,10 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/ */ -use heck::ToUpperCamelCase; use serde::Deserialize; +use crate::config::{lower, org_and_name}; + use super::{trim, trim_react_native}; #[derive(Deserialize)] @@ -34,14 +35,16 @@ impl PackageJson { .android .java_package_name .clone() - .unwrap_or_else(|| { - format!( - "com.{}", - trim_react_native(&self.name) - .to_upper_camel_case() - .to_lowercase() - ) - }) + .unwrap_or_else(|| self.default_android_package_name()) + } + + fn default_android_package_name(&self) -> String { + let (org, name) = org_and_name(&self.name); + if let Some(org) = org { + format!("com.{}.{}", lower(org), lower(name)) + } else { + format!("com.{}", lower(&trim_react_native(name))) + } } pub(crate) fn repo(&self) -> &PackageJsonRepo { diff --git a/crates/ubrn_cli/src/ios.rs b/crates/ubrn_cli/src/ios.rs index 53a6a15f..0d29af82 100644 --- a/crates/ubrn_cli/src/ios.rs +++ b/crates/ubrn_cli/src/ios.rs @@ -9,12 +9,13 @@ use std::{collections::HashMap, fmt::Display, process::Command, str::FromStr}; use anyhow::{Context, Error, Result}; use camino::{Utf8Path, Utf8PathBuf}; use clap::Args; +use heck::ToUpperCamelCase; use serde::{Deserialize, Serialize}; use ubrn_common::{mk_dir, rm_dir, run_cmd, CrateMetadata}; use crate::{ building::{CommonBuildArgs, ExtraArgs}, - config::{trim_react_native, ProjectConfig}, + config::{org_and_name, trim_react_native, ProjectConfig}, rust::CrateConfig, workspace, }; @@ -44,10 +45,14 @@ impl IOsConfig { } fn default_framework_name() -> String { - format!( - "{}Framework", - trim_react_native(&workspace::package_json().name()) - ) + let name = workspace::package_json().name(); + let (org, name) = org_and_name(&name); + let prefix = if let Some(org) = org { + format!("{}_{}", org, name).to_upper_camel_case() + } else { + trim_react_native(name).to_upper_camel_case() + }; + format!("{prefix}Framework") } fn default_cargo_extras() -> ExtraArgs { diff --git a/integration/fixtures/turbo-module-testing/App.tsx b/integration/fixtures/turbo-module-testing/App.tsx new file mode 100644 index 00000000..c6f3a6a3 --- /dev/null +++ b/integration/fixtures/turbo-module-testing/App.tsx @@ -0,0 +1,56 @@ +import { StyleSheet, View, Text } from "react-native"; +import { multiply } from "react-native-by-hand"; +import { + Calculator, + type BinaryOperator, + SafeAddition, + ComputationResult, +} from "../../src"; + +// A Rust object +const calculator = new Calculator(); +// A Rust object implementing the Rust trait BinaryOperator +const addOp = new SafeAddition(); + +// A Typescript class, implementing BinaryOperator +class SafeMultiply implements BinaryOperator { + perform(lhs: bigint, rhs: bigint): bigint { + return lhs * rhs; + } +} +const multOp = new SafeMultiply(); + +// bigints +const three = 3n; +const seven = 7n; + +// Perform the calculation, and to get an object +// representing the computation result. +const computation: ComputationResult = calculator + .calculate(addOp, three, three) + .calculateMore(multOp, seven) + .lastResult()!; + +// Unpack the bigint value into a string. +const result = computation.value.toString(); + +export default function App() { + return ( + + Result: {result} + + ); +} + +const styles = StyleSheet.create({ + container: { + flex: 1, + alignItems: "center", + justifyContent: "center", + }, + box: { + width: 60, + height: 60, + marginVertical: 20, + }, +}); diff --git a/scripts/test-turbo-modules.sh b/scripts/test-turbo-modules.sh index 580749a4..96d66c66 100755 --- a/scripts/test-turbo-modules.sh +++ b/scripts/test-turbo-modules.sh @@ -6,7 +6,7 @@ PWD= reset_args() { PROJECT_DIR=my-test-library - KEEP_ROOT_ON_ERROR=false + KEEP_ROOT_ON_EXIT=false BOB_VERSION=latest PROJECT_SLUG=my-test-library FORCE_NEW_DIR=false @@ -14,6 +14,7 @@ reset_args() { SKIP_IOS=false SKIP_ANDROID=false UBRN_CONFIG= + APP_TSX= } usage() { @@ -23,6 +24,7 @@ usage() { echo " -A, --skip-android Skip building for Android." echo " -I, --skip-ios Skip building for iOS." echo " -C, --ubrn-config Use a ubrn config file." + echo " -T, --app-tsx Use a App.tsx file." echo " -u, --builder-bob-version VERSION Specify the version of builder-bob to use." echo " -s, --slug PROJECT_SLUG Specify the project slug." @@ -48,7 +50,7 @@ diagnostics() { } error() { - if [ "$KEEP_ROOT_ON_ERROR" == false ] && [ -d "$PROJECT_DIR" ]; then + if [ "$KEEP_ROOT_ON_EXIT" == false ] && [ -d "$PROJECT_DIR" ]; then cleanup fi diagnostics @@ -69,6 +71,16 @@ derive_paths() { PWD=$(pwd) } +join_paths() { + local prefix="$1" + local suffix="$2" + if [[ "$suffix" = /* ]] ; then + echo -n "$suffix" + else + echo -n "$prefix/$suffix" + fi +} + parse_cli_options() { reset_args # Parse command line options @@ -87,17 +99,15 @@ parse_cli_options() { shift ;; -C|--ubrn-config) - local config_file - config_file="$2" - if [[ "$config_file" = /* ]] ; then - UBRN_CONFIG="$config_file" - else - UBRN_CONFIG="$PWD/$config_file" - fi + UBRN_CONFIG=$(join_paths "$PWD" "$2") + shift + ;; + -T|--app-tsx) + APP_TSX=$(join_paths "$PWD" "$2") shift ;; -k|--keep-directory-on-exit) - KEEP_ROOT_ON_ERROR=true + KEEP_ROOT_ON_EXIT=true ;; -f|--force-new-directory) FORCE_NEW_DIR=true @@ -113,7 +123,7 @@ parse_cli_options() { exit 0 ;; -*) - KEEP_ROOT_ON_ERROR=true + KEEP_ROOT_ON_EXIT=true error "Bad argument: $1" ;; *) @@ -229,6 +239,7 @@ check_lines() { check_line_unchanged "./src/Native*" "getEnforcing" check_line_unchanged "./android/CMakeLists.txt" "^project" + check_line_unchanged "./android/CMakeLists.txt" "^add_library.*SHARED" check_line_unchanged "./android/build.gradle" "return rootProject" check_line_unchanged "./android/build.gradle" "libraryName" check_line_unchanged "./android/src/*/*Package*" "package" @@ -247,10 +258,11 @@ check_lines() { check_line_unchanged "./ios/*.mm" "#import \"" check_line_unchanged "./ios/*.mm" "@implementation" check_line_unchanged "./ios/*.mm" "::multiply" + check_line_unchanged "./*.podspec" "s.name" } clean_turbo_modules() { - rm -Rf cpp/ android/src/main/java ios/ src/Native* src/generated/ src/index.ts* + rm -Rf cpp/ android/src/main/java ios/ src/Native* src/generated/ src/index.ts* ./*.podspec } generate_turbo_module_for_diffing() { @@ -278,6 +290,9 @@ generate_turbo_module_for_compiling() { echo "-- Running ubrn checkout" clean_turbo_modules "$UBRN_BIN" checkout --config "$UBRN_CONFIG" + if [ -f "$APP_TSX" ] ; then + cp "$APP_TSX" ./example/src/App.tsx + fi exit_dir } @@ -347,7 +362,9 @@ main() { if [ "$SKIP_IOS" == false ]; then build_ios_example fi - cleanup + if [ "$KEEP_ROOT_ON_EXIT" == false ] && [ -d "$PROJECT_DIR" ]; then + cleanup + fi echo "✅ Success!" } @@ -355,64 +372,121 @@ run_default() { local fixture_dir="$ROOT/integration/fixtures/turbo-module-testing" local working_dir="/tmp/turbomodule-tests" local config="$fixture_dir/ubrn.config.yaml" + local app_tsx="$fixture_dir/App.tsx" main \ - --force-new-directory \ - --keep-directory-on-exit \ - --ubrn-config "$config" \ - --builder-bob-version 0.35.1 \ - --skip-ios \ - --skip-android \ - --slug dummy-lib \ - "$working_dir/dummy-lib" + --force-new-directory \ + --keep-directory-on-exit \ + --ubrn-config "$config" \ + --builder-bob-version 0.35.1 \ + --skip-ios \ + --skip-android \ + --slug dummy-lib \ + "$working_dir/dummy-lib" main \ - --force-new-directory \ - --keep-directory-on-exit \ - --ubrn-config "$config" \ - --builder-bob-version 0.35.1 \ - --skip-ios \ - --skip-android \ - --slug rn-dummy-lib \ - "$working_dir/rn-dummy-lib" + --force-new-directory \ + --keep-directory-on-exit \ + --ubrn-config "$config" \ + --builder-bob-version 0.35.1 \ + --skip-ios \ + --skip-android \ + --slug rn-dummy-lib \ + "$working_dir/rn-dummy-lib" main \ - --force-new-directory \ - --keep-directory-on-exit \ - --ubrn-config "$config" \ - --builder-bob-version 0.35.1 \ - --skip-ios \ - --skip-android \ - --slug react-native-dummy-lib \ - "$working_dir/react-native-dummy-lib" + --force-new-directory \ + --keep-directory-on-exit \ + --ubrn-config "$config" \ + --builder-bob-version 0.35.1 \ + --skip-ios \ + --skip-android \ + --slug react-native-dummy-lib \ + "$working_dir/react-native-dummy-lib" main \ - --force-new-directory \ - --keep-directory-on-exit \ - --ubrn-config "$config" \ - --builder-bob-version 0.35.1 \ - --skip-ios \ - --skip-android \ - --slug dummy-lib-react-native \ - "$working_dir/dummy-lib-react-native" + --force-new-directory \ + --keep-directory-on-exit \ + --ubrn-config "$config" \ + --builder-bob-version 0.35.1 \ + --skip-ios \ + --skip-android \ + --slug dummy-lib-react-native \ + "$working_dir/dummy-lib-react-native" main \ - --force-new-directory \ - --keep-directory-on-exit \ - --ubrn-config "$config" \ - --builder-bob-version 0.35.1 \ - --skip-ios \ - --skip-android \ - --slug dummy-lib-react-native \ - "$working_dir/dummy-lib-rn" + --force-new-directory \ + --keep-directory-on-exit \ + --ubrn-config "$config" \ + --builder-bob-version 0.35.1 \ + --skip-ios \ + --skip-android \ + --slug dummy-lib-react-native \ + "$working_dir/dummy-lib-rn" # ReactNativeDummyLib fails with "› Must be a valid npm package name" + main \ + --force-new-directory \ + --keep-directory-on-exit \ + --ubrn-config "$config" \ + --builder-bob-version 0.35.1 \ + --skip-ios \ + --skip-android \ + --slug @my-org/react-native-dummy-lib \ + "$working_dir/@my-org/react-native-dummy-lib" + main \ + --force-new-directory \ + --keep-directory-on-exit \ + --ubrn-config "$config" \ + --builder-bob-version 0.35.1 \ + --skip-ios \ + --skip-android \ + --slug @my-org/dummy-lib \ + "$working_dir/@my-org/dummy-lib" + main \ + --force-new-directory \ + --keep-directory-on-exit \ + --ubrn-config "$config" \ + --builder-bob-version 0.35.1 \ + --skip-ios \ + --skip-android \ + --slug @react-native/dummy-lib \ + "$working_dir/@react-native/dummy-lib" + main \ + --force-new-directory \ + --keep-directory-on-exit \ + --ubrn-config "$config" \ + --builder-bob-version 0.35.1 \ + --skip-ios \ + --skip-android \ + --slug @react-native-org/dummy-lib \ + "$working_dir/@react-native-org/dummy-lib" + main \ + --force-new-directory \ + --keep-directory-on-exit \ + --ubrn-config "$config" \ + --builder-bob-version 0.35.1 \ + --skip-ios \ + --skip-android \ + --slug @react-native/dummy-lib \ + "$working_dir/@react-native/react-native-lib" local os os=$(uname -o) if [ "$os" == "Darwin" ] ; then main \ - --force-new-directory \ - --keep-directory-on-exit \ - --ubrn-config "$config" \ - --builder-bob-version 0.35.1 \ - --slug react-native-dummy-lib-for-ios \ - --skip-android \ - --ios-name DummyLibForIos \ - "$working_dir/react-native-dummy-lib-for-ios" + --force-new-directory \ + --keep-directory-on-exit \ + --ubrn-config "$config" \ + --builder-bob-version 0.35.1 \ + --slug react-native-dummy-lib-for-ios \ + --skip-android \ + --app-tsx "$app_tsx" \ + --ios-name DummyLibForIos \ + "$working_dir/react-native-dummy-lib-for-ios" + main \ + --force-new-directory \ + --keep-directory-on-exit \ + --ubrn-config "$config" \ + --builder-bob-version 0.35.1 \ + --skip-android \ + --app-tsx "$app_tsx" \ + --ios-name ReactNativeDummyLibForIos \ + --slug @my-org/react-native-dummy-lib-for-ios \ + "$working_dir/@my-org/react-native-dummy-lib-for-ios" fi main \ --force-new-directory \ @@ -421,7 +495,17 @@ run_default() { --builder-bob-version 0.35.1 \ --slug react-native-dummy-lib-for-android \ --skip-ios \ + --app-tsx "$app_tsx" \ "$working_dir/react-native-dummy-lib-for-android" + main \ + --force-new-directory \ + --keep-directory-on-exit \ + --ubrn-config "$config" \ + --builder-bob-version 0.35.1 \ + --skip-ios \ + --app-tsx "$app_tsx" \ + --slug @my-org/react-native-dummy-lib-for-android \ + "$working_dir/@my-org/react-native-dummy-lib-for-android" } derive_paths