gofiber
📝 [Proposal]: Refactor Koa-Style `Req` and `Res` structs into interfaces
Feature Proposal Description
This issue proposes to refactor the Req and Res structs into views/interfaces onto the Ctx interface. This should improve DX when adding new Ctx methods, as we currently have to manually write new function code in both ctx.go and either req.go or res.go.
This update would consist of:
- Using
ifacemaker(if possible) to generateCtx,Req, andResinterfaces for the sameDefaultCtxstruct - Updating
c.Req()andc.Res()to returnCtxbut type-casted to the appropriate interface
After this is done, Req and Res methods should then call the underlying Ctx method, since the Ctx interface implements both the Req and Res interfaces.
Originally posted by @grivera64 in https://github.com/gofiber/fiber/issues/2894#issuecomment-2664328217
Alignment with Express API
This improvement shouldn't deviate from Fiber's alignment with the Express API, but rather updates the internal representation of the Req and Res struct into interfaces, which is part of the Express API.
HTTP RFC Standards Compliance
N/A
API Stability
Fiber's API will have improved stability, since all methods in Ctx will be automatically be used into the respective Req and Res interfaces. This will make it so that we don't accidentally forget to update the Req/Res implementations after making a change to the core Ctx methods.
Feature Examples
N/A
Checklist:
- [x] I agree to follow Fiber's Code of Conduct.
- [x] I have searched for existing issues that describe my proposal before opening this one.
- [x] I understand that a proposal that does not meet these guidelines may be closed without explanation.
I was thinking that if we can't do what we want with ifacemaker, we could write a tiny generator that looks at the doc comments of each DefaultCtx method to check for a specific tag or string, like Request method. or @req or something, and based on that add it to either Req or Res.
I think something as simple as
func (c *DefaultCtx) Req() Req {
return c
}
...has the advantage of simplicity, guaranteeing identical behavior between API surfaces, and eliminating the need for "duplicate" tests.
@nickajacks1 Currently @gaby and I are trying to push an upstream bugfix to ifacemaker in vburenin/ifacemaker Issue #69 and vburenin/ifacemaker PR #72. If we can get this merged and have a new release for ifacemaker, we should be able to use ifacemaker for this task.
I already did a proof-of-concept, where we split all Req() methods for *DefaultCtx in req.go and all Res() methods for *DefaultCtx in res.go. This just requires modifying the existing //go:gen ifacemaker ... comment to search the right files and then add two more to generate Res and Req as interfaces based on *DefaultCtx. This would then make it so that you only have to write new methods in req.go and res.go and have it update Ctx, Req, and Res interfaces.
@nickajacks1 ifacemaker should have the fixes this week. The tool had not been updated in 2 years. The owner gave me push access to help update/fix stuff.
To update this issue with discussion on the GoFiber discord, we discussed that currently the DefaultRes struct depends on an adapter version of the method Get() to map to Ctx.GetRespHeader() res.go.
If we were to remove the Get() mapping, and simply map directly to Resp.GetRespHeader(), this feature is currently doable. From a behind-the-scenes discussion on discord, we found that Resp.Get() is preferable to keep.
To allow this, I proposed a new ifacemaker feature to support including promoted methods. With this new feature, we can then simply have a ResDefault struct that embeds the DefaultCtx struct and only define the single Get() method that wraps GetRespHeader(), and leave the rest to be auto-inferred form the embedded struct's method definitions.
This should be the final requirement for this feature so that we can still use the shorthand Resp.Get() method, while getting all of the benefits of avoid duplicate code in ctx.go and req.go/res.go.