search

jhugman / uniffi-bindgen-react-native

A uniffi bindings generator for calling Rust from react-native
https://jhugman.github.io/uniffi-bindgen-react-native/
Other
50 stars 5 forks source link

Allow turbo-modules with names with @my-org/project-name #127

Closed jhugman closed 1 month ago

jhugman commented 1 month ago

Fixes #124

According to 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.

zzorba commented 1 month ago

I'll test out this build on my side @Johennes to see if I get similar results. I don't have a namespaced package like this, but I'm guessing the crash you are seeing is unrelated to that?

Johennes commented 1 month ago

I'll test out this build on my side @Johennes to see if I get similar results. I don't have a namespaced package like this, but I'm guessing the crash you are seeing is unrelated to that?

Possibly though I'm not entirely sure since the organisation ends up in some generated names but not others. I'll try to also give it some more testing tomorrow.

zzorba commented 1 month ago

One thing I noticed here, it seems like it went from src/index.ts to src/index.tsx, so I had to update my package.json slightly.

Johennes commented 1 month ago

Yeah, I noticed that, too, but already had index without any extension in package.json so didn't have to change anything. I think I'll try running the app from Android Studio later today to see that gives me an actual error message or stack trace.

jhugman commented 1 month ago

I'll test out this build on my side @Johennes to see if I get similar results. I don't have a namespaced package like this, but I'm guessing the crash you are seeing is unrelated to that?

Possibly though I'm not entirely sure since the organisation ends up in some generated names but not others. I'll try to also give it some more testing tomorrow.

I found it!

The Android crash was due to the library name in System.loadLibrary not matching the one built in CMakeLists.txt.

I think I broke it in #123, but might have broken it in this one.

I've changed this and added a test.

jhugman commented 1 month ago

One thing I noticed here, it seems like it went from src/index.ts to src/index.tsx, so I had to update my package.json slightly.

Our new tests check for differences between the generated files. Builder bob was generating an index.tsx file, and ubrn was generating an index.ts file.

zzorba commented 1 month ago

Confirmed the fix is working!