feat: removeAllRoutes public method

[robertsLando]

This method allows to clear routes stack. As discussed here: expressjs/express#4296

[dougwilson]

Thank you for this! I added a few labels, namely that it needs documentation and tests. But addition, it seems like it will likely need some information from you on what the use-case is for this method, clearly defined so we have help discuss on any additional changes that need to perhaps be made or not. For example, would a use expect that calling this would reset the registered parameters? Why or why not? Having the full use-case laid out in either the top of this PR or an additional comment will help us understand (1) what, if anything, else needs to change in the module to support this and (2) how we can help users in the future with on going questions around this feature.

Thank you again and I'm looking forward to the additional information to continue discussion/work on this feature proposal.

[robertsLando]

In my case, I have some plugins that a user can add to the running server, those plugins can be customized and can also add apis (routes) to the server. When a plugin is refreshed I need to remove all eisting routes for that plugin and add back the new updated routes.

would a use expect that calling this would reset the registered parameters

I don't know what you mean as I didn't checked all the source code. By consider my use case do you think that is necessary? Please let me know if I need to add something or feel free to edit my pr.

I added a test, let me know if it's good for you

[dougwilson]

Based on your use case description, I'm not sure, but it sounds like your plugin should be returning a new router each time it is "refreshed", which would make this addition not necessary, right? What does this addition add or make possible vs returning a new router fresh for each refresh?

For example, would a user who removes all the routes expect that the router kept the prior strict routing behavior? What if the user wanted to remove all the routes in order to add new routes with a different strict behavior? I guess the same could be asked about the case-sensitivity feature. Those switches are of course aspects of how the routes are interpreted, same as for the params.

For example, say you have a router with some routes like /user/:id. You get a reference to that router and want to remove all the routes, so call this new method. You then add new ones like /obj/:id and of course call .param to define what the :id will look like... but it's different than that user one and now conflicts with the previous definition.

So the above scenario may be confusing for how one would use this new method as it's currently (as of the comment) implemented, as there are still aspects of the previous routes still lingering on the router even after calling this method. I of course understand this method is called "removeAllRoutes", but without docs in this PR it's still hard for me to understand what that is expected to do.

Of course, most importantly of all the questions, is the first couple in the comment if you can get to them. Many times having multiple ways to do the same thing leads to user confusion, but also if there is already a way to do this and it is more configurable/capable, it seems adding a partial feature may not be the best long-term route to take, but that's what I'm trying to figure out 😂 .

I think your use-case outline above is leaving a lot to be desired, as I think you are accidentally excluding aspects that is important for us to know here. For example, you say "plugins" but there is no such thing in this module or Express, so I really don't know what that means exactly, which makes following your use-case quite difficult.

[dougwilson]

Tests should only be asserting against the public-visible behavior/API. I assume that by adding this method that you are saying that router.stack is private and not public. This test using .stack to make an assertion would seemingly re-enforce the idea that it is public (as changing the name of the property would break this test -- the indicator it's not private). If it is public, then I'm missing the purpose of this function, as router.stack = [] would already be clearly possible for a user to do, is that right?

[robertsLando]

Yes I could do that already, but having it in the router as a function makes it more "safe" against updates that could break that

[dougwilson]

but having it in the router as a function makes it more "safe" against updates that could break that

How so? Can you ellaborate on that?

[jonchurch]

Can you provide us a sample app that would benefit from this feature? Maybe a comparison w/ how things are now, and how you would like them to work with the inclusion of this feature.

This is an interesting feature, but it also feels a lot like state management which I'm wary of. The route stack is of course already a type of application state, and Express is very unopinionated so if you want to manage routes as state that's up to the user. But something feels off about providing an API that encourages users to treat the route stack like state that can be wiped with a single function call.

[robertsLando]

Based on your use case description, I'm not sure, but it sounds like your plugin should be returning a new router each time it is "refreshed", which would make this addition not necessary, right? What does this addition add or make possible vs returning a new router fresh for each refresh?

This allows me to keep using the same router instance without creating another one or use the trick you mentioned here

So the above scenario may be confusing for how one would use this new method as it's currently (as of the comment) implemented, as there are still aspects of the previous routes still lingering on the router even after calling this method. I of course understand this method is called "removeAllRoutes", but without docs in this PR it's still hard for me to understand what that is expected to do.

I never used the param so I need to check what it is used but if that is the behaviour of course also that should be 'cleared' someway giving you a clean router instance

I think you are accidentally excluding aspects that is important for us to know here.

That is sure as I don't know ROuter as you and I didn't checked all the source code 😄

For example, you say "plugins" but there is no such thing in this module or Express, so I really don't know what that means exactly, which makes following your use-case quite difficult.

Yes in that case plugins is something to identify a 'program' that is added on runtime to my webserver, in poor words that is a plugin for me. I pass the router as a parameter in this "plugin" constructor and the plugin can use that instance to add new routes. But If I reload a plugin some routes could persist without clearing old ones.

[dougwilson]

This allows me to keep using the same router instance without creating another one or use the trick you mentioned here

So I think your view of it as "trick" is not correct -- it it no trick at all, just code to implement what you want. There is nothing wrong with it.

I think you are accidentally excluding aspects that is important for us to know here.

That is sure as I don't know ROuter as you and I didn't checked all the source code 😄

Oh, I think I may have miss-spoke here. I'm referring to excluding aspects of your use-case us, and important for us to know about your use-case. We understand you don't know the router code, and that is what we want to help with, but are still unclear on your use-case.

I think what @jonchurch said in #93 (comment) would be helpful for sure 👍

[robertsLando]

This is an interesting feature, but it also feels a lot like state management which I'm wary of. The route stack is of course already a type of application state, and Express is very unopinionated so if you want to manage routes as state that's up to the user. But something feels off about providing an API that encourages users to treat the route stack like state that can be wiped with a single function call.

Keep it like a proposal guys, I saw some questions around stackoverflow and plugins like https://www.npmjs.com/package/express-remove-route that would like someting to manage routes. In my case this is enought but could open a new way of features around routes management (add/remove specific ones for example)

[wesleytodd]

I think it sounds like this was agreed to be better done with a new router instance. So I am going to close this. If you want to open a new issue to discuss alternatives please do. One alternative I think worth discussing is showing folks how to easily use a new router to achieve this behavior.