crux icon indicating copy to clipboard operation
crux copied to clipboard

Add `bridge` macro for apps using new typegen, other DX improvements, and new `counter-next` example

Open StuartHarris opened this issue 1 year ago • 7 comments

Adds a new bridge attribute macro to decorate the root App in a Crux application, which creates an FFI bridge (with associated scaffolding). This allows us to have a vanilla lib.rs without special boilerplate.

#[bridge]
pub struct MyApp;

Also adds a counter-next example, which has no shared_types crate, in order to see how DX is improved with the new typegen and bindings generation.

This PR should be safe to merge and shouldn't affect any existing Crux apps.

  • [x] add ffi command to crux_cli to generate bindings for Swift and Kotlin
  • [x] re-export crux_cli in crux_core under feature cli, so that users build a crate-local binary of the right version
  • [x] fix bug in typegen where TypeScript was being installed as a dependency, rather than a devDependency.
  • [x] unify flag names between gen and ffi commands
  • [x] introduce counter-next example without shared_types crate
  • [x] add crux_cli binary to shared
  • [x] add generated files to git, so we can review them as they evolve
  • [x] add script (for now) to do both foreign typegen and ffi bindgen
  • [x] switch to uniffi proc macros and remove udl file
  • [x] remove shells other than Android, iOS, web/TS and web/Rust
  • [x] update Android gradle files to use generated code
  • [x] update XCode build to use generated code
  • [x] ensure all shells work properly
  • [x] lock uniffi versions so that the version used by shared is the same as that used by crux_cli bin

StuartHarris avatar Apr 11 '25 15:04 StuartHarris

Judging by the time of appearance of the first commit, would it be wise to claim that this pr is likely to be merged soon™?

srfsh avatar Apr 11 '25 20:04 srfsh

@srfsh It probably will be merged soon, but not released just yet. There is some way to go before we would recommend it as part of the build process for a Crux app. There are some outstanding issues, a migration plan to think about and a fair amount of confidence building to do. — It would be wonderful if you wanted to try it out and feedback any issues you might find 😀

StuartHarris avatar Apr 14 '25 11:04 StuartHarris

I would really love to, however I've come to realize that crux, although definitely nice, does not suit my needs. I started using it a few days ago, and I was really liking it, however it was a bit too late for me to realize that the core part of my app does not have access to the network directly. I know that the shell could help me with simple http requests, but the crates I want to utilize do not work like that. But thanks for everything. Keep up the good work! I know someday I might find a usecase for crux, definitely something like a typst renderer could be created using compose and crux.

srfsh avatar Apr 14 '25 12:04 srfsh

Hey @srfsh you're right. I'm sorry that we're not quite there for your use case yet. There is work going on to introduce the concept of "middleware" that should enable what you're trying to achieve in the (not too distant future). Once we have something like this in place, you would be able to use Rust libraries that do their own I/O via the middleware (hybrid shell). This will allow Crux to remain pure and side-effect free, which is obviously a design goal. Until then the only options are to either use Rust libraries that adopt the Sans-IO approach, or manually handle the relevant effects in your shell by calling back in through separate FFI.

StuartHarris avatar Apr 14 '25 12:04 StuartHarris

That definitely looks promising! I will be keeping a close eye on this and the relevant PR. In the meantime, however, I must hold off for a while.

It is definitely nice to see the rust community trying its ways around the mobile gui. I think one of these projects (slint, iced, xilem...) will definitely have something usable in terms of aesthetics and DX (this is so horrible with flutter and jetpack compose). I am particularly in love with the elm design, and this project offers sort of the middle ground between all of the mist right now. Hope you all the best!

srfsh avatar Apr 14 '25 13:04 srfsh

I have a few patches applied locally that changes the suggestions I mentioned. I would like to upstream these if that's okay. There are a few additional things I would like to propose as well. I will list them here.

  • I think we can directly embed which crate to use for generation inside our crux_cli.rs. This way, you'd not need to specify --crate-name. Since it is already in the shared crate, I think it only makes sense and allows for better DX. I didn't test this (I will), but I am pretty sure it is possible using the CARGO_* variables.

This could work. We need to ensure the cli works if run independently from the shared lib (e.g. if installed with cargo install).

  • Since we got rid of the udl file for a better DX using macros, I think we can get rid of uniffi.toml that is used not much as well. I figured out a way using the BindgenCrateConfigSupplier trait . I would really love this to go in, but I can understand if this would be rejected as it hides this from the developer. I think it is better if this information is provided in Cargo.toml as a metadata.

What would be nice is to let the user choose how to specify the config, with levels of preference for where the values come from.

Tell me what you think. And thanks! I will be testing these with my changes.

P.S.: One might wonder why I had a change of heart all of a sudden. It is really simple: I figured I can leverage uniffi to fill in the gaps crux being pure. :)

❤️

StuartHarris avatar Apr 26 '25 11:04 StuartHarris

This could work. We need to ensure the cli works if run independently from the shared lib (e.g. if installed with cargo install).

Since we utilize the cli from our binary, we could choose to call a different function that takes in the crate name which we can get from the CARGO_* variables. I haven't inspected the functionality of crux_cli, but as I mentioned, we can call a different function exported by crux_cli. I can write the write-up code for this. This is relatively trivial.

