-
Notifications
You must be signed in to change notification settings - Fork 10
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
Use the C-ABI to call into Rust crates for WASM! #202
Conversation
f0120b4
to
0e2d115
Compare
0e2d115
to
98861db
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self review.
let runtime_alias_ident = self.runtime_ident(); | ||
let runtime_ident = self.flavor.runtime_module(); | ||
let namespace_ident = ident(ci.namespace()); | ||
let module_ident = self.module_ident(); | ||
let library_ident = ident(crate_.library_name()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting the CrateMetadata
from cli.rs
has been leading up to this line.
@@ -88,6 +88,31 @@ impl CrateMetadata { | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This refactor used to be relevant to this PR, but now we're using ci.crate_name()
it is not.
It is still a useful refactor, but we're in a high velocity phase of development, so would rather not spend time picking it out into a separate PR.
WDYT? should I leave it here, or break it out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My opinion, leave it in.
I've generally found that trying to separate out changes after the fact leads to more problems than not, but ultimately your call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sorry, realized this was a dangling PR I had finished but not hit submit on. |
It's time for a release. If your change hasn't landed before this release, don't worry, releases are cheap! We can do another one! # 0.28.3-2 ## ✨ What's New * Add `--profile` build argument ([#192](#192)) * Thank you [@Johennes](https://github.com/Johennes)! ## 🦊 What's Changed * Adjust template to allow for hot reload via metro of running apps ([#207](#207)). * Stabilise `require.resolve` by looking up `package.json` instead of entrypoint ([#200](#200)). * Thank you [@hassankhan](https://github.com/hassankhan)! * Split compat job by platform and version ([#211](#211)). * This shows on the README.md if builder-bob or React Native has changed breaking the tutorial. * Thank you [@Johennes](https://github.com/Johennes)! * Fixed GC'ing objects with callbacks intermittent crasher ([#208](#208) and [#209](#209)) * Reproducibly pick the same library file when using `--and-generate` ([#194](#194)) * Thank you [@Johennes](https://github.com/Johennes)! ## 🌏🕸️ WASM! * Fixtures `coverall`, `custom-types-example`, `enum-types`, `trait-methods` ([#202](#202)). * Switched from passing `ArrayBuffer`s to using `Uint8Array`, to accommodate WASM better. ([#187](#187)) Callbacks now have UniffiResult to communicate between typescript and C++ ([#205](#205)). * Fixtures `coverall2` and `rondpoint` ([#191](#191)). * Fixture `arithmetic` ([#188](#188)). ## 📰 Documentation * Remove duplicate parentheses ([#203](#203)). * Minor typo fixes in GC docs ([#204](#204)). * Remove reference to name field in the ubrn.config.yaml docs ([#189](#189)). **Full Changelog**: 0.28.3-1...0.28.3-2
This PR started off as getting errors working.
This, turned out, fairly straightforward. However, showing that the fixed worked by enabling a test fixture turned out to be. less straightforward.
This led to a re-write of the rust generation. In this PR, we switch from using a Rust-to-Rust calls which happen to be exposed by uniffi as
extern "C"
to relying on the ABI and only the ABI.This means we do not have to rely on Rust visibility rules, meaning that a large number of fixtures now Just Work™, without needing to wait for uniffi-rs to release
v0.29.x
.We don't need to keep track of the module paths, so I am hoping that multi-module crates should be straightforward. The only reason why the
ext-types
fixture is not enabled is because theuniffi_one.udl
has a callback interface.I don't think this approach is compatible with a some of the optimizations outlined in mozilla/uniffi-rs#2331, but I think this is a good approach for 0.28.