feat!: Switch to Axum's FromRef for custom state - #250
Merged
Conversation
spencewenski
force-pushed
the
gh-249-custom-state-from-ref
branch
from
June 29, 2024 09:36
3c29326 to
4529d65
Compare
Codecov ReportAttention: Patch coverage is
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
spencewenski
force-pushed
the
gh-249-custom-state-from-ref
branch
2 times, most recently
from
June 29, 2024 09:48
ce150d5 to
46abba9
Compare
spencewenski
force-pushed
the
gh-249-custom-state-from-ref
branch
2 times, most recently
from
June 29, 2024 10:29
97ffd4d to
1dc5fbc
Compare
FromRef to allow custom state
spencewenski
force-pushed
the
gh-249-custom-state-from-ref
branch
2 times, most recently
from
June 29, 2024 18:51
e5b0714 to
7b5d176
Compare
Our previous approach to allowing consumers to provide custom state/context was to embed it as a field in Roadster's `AppContext`. This is not how Axum recommends libraries support custom state, which makes it difficult (impossible?) to integrate Roadster with other Axum libraries. The recommended approach is to keep the state generic, and add a type constraint to require Roadster's state to be able to be retrieved from the custom state using the [FromRef](https://docs.rs/axum/latest/axum/extract/derive.FromRef.html) trait. See the following for more details: https://docs.rs/axum/latest/axum/extract/struct.State.html#for-library-authors One example of an integration that was blocked by our non-recommended approach: Leptos requires the app's state provide Leptos's state using `FromRef`. In order to support Leptos with our previous approach, we would have needed to add the Leptos state directly to Roadster's `AppContext`. This adds a dependency on Roadster for consumers who may want to use Leptos, and also adds a direct dependency between Roadster and Leptos (though, it would be optional behind a feature flag). This maybe could work in the short term, but this approach doesn't scale to other libraries -- Roadster would need to follow a similar approach for all other libraries consumers may want to use. Instead, it's better to simply follow Axum's recommendation to provide maximum flexibility to consumers.
spencewenski
force-pushed
the
gh-249-custom-state-from-ref
branch
from
June 30, 2024 00:37
7b5d176 to
5513855
Compare
FromRef to allow custom stateFromRef to allow custom state
FromRef to allow custom stateFromRef for custom state
spencewenski
marked this pull request as ready for review
June 30, 2024 00:41
This was referenced Jun 30, 2024
Closed
Merged
spencewenski
pushed a commit
that referenced
this pull request
Jul 1, 2024
## 🤖 New release * `roadster`: 0.3.5 -> 0.4.0 <details><summary><i><b>Changelog</b></i></summary><p> <blockquote> ## [0.4.0](roadster-v0.3.5...roadster-v0.4.0) - 2024-07-01 ### Added - [**breaking**] Implement health check API using `HealthCheck` trait ([#255](#255)) - [**breaking**] Switch to Axum's `FromRef` for custom state ([#250](#250)) ### Other - [**breaking**] Remove deprecated items in preparation of 0.4 release ([#253](#253)) - Add example for integrating with Leptos ([#252](#252)) - Use small number of sidekiq workers for `full` example in dev/test ([#251](#251)) </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Our previous approach to allowing consumers to provide custom
state/context was to embed it as a field in Roadster's
AppContext.This is not how Axum recommends libraries support custom state, which
makes it difficult (impossible?) to integrate Roadster with other
Axum libraries.
The recommended approach is to keep the state generic, and add a type
constraint to require Roadster's state to be able to be retrieved from
the custom state using the
FromRef
trait. See the following for more details: https://docs.rs/axum/latest/axum/extract/struct.State.html#for-library-authors
One example of an integration that was blocked by our non-recommended
approach: Leptos requires the app's state provide Leptos's state using
FromRef. In order to support Leptos with our previous approach, wewould have needed to add the Leptos state directly to Roadster's
AppContext. This adds a dependency on Roadster for consumers who maywant to use Leptos, and also adds a direct dependency between Roadster
and Leptos (though, it would be optional behind a feature flag). This
maybe could work in the short term, but this approach doesn't scale to
other libraries -- Roadster would need to follow a similar approach for
all other libraries consumers may want to use. Instead, it's better to
simply follow Axum's recommendation to provide maximum flexibility to
consumers.