feat(dart_frog): allow access of shelf.Message.context

[sun-jiao]

Status

READY

Description

Allow access of shelf.Message.context, which is used by some shelf middleware and shelf handlers.

closes #773:

Handler middleware(Handler handler) {
  return (context) {
    final request = context.request;
    print((request.shelfContext['shelf_router/params'] as Map<String, String>?)?['test']); // no longer needs to use `fromShelfHandler` and `toShelfHandler`.
    return handler(context);
  };
}

Type of Change

• ✨ New feature (non-breaking change which adds functionality)
• 🛠️ Bug fix (non-breaking change which fixes an issue)
• ❌ Breaking change (fix or feature that would cause existing functionality to change)
• 🧹 Code refactor
• ✅ Build configuration change
• 📝 Documentation
• 🗑️ Chore

[tomarra]

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!

[sun-jiao]

@tomarra Testcases are already added.

[sun-jiao]

@tomarra any updates?

[wolfenrain]

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 onRequest method.

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

[sun-jiao]

@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 request.context. This is the library: https://github.com/sun-jiao/session_shelf/tree/dart_frog.

[sun-jiao]

@wolfenrain Hi, any updates?

[sun-jiao]

Hello, any updates? Or is there any more elegant way to implement the same function without exposing shelf context? @wolfenrain

Sessions are put in shelf.Context in my code: https://github.com/sun-jiao/session_shelf/blob/dart_frog/lib/middleware/cookies_middleware.dart#L67C22-L67C34

In fact, I just did a search before opening this PR to confirm if there was a duplicate, and found issue #773 occasionally.

[wolfenrain]

Hello, any updates? Or is there any more elegant way to implement the same function without exposing shelf context? @wolfenrain

Sessions are put in shelf.Context in my code: sun-jiao/session_shelf@dart_frog/lib/middleware/cookies_middleware.dart#L67C22-L67C34

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 @visibleForTesting so you could use it without us promoting for normal use too much

[sun-jiao]

@wolfenrain Thanks.

[alestiago]

@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 👀