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

[question] Routes management #4296

Closed
robertsLando opened this issue May 27, 2020 · 11 comments
Closed

[question] Routes management #4296

robertsLando opened this issue May 27, 2020 · 11 comments
Labels

Comments

@robertsLando
Copy link

Hi everyone. I have a server that supports plugins that can be added on runtime, those plugins can add new routes to the server, the problem is that at the end of my routes I have an error handler like:

app.use(function (req, res, next) {
    var err = new Error('Not Found');
    err.status = 404;
    next(err);
  });

  // error handler
  app.use(function (err, req, res, next) {
    // set locals, only providing error in development
    res.locals.message = err.message;
    res.locals.error = req.app.get('env') === 'development' ? err : {};

    logger.warn(`${err.status || 500} - ${err.message} - ${req.originalUrl} - ${req.method} - ${req.ip}`);

    // render the error page
    res.status(err.status || 500);
    res.render('error.ejs', {
      status: err.status || 500
    });
  });

When I init the plugin the route is added but never called as it has been inited after this error handlers, so how could I manage this?

@dougwilson
Copy link
Contributor

Express router is an append-only system, which calls the routes in the order they are added. If you want to have a point at which you can append new routes above some other routes, use a sub router. For example, create a sub router (express.Router()) and app.use it before your error handler, at the point you want the plugin to append the routes. Then have the plugin append to that sub router instance instead of top level router (app).

@robertsLando
Copy link
Author

@dougwilson This doesn't really answer my question that at this point could become a feature request. Could it be an interesting feature to add? The ability to manage routes order and add/remove routes from the list?

@dougwilson
Copy link
Contributor

Hi @robertsLando , Express does not limit you to it's included router and you can use any router you like, including ones that have the feature you like. I believe what I provided did answer your question with the information you provided. If it did not, I'm very sorry. Likely I need more information about what you are trying to do in order to understand your question and help. Generally, the issue tracker is not really the place for question on "how to do X with Express", so we try to close them out. You can drop by our Gitter or try Stackoverflow where we and the greater community can help on general how to do X type of questions.

@robertsLando
Copy link
Author

I said it doesn't answer as I'm looking for a way to add routes on runtime (so after the error handlers are setted up), I cannot add them before error routes as I may not know them on startup.

I think that this could be an interesting feature request so:

The ability to manage routes order and add/remove routes from the list

Actually I saw there are users that are archiving this by manually handling express routes object but it's not something provided by express so could be break easily by express updates.

@dougwilson
Copy link
Contributor

dougwilson commented May 27, 2020

Right, but that was the answer I gave: how to add them on runtime (so after the error handlers are setted up), after everything was set up.

@dougwilson
Copy link
Contributor

Here is a full example, if it helps to illustrate what I said in words, but in code:

var app = express()
var router = express.Router()

app.use(... stuff)
app.use(router)
app.use(... error handlers)

// later in your plugin
router.get('/foo', (req, res, next) => ...)

@robertsLando
Copy link
Author

Sorry @dougwilson, my fault, I have misunderstood your answer. In fact that should work! Thanks :)

@robertsLando
Copy link
Author

robertsLando commented May 27, 2020

There is no way to "unuse" that router right? If I would like to remove all routes of that router from express I mean

something like app.unuse(pluginsRouter)

@dougwilson
Copy link
Contributor

So there are usually two answers for that. The typical desire when folks want to unuse a router is usually because they want to replace it (so maybe this applies to you), in which case the current pattern is to call it via a local reference:

var router = express.Router()
// ...
app.use((req, res, next) => router(req, res, next))
// ...
// changing the "router" variable to a different instance will "swap it"
// can also use a property on an object to allow changing from other files

Completely unusing it at runtime because the idea is that you're going to add it once, then later remove it, never to add it back ever again, is typically rare, but still possible. The typical example is the same as above, but adding a if:

app.use((req, res, next) => router ? router(req, res, next) : next())

Of course, as to your point, folks are welcome, if they really desire it, to add features to the router via a pull request. The router source code for Express it at https://github.com/pillarjs/router . I mention this because you mentioned above a feature request, but history shows us a feature request just never gets done, as those who work on the project work on what they want/need, and so it would already exist if those who already worked on it wanted/needed it, so to make something happen takes a pull request from the outside contributor vs just making a feature request in an issue and waiting years and years :)

@robertsLando
Copy link
Author

@dougwilson Really thanks for your support and very quick answers to my problems (and sorry again if I misunderstood your first answer making you lose time)

I work in many open source problems and in such cases I would love to help if it can help others so if I find some time to make it I will submit a pr :)

I think that a method like router.flushAllRoutes() could be an interesting starting point

robertsLando added a commit to robertsLando/router that referenced this issue May 27, 2020
This method allows to clear routes stack. As discussed here: expressjs/express#4296
@robertsLando
Copy link
Author

@dougwilson Just made a proposal change for this feature, I don't know if it is that easy or I'm missing something, let me know :)

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

No branches or pull requests

2 participants