Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make AppContext extensible via traits #777

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mstallmo
Copy link

Closes #522

This PR changes the Hooks trait to be generic over the AppContextTrait allowing end users to provide their own structs for application context. This allows for greater flexibility of end users to define the context of their web server as well as makes accessing user defined global application state smoother in various parts of the loco framework.

This change is implemented via two traits. The first being the object-safe Context trait and the second being the full AppContextTrait. The reason behind the split is that tasks and initializers are required to be object-safe and to pass in a generic type, that trait also needs to be object-safe. There are other places where the generic type is required to be Clone and/or Default making it impossible to implement in one trait. So where object-safety is required the Context trait is used and everywhere else AppContextTrait is used.

Splitting the implementation into the Context trait and AppContextTrait also allows for the definition of a create method so the internals of the framework can construct the generic type with minimal disruption to the current code.

mstallmo and others added 6 commits September 21, 2024 00:32
Transition from using the `AppContext` struct internally to using
`AppContextTrait` to represent shared global state.

This allows end users to implement and extend their own shared
application state beyond what loco provides out of the box.

The `AppContextTrait` is implemented for `AppContext` so `AppContext`
can be used directly by users that don't have a need to extend what is
already provided by the context.
@jondot
Copy link
Contributor

jondot commented Sep 25, 2024

@mstallmo this looks like a really interesting approach, thanks!
I activated CI, we have breaking changes.

I'm wondering how we can give users a pleasant path with all the breaking changes, as there are quite a few (considering AppContext is such a central subject).

@mstallmo
Copy link
Author

@jondot thanks for looking at my PR! I'll go ahead and make the changes to pass CI.

I'm wondering how we can give users a pleasant path with all the breaking changes, as there are quite a few (considering AppContext is such a central subject).

Providing an easy path for users to upgrade has also crossed my mind. As a first step I would be happy to write up a section in the guide and a dedicated blog post detailing how to upgrade an existing app to this new way of implementing AppContext. From my experience with making the updates to the demo app the locations where users would have to make changes should all be pretty much the same and mostly consist of adding type annotations where needed.

Because the existing AppContext struct implements the new context traits anywhere that AppContext is used explicitly by users should upgrade without any changes on their part.


Another option would be looking into writing a CLI tool or task to make the updates on the user's behalf. I don't have any prior experience in writing a tool like this but would be happy to work on one and take any suggestions the community might have on this topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extensibility of AppContext
2 participants