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]: Allow removing registered route #3098

Open
3 tasks done
rebaz94 opened this issue Aug 8, 2024 · 7 comments
Open
3 tasks done

📝 [Proposal]: Allow removing registered route #3098

rebaz94 opened this issue Aug 8, 2024 · 7 comments

Comments

@rebaz94
Copy link

rebaz94 commented Aug 8, 2024

Feature Proposal Description

Following the addition of RebuildTree method #2769, which enabled dynamic request handling, I propose enhancing this functionality by allowing the removal of registered routes.

Currently, while we can dynamically add routes, the ability to remove them is limited. When adding a route with the same path, instead of updating the existing handler, a new one is added, which prevents the updated handler from being invoked. To address this issue, and to provide a way to remove routes that are no longer needed, we require a method to completely remove the route.

Alignment with Express API

N/A

HTTP RFC Standards Compliance

N/A

API Stability

The proposed feature will involve the addition of a single method, RemoveRoute, which will be used to unregister specific routes based on the path and HTTP methods. This function will maintain the stability of the API by ensuring that the removal process is straightforward and does not interfere with other routes or the overall routing structure.

func (app *App) RemoveRoute(path string, methods ...string) {
    // TODO:
}
 

Feature Examples

func addMyRoute() {
    // Remove route if it already exists
    s.app.RemoveRoute("/hello", fiber.MethodHead, fiber.MethodGet)

    // Add new route
    s.app.Get("/hello", func(c *fiber.Ctx) error {
        return c.SendString("hello")
    })

    // Rebuild the routing tree to reflect changes
    s.app.RebuildTree()
}

s.app.Get("/define", func(c *fiber.Ctx) error {
    before := s.app.HandlersCount()
    addMyRoute()
    after := s.app.HandlersCount()

    return c.JSON(map[string]any{
        "hcBefore": before,
        "hcAfter":  after,
    })
})

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
Copy link
Member

gaby commented Aug 8, 2024

@rebaz94 Rebuilding the tree by itself is not thread-safe already, adding this functionality would make it even worse.

@rebaz94
Copy link
Author

rebaz94 commented Aug 8, 2024

Hi @gaby, of course but it's important to note that production applications typically don't define routes at runtime. This approach is safe during development when you're iterating on your application. In my case I'm building a no-code solution where I need to add, update or remove routes while the server is running during development but for the production environment, I will generate the routes statically.

@gaby
Copy link
Member

gaby commented Aug 8, 2024

@rebaz94 The trade-off is huge. We ran benchmarks and it requires adding a Mutex in several parts which would make Fiber way slower across the board. Especially when calling Next().

@rebaz94
Copy link
Author

rebaz94 commented Aug 8, 2024

Sure. but I don't think adding RemoveRoute would cause any issues during development. we just need to remove route info and call RebuildTree.

@rebaz94
Copy link
Author

rebaz94 commented Aug 8, 2024

@ReneWerner87, is there any chance this feature could be included in v3?

@gaby
Copy link
Member

gaby commented Aug 8, 2024

@rebaz94 From me, I'd say yes, but we may need to add a "Development" page to the docs 😂

@luk3skyw4lker
Copy link
Contributor

This probably needs to be analyzed thoroughly, and also it's good to notice that the RebuildTree method is extremely intense and slow for use. If the feature is approved, I can check out the code and see if it's not going to break anything major.

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

No branches or pull requests

3 participants