search

sanic-org / sanic

Accelerate your web app development | Build fast. Run fast.
https://sanic.dev
MIT License
18.11k stars 1.55k forks source link

Allows classes as middleware #2923

Closed nicolaipre closed 7 months ago

nicolaipre commented 8 months ago

Tiny fix that seemingly solves #2912

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 87.997%. Comparing base (acb29c9) to head (f95e33e).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2923 +/- ## ============================================= - Coverage 88.039% 87.997% -0.042% ============================================= Files 94 94 Lines 7433 7432 -1 Branches 1283 1283 ============================================= - Hits 6544 6540 -4 - Misses 622 624 +2 - Partials 267 268 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nicolaipre commented 7 months ago

😎 Thanks Can you add a unit test?

Will do. Currently working on it but experienced some weird errors that I am not quite sure how to solve.

Some of the MiddlewareMixin Overloads seem to cause some confusion. My tests work when running locally, but fails with pytest for some reason.

I could push the test as it is right now if you would be willing to look at it to see if you are able to spot any obvious mistakes?

nicolaipre commented 7 months ago

I have decided to close this PR as the change would introduce confusion since decorators are not intended to be used on Classes anyways.

The main goal was to allow users to register routes and middleware without using decorators as an alternative way of using Sanic, but this fix crashes with the way Sanic is intended to be used and is therefore no point adding in.