This RFC is an addendum to the [original](https://github.com/StarArawn/kayak_ui/pull/56) RFC 1. The reason for this one is to cover some of the changes that RFC 1 went through during implementation that (until now) has not been documented. Additionally, I wanted to discuss a new way of adding widget contexts based on some of the discoveries made thus far.
> **Note:** This won't cover *every* change— just the ones I believe are noteworthy.
## What Changed?
### Trait Objects & Widget
One of the difficulties with the addition of `WidgetProps`, was how we would handle trait objects. Originally, the plan
was to have widgets stored as `Box<dyn Widget<Props=dyn WidgetProps>>`. This allowed us to keep things generic while still keeping the trait object. To our surprise, this compiled no problem! So we went with it. But it had a hidden
Attempting to _actually use_ this object resulted in a compilation error. Lesson learned: when testing, test more than
just a single use-case.
So how did we go about fixing this? By adding a new trait!
The idea comes from yew's [component source](https://github.com/yewstack/yew/blob/master/packages/yew/src/html/component/mod.rs), but essentially we introduce a new trait, `BaseWidget`, which is mostly just a copy of the `Widget` trait:
Another small change from RFC 1 is the required naming system. It was thought props would need to use a name in the form of `<Widget Name>Props` in order for us to actually create a widget's props in the `rsx!` macro (remember, we don't have great access to type information when processing macros).
As it turns out, though, we don't need this requirement due to the changes `Widget` mentioned earlier. Instead, we can
// let mut props = <MyWidget as kayak_ui::core::Widget>::Props::default();
```
From there, we just assign the props individually, line by line. Easy!
#### Common Prop Types
All common props now have more consistent types. Namely, `Children` has been converted to a proper struct rather than a type alias. It also no longer wraps `Option` but is instead wrapped *by*`Option` so as to be more like the other common props.
Additionally, both `OnEvent` and `Children` now implement `Debug`, `Clone`, and `PartialEq` (the latter two essentially
do nothing). This means we no longer need to use the [derivative](https://crates.io/crates/derivative) crate to ignore those fields, thus reducing the verbosity and hidden gotchas of creating a prop struct.
For reference, this is what we used to have to do:
5. Removed the need for the [derivative](https://crates.io/crates/derivative) crate
## Widget Context
With [#68](https://github.com/StarArawn/kayak_ui/pull/68) and [#69](https://github.com/StarArawn/kayak_ui/pull/69) just merged, the only big change from RFC 1 left is the widget context.
The RFC suggested that this be generated by the `#[widget]` macro. The reason for this was so that we could store a
direct reference to the widget on the generated struct:
```rust
structMyWidgetContext<'a>{
context:&'amutKayakContextRef,
widget:&'amutMyWidget,
}
```
However, RFC 1.1 suggests a different way of doing this.
Instead of generating a struct for every widget, we can specify a generic `WidgetContext` struct. This struct would look
more like:
```rust
structWidgetContext<'a,TProps:WidgetProps>{
context:&'amutKayakContextRef,
props:&'amutTProps,
id:Index,
}
```
But why? What benefits do we gain by doing this instead of the original?
Well for one, it reduces the amount of code we actually need to generate. But beyond that it significantly reduces the *
magic* 🪄 involved in defining a widget. The idea was just to replace the hidden `KayakContextRef` with a hidden widget context— not great.
By using `WidgetContext` our functional widget actually works more like a regular function now:
```rust
fnMyWidget(context:WidgetContext<MyWidgetProps>){
// ...
}
```
No more hidden `context`. No more hidden `self`. What's available to the user is defined by *them* very plainly.
#### Prop Access
This also makes prop access much more intuitive. By calling something like `context.props()` we can return an immutable reference to the widget's props. This fixes two minor issues in the current system.
The first is implicit cloning. Right now the `rsx!` macro inserts a clone on the props. However, this may be a costly
operation for some prop values and could simply be avoided by just letting the user clone what they need.
The second problem this solves is the (in my opinion) anti-pattern of mutating props. Props should be purely input data. It doesn't make sense that we can change or mutate this input when it's just going to change on the next render.
This feels wrong:
```rust
fnMyWidget(props:MyWidgetProps){
letmy_styles=Some(Style{/* ... */});
props.style=my_styles;
// ...
}
```
Instead, it makes more sense to do this:
```rust
fnMyWidget(context:WidgetContext<MyWidgetProps>){
letmy_styles=Some(Style{/* ... */});
context.set_styles(my_styles);
// ...
}
```
This might help avoid confusion wherein users think they can change other non-common props without realizing that those changes will not stick across renders.
And note that either way works. In fact, `WidgetContext::set_styles(...)` would internally just do something like in the first code snippet. That's just the nature of how we store styles, event handlers, etc. However, the important thing is
that it doesn't leave users thinking they can or should mutate props.
> This is all leans more on the subjective side, so perhaps I'm wrong here and we should just allow calling `WidgetContext::props_mut()`.
#### Naming Requirement
The last bit of hidden code we have left (at least one of the more important ones) is the naming of `context`. Ideally,
a user could call this whatever they want (such as `ctx`, `this`, or `self`). But we need it to be called `context` when
we get to the `rsx!` macro. This also means we can't shadow the `context` identifier within the function body.
Unfortunately, there's no easy way of getting around this. The best we *could* do is create a variable that no one would
ever collide with (unintentionally at least):
```rust
letkayak_ui_widget_context=context;
// ...
```
However, this won't work due to `context` being a mutable reference.
So for now, the naming requirement remains (and should be enforced by the `#[widget]` macro).