wip: experiment around a hybrid (nacre) shell #333

o0Ignition0o · 2025-03-22T10:24:55Z

Super early draft, a lot of things to figure out together and document.

Coauthored by @regexident.

I hope I can draft a rough explainer of the various components soon ™️

TLDR:

One can register a callback crux must call to generate effects out of band (not within the request/response model)
The effect macro generates a NacreEffect implementation, depending on the sort of effect (notify_shell, request_from_shell, stream_from_shell)
Nacre is some sort of middleware that plugs between the core and the bridge, that has the ability to drive streams to completion.
NacreBridge deals with forwarding the effects via the callback
Probably more but i forgot, I ll try to draw a schema sometimes.

Next things I need to do:

turn the UDL based definition to a macro to please uniffi with the callback / trait registration
rebump dependencies
see what ideas are good and what ideas aren't
???
profit

o0Ignition0o · 2025-03-22T10:37:17Z

examples/simple_counter/shared/Cargo.toml

@@ -15,11 +15,17 @@ typegen = ["crux_core/typegen"]
 crux_core.workspace = true
 serde = { workspace = true, features = ["derive"] }
 lazy_static = "1.5.0"
-uniffi = "0.29.0"
+uniffi = "=0.28.3"


hope i can rebump these very soon

What problem caused you to revert the version?

I had an error with the way the shell / callback is registered in the .udl
I think I need to rely on the uniffi macros rather on the .udl file in order to rebump the dependency :/

mhedgpeth

I love the direction this request takes this project. I'm not a maintainer but in hopes of moving the conversation forward added my comments. Thanks so much for doing this work.

mhedgpeth · 2025-04-03T10:25:48Z

crux_core/src/bridge/mod.rs

@@ -27,13 +38,43 @@ where
 }
 // ANCHOR_END: request

+pub type RequestFfi<A> = Request<<<A as App>::Effect as Effect>::Ffi>;
+
+pub trait NacreTrait<A: App>


I had to look up this word Nacre - google tells me it means "mother of pearl". I believe you're trying to express here the "rust side effects processor" here. At the very least it could be explained in a comment. Could you tell me more about what the intent/meaning is here?

@mhedgpeth heh, while @o0Ignition0o I were sketching up the initial design for this PR we found we needed some name to refer to "the thing" we were working on ("let's do the name bike-shedding later!") and we sort of ended up at "the nacre thingy", which I had initially come up as a silly pun.

It ended up just sticking around for far longer than anticipated. I still think it's a cute name, but a more understandable "middleware"-ish one would probably be a better choice overall, for obvious reasons.
The reason we didn't just go with "middleware" was that those usually allow for multiple layers, while "nacre" is (in its current design) limited to just one layer, right between core and shell. Hence nacre, the inside of a shell.

Right, Middleware will do just fine, AFAICT Nacre refers to the "white side of an egg shell" as in it s the inner side if you re the shell, but it s the outer side if you're the core ^^

Middleware is more popular and may make more sense once the design settles anyway!

mhedgpeth · 2025-04-03T10:26:54Z

crux_core/src/bridge/mod.rs

        Self {
-            inner: BridgeWithSerializer::new(core),
+            inner: Arc::new(BridgeWithSerializer::new(core)),


After reading this through the first time I still didn't understand the need for reference counting here. What motivated this inclusion?

This might actually be a left-over of a previous iteration and no longer be necessary. Can't say for sure from the top of my head. 🤔 cc @o0Ignition0o

So I think I had some send + sync invariants to uphold with the callback.

I'll need to revisit that once the middleware api is set, especially since the core doesn't need it but the bridge might in the example. I'll double check that!

mhedgpeth · 2025-04-03T10:27:45Z

crux_core/src/bridge/mod.rs

+                    effects.into_iter().map(std::convert::Into::into),
+                    &mut <dyn erased_serde::Serializer>::erase(&mut ser),
+                )
+                .unwrap();


Could this be a try_from to avoid the unwrap?

Definitely :D

mhedgpeth · 2025-04-03T10:28:52Z

crux_core/src/bridge/mod.rs

            }
            Some(id) => {
-                self.registry.resume(id, data)?;
+                self.registry.resume(id, data).expect(


Could this be translated into an error?

mhedgpeth · 2025-04-03T10:36:43Z

crux_macros/src/effect.rs

+                    let name_and_variant = quote! { #effect_name::#variant };
+
+                    from_effect_to_shell_effect_impl.push(quote! { #effect_name::#variant(request) => panic!("capability {} is not implemented in the shell", stringify!(#name_and_variant)) });
+                    match_arms.push(quote! { #effect_name::#variant(request) => panic!("capability {} is not implemented in the shell", stringify!(#name_and_variant)) });


Reading through this, the panic! makes me nervous but this is only for a situation where the developer doesn't have something set up fundamentally properly, has to make that change once, then it will always work. I'm honestly (as not a maintainer) not quite sure what the panic policy is on this project, this one seems reasonable.

Right,

Afaict there are other panics in this macro, but maybe we can shift left and fail at compile time if this branch still exists by the time we build the app 🤔 not sure.

mhedgpeth · 2025-04-03T10:38:46Z

examples/simple_counter/shared/Cargo.toml

@@ -15,11 +15,17 @@ typegen = ["crux_core/typegen"]
 crux_core.workspace = true
 serde = { workspace = true, features = ["derive"] }
 lazy_static = "1.5.0"
-uniffi = "0.29.0"
+uniffi = "=0.28.3"


What problem caused you to revert the version?

mhedgpeth · 2025-04-03T10:42:17Z

examples/simple_counter/shared/src/app.rs

@@ -1,4 +1,6 @@
 // ANCHOR: app
+use super::capabilities::delay::Delay;


I wonder if we should fork this example, change simple_counter to be the minimum it needs to be to support this new callback (or do people need to do that?) and then add another example that shows this, since it is an advanced feature.

Definitely, the example grew a bit because I started with an example to have an idea of what the DX should be. I'll revert most of the things and probably create a different example with a delay and an interval.

wip: experiment around a hybrid (nacre) shell

bdd134a

o0Ignition0o marked this pull request as draft March 22, 2025 10:25

o0Ignition0o commented Mar 22, 2025

View reviewed changes

rebump most dependencies except uniffi for now

23498d6

mhedgpeth reviewed Apr 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wip: experiment around a hybrid (nacre) shell #333

wip: experiment around a hybrid (nacre) shell #333

o0Ignition0o commented Mar 22, 2025 •

edited

Loading

o0Ignition0o Mar 22, 2025

mhedgpeth Apr 3, 2025

o0Ignition0o Apr 3, 2025

mhedgpeth left a comment

mhedgpeth Apr 3, 2025

regexident Apr 3, 2025 •

edited

Loading

o0Ignition0o Apr 3, 2025

mhedgpeth Apr 3, 2025

regexident Apr 3, 2025

o0Ignition0o Apr 3, 2025

mhedgpeth Apr 3, 2025

o0Ignition0o Apr 3, 2025

mhedgpeth Apr 3, 2025

mhedgpeth Apr 3, 2025

o0Ignition0o Apr 3, 2025

mhedgpeth Apr 3, 2025

mhedgpeth Apr 3, 2025

o0Ignition0o Apr 3, 2025

		@@ -1,4 +1,6 @@
		// ANCHOR: app
		use super::capabilities::delay::Delay;

wip: experiment around a hybrid (nacre) shell #333

Are you sure you want to change the base?

wip: experiment around a hybrid (nacre) shell #333

Conversation

o0Ignition0o commented Mar 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhedgpeth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

regexident Apr 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

o0Ignition0o commented Mar 22, 2025 •

edited

Loading

regexident Apr 3, 2025 •

edited

Loading