Switch from ArrayBuffer to ByteArray (#187)

According to [The Big O of Code Reviews](https://www.egorand.dev/the-big-o-of-code-reviews/), this is a O(_n_) change. ubrn lowers most types to an `ArrayBuffer` and then maps it to a `jsi::ArrayBuffer`, and then on to `RustBuffer`. `wasm-bindgen` maps a `Uint8Array` on to a `Vec<u8>`. In order to lower types to a `Uint8Array` for WASM and `ArrayBuffer` for JSI, some amount of bundler violence was needed. Given we're using multiple bundlers to package up the frontend Typescript, _and_ we're not really in control of what bundler the developer uses, this doesn't seem like a viable approach. Instead, this PR changes ubrn to lower types down to a `Uint8Array`, so that both WASM and React Native use the same underlying representation of byte arrays. Unfortunately, jsi doesn't expose API for typed arrays (it may in the future), so we look up the [`buffer` property](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray#instance_properties) to get an underlying `ArrayBuffer`. On the way back, an object of type `{ buffer: ArrayBuffer; }` is returned, which is enough to make the uniffi work and the typescript compiler think it that a typed array has come back from Rust/C++. Most of this PR was mechanical refactoring (e.g. `ArrayBuffer` renamed to `UniffiByteArray`) but there are enough manual changes which merit it being O(_n_).
jhugman · Dec 17, 2024 · 0ea522e · 0ea522e
1 parent 093bb40
commit 0ea522e
Show file tree

Hide file tree

Showing 22 changed files with 177 additions and 99 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1,30 @@
+# Upcoming releases
+
+## 🦊 What's Changed
+* Switched from passing `ArrayBuffer`s to using `Uint8Array`, to accommodate WASM better. ([#187](https://github.com/jhugman/uniffi-bindgen-react-native/pull/187))
+
+**Full Changelog**: https://github.com/jhugman/uniffi-bindgen-react-native/compare/0.28.3-1...main
+
+# 0.28.3-1
+
+This is the first supported release of the `uniffi-bindgen-react-native`. Please hack responsibly. Share and enjoy.
+
+## 🦊 What's Changed
+* Handle type parameter change in crnl 0.45.1 ([#182](https://github.com/jhugman/uniffi-bindgen-react-native/pull/182))
+* Make first run more informative while compiling ([#185](https://github.com/jhugman/uniffi-bindgen-react-native/pull/185))
+* Initial refactor in preparing for WASM ([#174](https://github.com/jhugman/uniffi-bindgen-react-native/pull/174))
+* Add callbacks-example fixture from uniffi-rs ([#172](https://github.com/jhugman/uniffi-bindgen-react-native/pull/172))
+* Fix CLI working without an extension ([#183](https://github.com/jhugman/uniffi-bindgen-react-native/pull/183))
+* Use version released to Cocoapods and npm ([#184](https://github.com/jhugman/uniffi-bindgen-react-native/pull/184))
+
+**Full Changelog**: https://github.com/jhugman/uniffi-bindgen-react-native/compare/0.28.3-0...0.28.3-1
+
+/*
+## ✨ What's New
+
+## 🦊 What's Changed
+
+## ⚠️ Breaking Changes
+
+**Full Changelog**: https://github.com/jhugman/uniffi-bindgen-react-native/compare/{{previous}}...{{current}}
+*/
diff --git a/cpp/includes/ForeignBytes.h b/cpp/includes/ForeignBytes.h
@@ -12,27 +12,3 @@ struct ForeignBytes {
   int32_t len;
   uint8_t *data;
 };
-
-namespace uniffi_jsi {
-using namespace facebook;
-
-template <> struct Bridging<ForeignBytes> {
-  static ForeignBytes fromJs(jsi::Runtime &rt, const jsi::Value &value) {
-    try {
-      auto buffer = value.asObject(rt).getArrayBuffer(rt);
-      return ForeignBytes{
-          .len = static_cast<int32_t>(buffer.length(rt)),
-          .data = buffer.data(rt),
-      };
-    } catch (const std::logic_error &e) {
-      throw jsi::JSError(rt, e.what());
-    }
-  }
-
-  static jsi::Value toJs(jsi::Runtime &rt, std::shared_ptr<CallInvoker>,
-                         ForeignBytes value) {
-    throw jsi::JSError(rt, "Unreachable ForeignBytes.toJs");
-  }
-};
-
-} // namespace uniffi_jsi
diff --git a/cpp/includes/UniffiByteArray.h b/cpp/includes/UniffiByteArray.h
@@ -0,0 +1,38 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/
+ */
+#pragma once
+
+#include "Bridging.h"
+#include <jsi/jsi.h>
+
+namespace uniffi_jsi {
+using namespace facebook;
+
+template <> struct Bridging<jsi::ArrayBuffer> {
+  static jsi::ArrayBuffer value_to_arraybuffer(jsi::Runtime &rt,
+                                               const jsi::Value &value) {
+    try {
+      return value.asObject(rt)
+          .getPropertyAsObject(rt, "buffer")
+          .getArrayBuffer(rt);
+    } catch (const std::logic_error &e) {
+      throw jsi::JSError(rt, e.what());
+    }
+  }
+
+  static jsi::Value arraybuffer_to_value(jsi::Runtime &rt,
+                                         const jsi::ArrayBuffer &arrayBuffer) {
+    try {
+      jsi::Object obj(rt);
+      obj.setProperty(rt, "buffer", arrayBuffer);
+      return jsi::Value(rt, obj);
+    } catch (const std::logic_error &e) {
+      throw jsi::JSError(rt, e.what());
+    }
+  }
+};
+
+} // namespace uniffi_jsi
diff --git a/cpp/includes/UniffiJsiTypes.h b/cpp/includes/UniffiJsiTypes.h
@@ -21,6 +21,7 @@
 #include "ForeignBytes.h"
 #include "ReferenceHolder.h"
 #include "RustBuffer.h"
+#include "UniffiByteArray.h"
 #include "UniffiCallInvoker.h"
 #include "UniffiRustCallStatus.h"
 #include "UniffiString.h"

diff --git a/cpp/includes/UniffiString.h b/cpp/includes/UniffiString.h
@@ -15,7 +15,9 @@ template <> struct Bridging<std::string> {
   static jsi::Value arraybuffer_to_string(jsi::Runtime &rt,
                                           const jsi::Value &value) {
     try {
-      auto buffer = value.asObject(rt).getArrayBuffer(rt);
+      auto buffer =
+          uniffi_jsi::Bridging<jsi::ArrayBuffer>::value_to_arraybuffer(rt,
+                                                                       value);
       auto string =
           jsi::String::createFromUtf8(rt, buffer.data(rt), buffer.length(rt));
       return jsi::Value(rt, string);
@@ -40,7 +42,8 @@ template <> struct Bridging<std::string> {
           std::make_shared<CMutableBuffer>(CMutableBuffer(bytes, len));
       auto arrayBuffer = jsi::ArrayBuffer(rt, payload);
 
-      return jsi::Value(rt, arrayBuffer);
+      return uniffi_jsi::Bridging<jsi::ArrayBuffer>::arraybuffer_to_value(
+          rt, arrayBuffer);
     } catch (const std::logic_error &e) {
       throw jsi::JSError(rt, e.what());
     }

diff --git a/crates/ubrn_bindgen/src/bindings/gen_cpp/templates/RustBufferHelper.cpp b/crates/ubrn_bindgen/src/bindings/gen_cpp/templates/RustBufferHelper.cpp
@@ -11,17 +11,34 @@ template <> struct Bridging<RustBuffer> {
       );
   }
 
+  static void rustbuffer_free(RustBuffer buf) {
+    RustCallStatus status = { UNIFFI_CALL_STATUS_OK };
+    {{ ci.ffi_rustbuffer_free().name() }}(
+        buf,
+        &status
+    );
+  }
+
+  static RustBuffer rustbuffer_from_bytes(ForeignBytes bytes) {
+    RustCallStatus status = { UNIFFI_CALL_STATUS_OK };
+    return {{ ci.ffi_rustbuffer_from_bytes().name() }}(
+      bytes,
+      &status
+    );
+  }
+
   static RustBuffer fromJs(jsi::Runtime &rt, std::shared_ptr<CallInvoker>,
                            const jsi::Value &value) {
     try {
-      auto bytes = uniffi_jsi::Bridging<ForeignBytes>::fromJs(rt, value);
+      auto buffer = uniffi_jsi::Bridging<jsi::ArrayBuffer>::value_to_arraybuffer(rt, value);
+      auto bytes = ForeignBytes{
+          .len = static_cast<int32_t>(buffer.length(rt)),
+          .data = buffer.data(rt),
+      };
+
       // This buffer is constructed from foreign bytes. Rust scaffolding copies
       // the bytes, to make the RustBuffer.
-      RustCallStatus status = { UNIFFI_CALL_STATUS_OK };
-      auto buf = {{ ci.ffi_rustbuffer_from_bytes().name() }}(
-        bytes,
-        &status
-      );
+      auto buf = rustbuffer_from_bytes(bytes);
       // Once it leaves this function, the buffer is immediately passed back
       // into Rust, where it's used to deserialize into the Rust versions of the
       // arguments. At that point, the copy is destroyed.
@@ -47,15 +64,10 @@ template <> struct Bridging<RustBuffer> {
 
     // Once we have a Javascript version, we no longer need the Rust version, so
     // we can call into Rust to tell it it's okay to free that memory.
-    RustCallStatus status = { UNIFFI_CALL_STATUS_OK };
-
-    {{ ci.ffi_rustbuffer_free().name() }}(
-        buf,
-        &status
-    );
+    rustbuffer_free(buf);
 
     // Finally, return the ArrayBuffer.
-    return jsi::Value(rt, arrayBuffer);
+    return uniffi_jsi::Bridging<jsi::ArrayBuffer>::arraybuffer_to_value(rt, arrayBuffer);;
   }
 };
 

diff --git a/crates/ubrn_bindgen/src/bindings/gen_typescript/oracle.rs b/crates/ubrn_bindgen/src/bindings/gen_typescript/oracle.rs
@@ -70,7 +70,7 @@ impl CodeOracle {
             FfiType::Float64 => "0.0".to_owned(),
             FfiType::Float32 => "0.0".to_owned(),
             FfiType::RustArcPtr(_) => "null".to_owned(),
-            FfiType::RustBuffer(_) => "/*empty*/ new ArrayBuffer(0)".to_owned(),
+            FfiType::RustBuffer(_) => "/*empty*/ new Uint8Array(0)".to_owned(),
             FfiType::Callback(_) => "null".to_owned(),
             FfiType::RustCallStatus => "uniffiCreateCallStatus()".to_owned(),
             _ => unimplemented!("ffi_default_value: {ffi_type:?}"),
@@ -107,7 +107,7 @@ impl CodeOracle {
     pub(crate) fn ffi_type_label_for_cpp(&self, ffi_type: &FfiType) -> String {
         match ffi_type {
             FfiType::RustArcPtr(_) => "UniffiRustArcPtr".to_string(),
-            FfiType::ForeignBytes => "ArrayBuffer".to_string(),
+            FfiType::ForeignBytes => "Uint8Array".to_string(),
             FfiType::RustBuffer(_) => "string".to_string(),
             _ => self.ffi_type_label(ffi_type),
         }
@@ -123,7 +123,7 @@ impl CodeOracle {
             FfiType::Float64 => "number".to_string(),
             FfiType::Handle => "bigint".to_string(),
             FfiType::RustArcPtr(_) => "bigint".to_string(),
-            FfiType::RustBuffer(_) => "ArrayBuffer".to_string(),
+            FfiType::RustBuffer(_) => "Uint8Array".to_string(),
             FfiType::RustCallStatus => "UniffiRustCallStatus".to_string(),
             FfiType::ForeignBytes => "ForeignBytes".to_string(),
             FfiType::Callback(name) => self.ffi_callback_name(name),

diff --git a/crates/ubrn_bindgen/src/bindings/gen_typescript/templates/CallbackInterfaceImpl.ts b/crates/ubrn_bindgen/src/bindings/gen_typescript/templates/CallbackInterfaceImpl.ts
@@ -1,5 +1,6 @@
 {{- self.import_infra_type("UniffiHandle", "handle-map") }}
 {{- self.import_infra_type("UniffiReferenceHolder", "callbacks") }}
+{{- self.import_infra_type("UniffiByteArray", "ffi-types")}}
 {{- self.import_infra("UniffiRustCaller", "rust-call")}}
 {{- self.import_infra_type("UniffiRustCallStatus", "rust-call")}}
 {{- self.import_infra("RustBuffer", "ffi-types")}}
@@ -83,7 +84,7 @@ const {{ trait_impl }}: { vtable: {{ vtable|ffi_type_name }}; register: () => vo
                     }
                 );
             };
-            const uniffiHandleError = (code: number, errorBuf: ArrayBuffer) => {
+            const uniffiHandleError = (code: number, errorBuf: UniffiByteArray) => {
                 uniffiFutureCallback(
                     uniffiCallbackData,
                     /* {{ meth.foreign_future_ffi_result_struct().name()|ffi_struct_name }} */{

diff --git a/crates/ubrn_bindgen/src/bindings/gen_typescript/templates/EnumTemplate.ts b/crates/ubrn_bindgen/src/bindings/gen_typescript/templates/EnumTemplate.ts
@@ -1,4 +1,4 @@
-{{- self.import_infra("AbstractFfiConverterArrayBuffer", "ffi-converters") -}}
+{{- self.import_infra("AbstractFfiConverterByteArray", "ffi-converters") -}}
 {{- self.import_infra("FfiConverterInt32", "ffi-converters") -}}
 {{- self.import_infra("UniffiInternalError", "errors") -}}
 
@@ -18,7 +18,7 @@ export enum {{ type_name }} {
 const {{ ffi_converter_name }} = (() => {
     const ordinalConverter = FfiConverterInt32;
     type TypeName = {{ type_name }};
-    class FFIConverter extends AbstractFfiConverterArrayBuffer<TypeName> {
+    class FFIConverter extends AbstractFfiConverterByteArray<TypeName> {
         read(from: RustBuffer): TypeName {
             switch (ordinalConverter.read(from)) {
                 {%- for variant in e.variants() %}

diff --git a/crates/ubrn_bindgen/src/bindings/gen_typescript/templates/ErrorTemplate.ts b/crates/ubrn_bindgen/src/bindings/gen_typescript/templates/ErrorTemplate.ts
@@ -63,8 +63,8 @@ export const {{ decl_type_name }} = (() => {
 const {{ ffi_converter_name }} = (() => {
     const intConverter = FfiConverterInt32;
     type TypeName = {{ type_name }};
-    {{- self.import_infra("AbstractFfiConverterArrayBuffer", "ffi-converters") }}
-    class FfiConverter extends AbstractFfiConverterArrayBuffer<TypeName> {
+    {{- self.import_infra("AbstractFfiConverterByteArray", "ffi-converters") }}
+    class FfiConverter extends AbstractFfiConverterByteArray<TypeName> {
         read(from: RustBuffer): TypeName {
             switch (intConverter.read(from)) {
             {%-   for variant in e.variants() %}

diff --git a/crates/ubrn_bindgen/src/bindings/gen_typescript/templates/RecordTemplate.ts b/crates/ubrn_bindgen/src/bindings/gen_typescript/templates/RecordTemplate.ts
@@ -50,8 +50,8 @@ export const {{ decl_type_name }} = (() => {
 
 const {{ ffi_converter_name }} = (() => {
     type TypeName = {{ type_name }};
-    {{- self.import_infra("AbstractFfiConverterArrayBuffer", "ffi-converters") }}
-    class FFIConverter extends AbstractFfiConverterArrayBuffer<TypeName> {
+    {{- self.import_infra("AbstractFfiConverterByteArray", "ffi-converters") }}
+    class FFIConverter extends AbstractFfiConverterByteArray<TypeName> {
         read(from: RustBuffer): TypeName {
             return {
             {%- for field in rec.fields() %}

diff --git a/crates/ubrn_bindgen/src/bindings/gen_typescript/templates/StringHelper.ts b/crates/ubrn_bindgen/src/bindings/gen_typescript/templates/StringHelper.ts
@@ -1,11 +1,12 @@
-{{- self.import_infra("UniffiInternalError", "errors") }}
 {{- self.import_infra("RustBuffer", "ffi-types") }}
+{{- self.import_infra_type("UniffiByteArray", "ffi-types") }}
 {{- self.import_infra("FfiConverterInt32", "ffi-converters") }}
 {{- self.import_infra_type("FfiConverter", "ffi-converters") }}
-const stringToArrayBuffer = (s: string): ArrayBuffer =>
+
+const stringToArrayBuffer = (s: string): UniffiByteArray =>
     uniffiCaller.rustCall((status) => {% call ts::fn_handle(ci.ffi_function_string_to_arraybuffer()) %}(s, status));
 
-const arrayBufferToString = (ab: ArrayBuffer): string =>
+const arrayBufferToString = (ab: UniffiByteArray): string =>
     uniffiCaller.rustCall((status) => {% call ts::fn_handle(ci.ffi_function_arraybuffer_to_string()) %}(ab, status));
 
 const stringByteLength = (s: string): number =>
@@ -14,20 +15,20 @@ const stringByteLength = (s: string): number =>
 const FfiConverterString = (() => {
     const lengthConverter = FfiConverterInt32;
     type TypeName = string;
-    class FFIConverter implements FfiConverter<ArrayBuffer, TypeName> {
-        lift(value: ArrayBuffer): TypeName {
+    class FFIConverter implements FfiConverter<UniffiByteArray, TypeName> {
+        lift(value: UniffiByteArray): TypeName {
             return arrayBufferToString(value);
         }
-        lower(value: TypeName): ArrayBuffer {
+        lower(value: TypeName): UniffiByteArray {
             return stringToArrayBuffer(value);
         }
         read(from: RustBuffer): TypeName {
             const length = lengthConverter.read(from);
             const bytes = from.readBytes(length);
-            return arrayBufferToString(bytes);
+            return arrayBufferToString(new Uint8Array(bytes));
         }
         write(value: TypeName, into: RustBuffer): void {
-            const buffer = stringToArrayBuffer(value);
+            const buffer = stringToArrayBuffer(value).buffer;
             const numBytes = buffer.byteLength;
             lengthConverter.write(numBytes, into);
             into.writeBytes(buffer);

diff --git a/crates/ubrn_bindgen/src/bindings/gen_typescript/templates/TaggedEnumTemplate.ts b/crates/ubrn_bindgen/src/bindings/gen_typescript/templates/TaggedEnumTemplate.ts
@@ -140,8 +140,8 @@ export const {{ decl_type_name }} = (() => {
 const {{ ffi_converter_name }} = (() => {
     const ordinalConverter = FfiConverterInt32;
     type TypeName = {{ type_name }};
-    {{- self.import_infra("AbstractFfiConverterArrayBuffer", "ffi-converters") }}
-    class FFIConverter extends AbstractFfiConverterArrayBuffer<TypeName> {
+    {{- self.import_infra("AbstractFfiConverterByteArray", "ffi-converters") }}
+    class FFIConverter extends AbstractFfiConverterByteArray<TypeName> {
         read(from: RustBuffer): TypeName {
             switch (ordinalConverter.read(from)) {
             {%- for variant in e.variants() %}

diff --git a/fixtures/callbacks-regression/tests/bindings/test_callbacks_regression.ts b/fixtures/callbacks-regression/tests/bindings/test_callbacks_regression.ts
@@ -44,7 +44,7 @@ async function testToMax(max: number, t: AsyncAsserts) {
 }
 
 (async () => {
-  for (let i = 1; i <= 1024; i *= 2) {
+  for (let i = 1; i <= 512; i *= 2) {
     await asyncTest(
       `Full tilt test up to ${i}`,
       async (t) => await testToMax(i, t),

diff --git a/typescript/src/async-callbacks.ts b/typescript/src/async-callbacks.ts
@@ -5,6 +5,7 @@
  */
 import { CALL_ERROR, CALL_UNEXPECTED_ERROR } from "./rust-call";
 import { type UniffiHandle, UniffiHandleMap } from "./handle-map";
+import { type UniffiByteArray } from "./ffi-types";
 
 // Some additional data we hold for each in-flight promise.
 type PromiseHelper = {
@@ -23,7 +24,7 @@ const UNIFFI_FOREIGN_FUTURE_HANDLE_MAP = new UniffiHandleMap<PromiseHelper>();
 
 // Some degenerate functions used for default arguments.
 const notExpectedError = (err: any) => false;
-function emptyLowerError<E>(e: E): ArrayBuffer {
+function emptyLowerError<E>(e: E): UniffiByteArray {
   throw new Error("Unreachable");
 }
 
@@ -38,8 +39,11 @@ export type UniffiForeignFuture = {
 export function uniffiTraitInterfaceCallAsync<T>(
   makeCall: (signal: AbortSignal) => Promise<T>,
   handleSuccess: (value: T) => void,
-  handleError: (callStatus: /*i8*/ number, errorBuffer: ArrayBuffer) => void,
-  lowerString: (str: string) => ArrayBuffer,
+  handleError: (
+    callStatus: /*i8*/ number,
+    errorBuffer: UniffiByteArray,
+  ) => void,
+  lowerString: (str: string) => UniffiByteArray,
 ): UniffiForeignFuture {
   return uniffiTraitInterfaceCallAsyncWithError(
     makeCall,
@@ -54,10 +58,13 @@ export function uniffiTraitInterfaceCallAsync<T>(
 export function uniffiTraitInterfaceCallAsyncWithError<T, E>(
   makeCall: (signal: AbortSignal) => Promise<T>,
   handleSuccess: (value: T) => void,
-  handleError: (callStatus: /*i8*/ number, errorBuffer: ArrayBuffer) => void,
+  handleError: (
+    callStatus: /*i8*/ number,
+    errorBuffer: UniffiByteArray,
+  ) => void,
   isErrorType: (error: any) => boolean,
-  lowerError: (error: E) => ArrayBuffer,
-  lowerString: (str: string) => ArrayBuffer,
+  lowerError: (error: E) => UniffiByteArray,
+  lowerString: (str: string) => UniffiByteArray,
 ): UniffiForeignFuture {
   const settledHolder: { settled: boolean } = { settled: false };
   const abortController = new AbortController();

diff --git a/typescript/src/async-rust-call.ts b/typescript/src/async-rust-call.ts
@@ -4,6 +4,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/
  */
 import { UniffiInternalError } from "./errors";
+import { type UniffiByteArray } from "./ffi-types";
 import { UniffiHandleMap, type UniffiHandle } from "./handle-map";
 import {
   type UniffiErrorHandler,
@@ -57,7 +58,7 @@ export async function uniffiRustCallAsync<F, T>(
   completeFunc: (rustFuture: bigint, status: UniffiRustCallStatus) => F,
   freeFunc: (rustFuture: bigint) => void,
   liftFunc: (lower: F) => T,
-  liftString: (arrayBuffer: ArrayBuffer) => string,
+  liftString: (bytes: UniffiByteArray) => string,
   asyncOpts?: { signal: AbortSignal },
   errorHandler?: UniffiErrorHandler,
 ): Promise<T> {