redbadger / crux

Cross-platform app development in Rust
https://redbadger.github.io/crux/
Apache License 2.0
1.7k stars 58 forks source link

Adding Compose capability caused typegen to fail #222

Closed wasnotrice closed 2 months ago

wasnotrice commented 5 months ago

I tried adding the Compose capability to my app. The shared crate compiles, but typegen fails in shared_types, because the Never enum has no cases. See the backtrace below for details.

We don't need to include Compose or Never in typegen, since they are not exposed to shells, but is there a way to mark a capability as "excluded"? Or maybe another workaround?

Here's a minimal repo that produces the error. You don't even have to use Compose, just include it in Capabilities.

❯ RUST_BACKTRACE=1 RUSTFLAGS=-Awarnings cargo build -p shared_types
   Compiling shared_types v0.1.0 (/Users/eric/code/crux_compose/shared_types)
error: failed to run custom build command for `shared_types v0.1.0 (/Users/eric/code/crux_compose/shared_types)`
note: To improve backtraces for build dependencies, set the CARGO_PROFILE_DEV_BUILD_OVERRIDE_DEBUG=true environment variable to enable debug information generation.

Caused by:
  process didn't exit successfully: `/Users/eric/code/crux_compose/target/debug/build/shared_types-5b2877aeefdefaf8/build-script-build` (exit status: 101)
  --- stdout
  cargo:rerun-if-changed=../shared

  --- stderr
  thread 'main' panicked at /Users/eric/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-reflection-0.3.6/src/de.rs:431:18:
  variant indexes must be a non-empty range 0..variants.len()
  stack backtrace:
     0: rust_begin_unwind
               at /rustc/7cf61ebde7b22796c69757901dd346d0fe70bd97/library/std/src/panicking.rs:647:5
     1: core::panicking::panic_fmt
               at /rustc/7cf61ebde7b22796c69757901dd346d0fe70bd97/library/core/src/panicking.rs:72:14
     2: core::panicking::panic_display
               at /rustc/7cf61ebde7b22796c69757901dd346d0fe70bd97/library/core/src/panicking.rs:196:5
     3: core::panicking::panic_str
               at /rustc/7cf61ebde7b22796c69757901dd346d0fe70bd97/library/core/src/panicking.rs:171:5
     4: core::option::expect_failed
               at /rustc/7cf61ebde7b22796c69757901dd346d0fe70bd97/library/core/src/option.rs:1988:5
     5: core::option::Option<T>::expect
     6: <serde_reflection::de::Deserializer as serde::de::Deserializer>::deserialize_enum::{{closure}}
     7: alloc::collections::btree::map::entry::Entry<K,V,A>::or_insert_with
     8: <serde_reflection::de::Deserializer as serde::de::Deserializer>::deserialize_enum
     9: crux_core::capability::_::<impl serde::de::Deserialize for crux_core::capability::Never>::deserialize
    10: serde_reflection::trace::Tracer::trace_type_once
    11: serde_reflection::trace::Tracer::trace_type
    12: serde_reflection::trace::Tracer::trace_simple_type
    13: crux_core::typegen::TypeGen::register_type
    14: <shared::app::Capabilities as crux_core::typegen::Export>::register_types
    15: crux_core::typegen::TypeGen::register_app
    16: build_script_build::main
    17: core::ops::function::FnOnce::call_once
  note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
charypar commented 2 months ago

Apologies for taking this long to respond.

It should be possible to work around this by marking the compose capability with #[effect(skip)], e.g.

#[derive(Effect)]
pub struct Capabilities {
    pub render: Render<Event>,
    #[effect(skip)]
    pub compose: Compose<Event>,
}

It may or may not be possible for us to detect this during type gen and skip it, but it's unclear to me whether with the current typegen we can only do it specifically for Compose or for any capability with Never as the operation type.

mmannes commented 2 months ago

Using #[effect(skip)] had no effect.

nvowell commented 2 months ago

I've been working around this by just adding a dummy case to the enum

pub enum Never { Never }

I'm not sure if this has any side effects, but it works fine for my purpose

wasnotrice commented 2 months ago

@mmannes thanks for the fix for #[effect(skip)] ❤️

atomrc commented 1 month ago

I saw that this was fixed with https://github.com/redbadger/crux/pull/259 (thanks @mmannes for that 🙏). Quick question: Do we have a timeline when this could be released?

I'm currently using a quite ugly workaround where I manually register all the types of the effects and exclude compose. I'd love to get rid of this piece of code :)

StuartHarris commented 1 month ago

Hey, sorry about this. We're cutting a release candidate at the moment, so will keep you up to date.