Skip to content

Do we need IxaEvent? If so, do we need the proc macro to derive it? #933

Open

Description

@RobertJacobsonCDC

This issue catalogs weirdness related to IxaEvent and asks whether / how we can resolve the weirdness.

Background

The IxaEvent trait is for event types that client code can subscribe to with an event handler:

pub trait IxaEvent {
    fn on_subscribe(_context: &mut Context) {}
}

Notice the default trivial implementation for the single trait method on_subscribe. Thus, client code can implement this trait with a single line:

struct MyStruct {}

impl IxaEvent for MyStruct {}

The public event API on Context is just two methods:

Context::subscribe_to_event<E: IxaEvent + Copy + 'static>(
    &mut self,
    handler: impl Fn(&mut Context, E) + 'static,
);
Context::emit_event<E: IxaEvent + Copy + 'static>(&mut self, event: E);

Notice that in both cases we have the trait constraints E: IxaEvent + Copy + 'static.

Questions and Issues

Do we even need IxaEvent at all? Do we need its single method on_subscribe?

The only known uses of IxaEvent are the internal types EntityCreatedEvent<E: Entity> and PropertyChangeEvent<E: Entity, P: Property<E>>. Neither use the on_subscribe method.

Options:

  • Replace IxaEvent with the naked trait constraints Copy + Any (which implies Clone + 'static), removing the on_subscribe feature.
  • Remove IxaEvent as a publicly implementable trait entirely, and just support PropertyChangeEvent and EntityCreatedEvent. (Probably not what we want.)
  • Keep the current implementation, but add user-guide documentation explaining use case.

Does IxaEvent need a proc macro?

Since the single trait method on IxaEvent has a default implementation, client code can implement it with a single line:

struct MyStruct {}

impl IxaEvent for MyStruct {}

Maybe this is not a burden on client code to require they do this. We could eliminate the proc macro, which would ultimately eliminate ixa-derive altogether (if/when we remove Property::CanonicalValue machinery for multi-properties).

Interaction between generics and derived traits

Consider a type like this:

#[derive(Copy, Clone, IxaEvent)]
struct MyStruct<T>(PhantomData<T>) {}

context.emit_event(MyStruct::<String>(PhantomData::default())); // Error: Trait constraint not satisfied!

One of Rust's annoyances is, it type checks and thus checks trait constraints before monomorphization, and the derive only emits an impl for Copy and Clone during monomorphization. Thus, the compiler can't "see" the derived trait implementations when its type checking the call to emit_event. The fix is to manually provide impls that are generic over T:

impl<T> Copy for MyStruct<T> {}

impl<T> Clone for MyStruct<T> {
    fn clone(&self) -> Self {
        *self
    }
}

But you don't need to do this unless the type has generics. This is confusing for client code. This isn't specific to IxaEvent, but it tends to come up in the context of trying to implement an IxaEvent, because the API needs Copy + 'static.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions