-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Feat: Register custom methods #2107
Feat: Register custom methods #2107
Conversation
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
There should be 2 ways to allow custom methods:
Maybe the first one would be overdesign. Current implementation seems OK but i'll review it then. What do you think @ReneWerner87 |
sorry very busy week, will answer tomorrow |
current implementation seems to be okay
feature will be in the next release |
@rafimuhammad01 pls check the last comments |
@ReneWerner87 please check the last comment |
will do this today |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
after the last adjustments i will merge it
@ReneWerner87 Already push the latest changes, please kindly check it. Thank you! |
don´t forget this @rafimuhammad01 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
through the test run i found out that there is one more problem
https://github.com/gofiber/fiber/actions/runs/3198073219/jobs/5222134276
because if you add a custom method and run the USE or ALL before, the new method will not be considered there, because it is not inital in the list of those to be considered
should we maybe not approach the feature so dynamically and add the methods on-the-fly and rather maintain a slice inside the fiber.config with the allowed methods ?
so the intMethod slice as config property with the 9 default methods?
then you could also reduce or extend the default if you don't want TRACE or CONNECT to work or LOAD to be added
but then you would have to consider the mounting, because these methods are set per fiber app and would have to be merged then
@efectn @rafimuhammad01 what do you think ?
Sounds good idea |
@rafimuhammad01 okay for you ? should i do this or you want todo this ? |
Hi @ReneWerner87 sorry for the late reply, I've been busy this week. I still didn't understand about the other approach we will implement, could you please elaborate more? Thanks |
the problem func main() {
app := fiber.New(fiber.Config{})
app.Use(func(ctx *fiber.Ctx) error { // is always called only not at LOAD
// doing something
return ctx.Next()
})
app.Add("LOAD", "/test", func(ctx *fiber.Ctx) error {
return ctx.SendString("hello")
})
log.Fatal(app.Listen(":3000"))
} the solution func main() {
app := fiber.New(fiber.Config{
RequestMethods: []string{ // default is the normal list
fiber.MethodGet,
fiber.MethodHead,
fiber.MethodPost,
fiber.MethodPut,
fiber.MethodDelete,
fiber.MethodConnect,
fiber.MethodOptions,
fiber.MethodTrace,
fiber.MethodPatch,
"LOAD",
},
// with this approach we can reduce the list and only allow what we want
})
app.Use(func(ctx *fiber.Ctx) error { // is always called
// doing something
return ctx.Next()
})
app.Add("LOAD", "/test", func(ctx *fiber.Ctx) error {
return ctx.SendString("hello")
})
log.Fatal(app.Listen(":3000"))
} |
I'm sorry for the late response, have been super busy right now. I see the problem and will try to fix this issue. Thank you! |
|
Congrats on merging your first pull request! 🎉 We here at Fiber are proud of you! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
Description
This PR is to support the custom method feature. I updated register function so that function won't be giving validation if the method is unsupported yet, so user can add any method that they want to. I also make changes to methodInt function to return the index instead of using hardcoded with switch-case statement.
Fixes #2096
Type of change
Please delete options that are not relevant.
Checklist:
Commit formatting:
Use emojis on commit messages so it provides an easy way of identifying the purpose or intention of a commit. Check out the emoji cheatsheet here: https://gitmoji.carloscuesta.me/