feat(humafiber): add Fiber v3 support#962
feat(humafiber): add Fiber v3 support#962wheinze wants to merge 11 commits intodanielgtaylor:mainfrom
Conversation
wolveix
left a comment
wolveix
left a comment
There was a problem hiding this comment.
This is awesome man, thanks for spending the time to work on this!
I just did a super quick initial review. Please forgive me ignorance around Fiber itself. I'll happily dive in a little more when I have the time :)
|
|
||
| type router interface { | ||
| Add(method, path string, handlers ...fiber.Handler) fiber.Router | ||
| Add(methods []string, path string, handler any, handlers ...any) fiber.Router |
There was a problem hiding this comment.
This seems a little odd. Why are there two handler parameters? Why are neither of them strict types anymore?
There was a problem hiding this comment.
This seems a little odd. Why are there two handler parameters? Why are neither of them strict types anymore?
This has been done to align with the changed signature in Fiber v3: https://github.com/gofiber/fiber/blob/v3.0.0/router.go#L35
|
|
||
| type requestTester interface { | ||
| Test(*http.Request, ...int) (*http.Response, error) | ||
| Test(*http.Request, ...fiber.TestConfig) (*http.Response, error) |
There was a problem hiding this comment.
Why the switch from int to fiber.TestConfig?
There was a problem hiding this comment.
Why the switch from
inttofiber.TestConfig?
There's been an improvement to not only support a timeout (by passing an int here), but rather passing a fiber.TestConfig in Fiber v3: https://docs.gofiber.io/next/whats_new/#test-config
@wolveix Thanks for your contributions to this project 👍 I'm totally happy that I got that first feedback that fast ;) Did address your feedback. Let me know, if there's anything else that needs clarification or adjustments =) |
|
Fixed the golangci-lint issue: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #962 +/- ##
=======================================
Coverage 93.11% 93.11%
=======================================
Files 23 23
Lines 4766 4766
=======================================
Hits 4438 4438
Misses 269 269
Partials 59 59 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I expect https://github.com/danielgtaylor/huma/actions/runs/22092253987/job/63897228707?pr=962 failed while compiling the test files with Go 1.24, which is not supported as mentioned in the PR description. Shall I remove it from the test matrix? |
Filed a PR onto my fork repo to verify this: wheinze#1 I totally get the idea of supporting as much Go versions as possible and would also give this high prio, but it seems, at least to me, that the frameworks (Fiber and Echo) are pushing it so we don't really have an option. Also the Go team itself only supports the latest two major versions: https://go.dev/doc/devel/release#policy |
|
@wheinze completely agreed, and that's the policy for most libraries, including Huma! I just figured I'd do a 1.24 release before 1.25 :) I'll bump deps shortly! |
Nice, thanks! |
2 participants
humafiberadapter to use Fiber v3 as the default (New/NewWithGroup/Unwrap)NewV2/NewV2WithGroup/UnwrapV2functions in a separate filegofiber/fiber/v3, etc.)Based on the approach in 9637414. Preferred to use dedicated files for v2 in order to simplify removal of v2 support at a later point in time.
Note on Go version and dependency bumps
I noticed that there's a preference to keep dependency and Go version changes out of feature PRs (as noted in #959). In this case the Go version bump to 1.25 is required because Fiber v3 declares
go 1.25.0in its owngo.mod, so the module won't resolve without it. The dependency bumps are transitive from that.That said, I'm very open to suggestions if there's a better way to handle this!
Breaking Changes
humafiber.New()andhumafiber.NewWithGroup()now requiregithub.com/gofiber/fiber/v3humafiber.Unwrap()now returnsfiber.Ctx(interface) instead of*fiber.Ctx(pointer)humafiber.NewV2()/humafiber.UnwrapV2()Fixes #920