Skip to content

Refactor Chain function to use Middleware type#1090

Open
72sevenzy2 wants to merge 5 commits intogo-chi:masterfrom
72sevenzy2:master
Open

Refactor Chain function to use Middleware type#1090
72sevenzy2 wants to merge 5 commits intogo-chi:masterfrom
72sevenzy2:master

Conversation

@72sevenzy2
Copy link
Copy Markdown

Add middleware type to slightly enhance readability.

@JRaspass
Copy link
Copy Markdown
Contributor

I actually had a similar change locally and was considering PRing it, one fairly important different is I made it a type alias rather than a wholly different type so existing code would keep working, I also defined it just before the Middlewares type in chi.go as that seemed a more logical place:

+// Middlware is a type alias to standard middleware handlers.
+type Middleware = func(http.Handler) http.Handler
+
 // Middlewares type is a slice of standard middleware handlers with methods
 // to compose middleware chains and http.Handler's.
-type Middlewares []func(http.Handler) http.Handler
+type Middlewares []Middleware

After that there's a few more places that can be updated to use it like the Router interface, WalkFunc, etc. Just search and replace!

Not sure if such a change is wanted but it definitely gets my vote for being more readable, so long as it's just a type alias and therefore won't require explicit conversion from existing callers!

@72sevenzy2
Copy link
Copy Markdown
Author

Thats actually smart, I never thought about that, thanks for giving me a heads up.

@VojtechVitek
Copy link
Copy Markdown
Contributor

I was always hoping for Middleware type to be defined in net/http stdlib package :)

I don't think chi is the right place to reinvent this, though.

@72sevenzy2
Copy link
Copy Markdown
Author

Oh I see.

Added type alias for Middleware and Middlewares.
Removed type alias for Middleware and Middlewares.
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.

3 participants