gotham
gotham copied to clipboard
RFC: remove StateData trait
Currently to be able to store something in State, the data needs to implement the StateData trait. This StateData trait is really just an alias for Any + Send. It would be really nice if implementing this trait would not be needed. Here are some reasons why:
- Not technically needed. This abstraction is not actually needed. As you can see in this branch, the
StateDatacan be removed and replaced with directAny + Sendwithout issues. There is thus no technical reason for the existence of this trait. - Unnecessary boilerplate. To be able to store a struct in
gotham::state::State, you need to either add theStateDataproc-macro to the declaration, or manually implementgotham::state::StateData. This is an extra line of code that is not needed. - Forces unnecessary wrapper structs. If you want to store a structure from a crate outside of yours in
State, you need to wrap it with of a wrapper struct just so you can implementStateData. This adds unnecessary bloat.
I propose that the StateData trait and StateData proc-macro are removed. All methods on State and FromState that currently take a StateData would take any Any + Send instead.
More benefits:
- This would also be one less thing that users need to learn about when starting out with Gotham.
- Removes ~150 LOC from gotham and examples.
I just looked through the history of gotham and it looks like StateData was introduced together with State, so it's not a leftover from a previous refactor. Does anyone know what the initial reason was to introduce that trait?
Considering #494, removing StateData might not be something we'd want to do. If a middleware sometimes puts type Foo into your state and sometimes doesn't, we could map Option<Foo> to State::try_take<Foo>. This would be impossible if Option<Foo> also implemented StateData (or, equivalently, we removed the trait).
This would be impossible if Option<Foo> also implemented StateData
How so? You could still make the opinionated assumption that Option is mapped to try_take. Possibly you could even force this in the type system (ie not let the user store Option in state).
Personally I think not being able to store non crate structures directly in the state is a far greater annoyance.
How so? You could still make the opinionated assumption that Option is mapped to try_take.
Well, impl<T: Any> FromState for T using take and impl<T: Any> FromState for Option<T> using try_take would clash because Option<T> also implements Any (but does not implement StateData).
I'll wait to see an actual implementation, maybe there's a clever trick that I didn't think of that makes this possible.