-
Notifications
You must be signed in to change notification settings - Fork 152
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
feat(dart_frog): allow access of shelf.Message.context
#1262
base: main
Are you sure you want to change the base?
Conversation
shelf.Message.context
shelf.Message.context
Hi @sun-jiao 👋 thanks for opening this PR. It looks like there is an error in the CI pipeline due to missing tests. Can you please take a look and get this passing? Then I can work with the team to get feedback/reviews. Thanks in advance! |
@tomarra Testcases are already added. |
@tomarra any updates? |
Hi thanks for the PR! I had a quick look and I think we should not expose the shelf context like this. Dart Frog tries to stay agnostic to it's underlying framework where it can and exposing this context would counter that. Based on the example you gave I presume you wanted to have access to the route params in the middleware. I think we should rather focus on that use-case than exposing all of the context as a map. We could potentially expose those parameters in the the same way that we do with the Handler middleware(Handler handler) {
return (context, String routeParam1, String routeParam2) {
// ... code ...
return handler(context);
}
} This would require us to tweak how we build the routing pipeline but it should be doable and would bring the API more inline with the rest of Dart Frog. Any thoughts on that? cc: @felangel |
@wolfenrain No, currently I am not concerned with the router's middleware parameters. I'm developing a library (in fact, based on another shelf library) which provides session persistence for shelf. And I want to add support for frog. While it saves the session in shelf's |
@wolfenrain Hi, any updates? |
Hello, any updates? Or is there any more elegant way to implement the same function without exposing shelf context? @wolfenrain Sessions are put in In fact, I just did a search before opening this PR to confirm if there was a duplicate, and found issue #773 occasionally. |
Hi! Sorry for the late reply I'll be looking into what you linked later this week, preferably we would not want to expose that kind of shell stuff so I'll have a look at your use-case to see if we can come up with a different approach, otherwise we can always expose it as a |
@wolfenrain Thanks. |
@sun-jiao we're also have a Dart Plugin System on the works that would probably help you improve the developer experience for the users of your tool that rely on Dart Frog. Keep an eye on the next updates 👀 |
Status
READY
Description
Allow access of
shelf.Message.context
, which is used by some shelf middleware and shelf handlers.closes #773:
Type of Change