dart_frog icon indicating copy to clipboard operation
dart_frog copied to clipboard

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

Open sun-jiao opened this issue 1 year ago โ€ข 10 comments

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

  • [x] โœจ 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

sun-jiao avatar Jan 28 '24 14:01 sun-jiao

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 avatar Jan 30 '24 14:01 tomarra

@tomarra Testcases are already added.

sun-jiao avatar Jan 30 '24 15:01 sun-jiao

@tomarra any updates?

sun-jiao avatar Feb 02 '24 03:02 sun-jiao

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

wolfenrain avatar Feb 02 '24 15:02 wolfenrain

@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 avatar Feb 03 '24 03:02 sun-jiao

@wolfenrain Hi, any updates?

sun-jiao avatar Feb 13 '24 01:02 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.

sun-jiao avatar Mar 10 '24 08:03 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: 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

wolfenrain avatar Mar 19 '24 13:03 wolfenrain

@wolfenrain Thanks.

sun-jiao avatar Mar 20 '24 06:03 sun-jiao

@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 ๐Ÿ‘€

alestiago avatar Aug 13 '24 12:08 alestiago