Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

📝 [Proposal]: Refactor Koa-Style Req and Res structs into interfaces #3347

Open
3 tasks done
grivera64 opened this issue Mar 10, 2025 · 3 comments
Open
3 tasks done

Comments

@grivera64
Copy link
Member

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 generate Ctx, Req, and Res interfaces for the same DefaultCtx struct
  • Updating c.Req() and c.Res() to return Ctx but 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 #2894 (comment)

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:

  • I agree to follow Fiber's Code of Conduct.
  • I have searched for existing issues that describe my proposal before opening this one.
  • I understand that a proposal that does not meet these guidelines may be closed without explanation.
@gaby gaby added this to v3 Mar 10, 2025
@gaby gaby moved this to Todo in v3 Mar 10, 2025
@gaby gaby added this to the v3 milestone Mar 10, 2025
@nickajacks1
Copy link
Member

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.

@grivera64
Copy link
Member Author

grivera64 commented Mar 16, 2025

@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.

@gaby
Copy link
Member

gaby commented Mar 16, 2025

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

3 participants