What would be nice is to let the user choose how to specify the config, with levels of preference for where the values come from.

I was thinking we should utilize Cargo.toml for this. I personally see no need for an additional configuration file, but this is a strong opinion of mine that should be taken with care. Thus, I would like to propose two paths we can take:

  • With the potentiality of upstreaming the changes, we can implement our own BindgenCrateConfigSupplier trait, which would be read from the metadata of Cargo.toml for the options.
  • Adding an option for the configuration path of uniffi.toml. This is already supported by previous bindgen cli (before crux_cli, using uniffi directly).

I would like to reiterate if you had missed from the previous comments that I also think we shouldn't generate every bindings all at once. I don't need pnpm dependency as I'm not targeting the web (or let's just say TS/JS). The same is true for swift. My main purpose for using crux is for kotlin (android, compose) and rust (desktop, not decided). Some people will have a use case like this, and I think it is common enough that we should allow people to explicitly generate their bindings. One way I would like to propose to do this is to use Option<String> instead of String fields on the *_package fields of codegen. The default value seems somewhat useless. I think people would like to change those already. So if they change (meaning they provide explicitly), we can infer that they want these bindings to be generated. I have it done like this in my local fork, and I would like to see this merged as well.

srfsh avatar Apr 26 '25 13:04 srfsh

  • I think we can directly embed which crate to use for generation inside our crux_cli.rs. This way, you'd not need to specify --crate-name. Since it is already in the shared crate, I think it only makes sense and allows for better DX. I didn't test this (I will), but I am pretty sure it is possible using the CARGO_* variables.

@srfsh Thanks for this suggestion. The --crate-name arg can now be read from CRATE_NAME env var, so the calling code can look like this:

    env::set_var("CRATE_NAME", env!("CARGO_PKG_NAME"));
    crux_core::run_cli()

StuartHarris avatar May 22 '25 12:05 StuartHarris

This PR is quite big now. I'll investigate the uniffi.toml config in another PR. This should be good to review now.

StuartHarris avatar May 23 '25 09:05 StuartHarris

This PR is quite big now. I'll investigate the uniffi.toml config in another PR. This should be good to review now. I am currently studying for my finals, but this is what I had if it is any good:

use std::{
	env::consts::{DLL_PREFIX, DLL_SUFFIX},
	process::Command,
};

use crate::args::BindgenArgs;
use anyhow::{Context as _, Result};

use cargo_metadata::MetadataCommand;
use uniffi_bindgen::{
	bindings::{KotlinBindingGenerator, SwiftBindingGenerator},
	library_mode, BindgenCrateConfigSupplier,
};

struct MyConfigSupplier {
	java_package: Option<String>,
	swift_package: Option<String>,
}

impl BindgenCrateConfigSupplier for MyConfigSupplier {
	fn get_toml(
		&self,
		_crate_name: &str,
	) -> Result<Option<toml::value::Table>> {
		let mut bindings = toml::value::Table::new();
		if let Some(ref pkg) = self.java_package {
			let mut kotlin = toml::value::Table::new();
			kotlin.insert("package_name".into(), pkg.as_str().into());
			bindings.insert("kotlin".into(), kotlin.into());
		}
		if let Some(ref pkg) = self.swift_package {
			let mut swift = toml::value::Table::new();
			swift.insert("package_name".into(), pkg.as_str().into());
			bindings.insert("swift".into(), swift.into());
		}

		let mut config = toml::value::Table::new();
		config.insert("bindings".into(), bindings.into());

		Ok(Some(config.try_into()?))
	}
}

pub fn bindgen(args: &BindgenArgs) -> Result<()> {
	let meta = MetadataCommand::new()
		.no_deps()
		.exec()
		.context("error running cargo metadata")?;
	let pkg = meta
		.packages
		.iter()
		.find(|x| x.name == args.crate_name)
		.context("provided crate not found")?;
	let lib = pkg
		.targets
		.iter()
		.find(|x| x.is_lib())
		.context("no library found; can this ever happen?")?;

	let library_path = meta
		.target_directory
		.join("debug") // TODO: should this be hardcoded?
		.join(format!("{DLL_PREFIX}{}{DLL_SUFFIX}", lib.name));

	let config_supplier = MyConfigSupplier {
		java_package: args.java_package.clone(),
		swift_package: args.swift_package.clone(),
	};

	let status = Command::new("cargo")
		.args(["build", "--package", &args.crate_name])
		.status()?;
	assert!(status.success());

	if args.java_package.is_some() {
		library_mode::generate_bindings(
			&library_path,
			None,
			&KotlinBindingGenerator,
			&config_supplier,
			None,
			&args.out_dir.join("java"),
			true,
		)?;
	}

	if args.swift_package.is_some() {
		library_mode::generate_bindings(
			&library_path,
			None,
			&SwiftBindingGenerator,
			&config_supplier,
			None,
			&args.out_dir.join("swift"),
			true,
		)?;
	}

	Ok(())
}

srfsh avatar Jun 07 '25 10:06 srfsh