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

feat: Add match method #4215

Closed
wants to merge 1 commit into from
Closed

Conversation

getspooky
Copy link
Contributor

Sometimes you may need to register a route that responds to multiple HTTP verbs.
You may do so using the match method.
example :
app.match(['get','post'], '/' ,function(req,res) { console.log(req.method); });

@dougwilson
Copy link
Contributor

dougwilson commented Mar 14, 2020

Hello, and thanks for your pull request! We actually have a nice API for doing this registration: The Route object 😃 :

app.route('/')
  .get(function(req,res) { console.log(req.method); })
  .post(function(req,res) { console.log(req.method); })

You can also type your method only once by using a function reference:

function handleRequest (req,res) { console.log(req.method); }

app.route('/')
  .get(handleRequest)
  .post(handleRequest)

For more information, see our route documentation: http://expressjs.com/en/api.html#app.route

@dougwilson dougwilson closed this Mar 14, 2020
@getspooky
Copy link
Contributor Author

getspooky commented Mar 14, 2020

Hello and thank you for responding 😉
Sometimes method chaining makes code more readable, but if you get carried away, it can make it less readable and there is no need to chain the methods that handle the same function.

function handleRequest (req,res) { console.log(req.method); }
app.route('/')
  .get(handleRequest)
  .post(handleRequest)
  .put(handleRequest)

I think the match method could solve this problem:

app.match(['get','post','put'], '/' ,function(req,res) {
 console.log(req.method);
 });

@dougwilson
Copy link
Contributor

Hm. I have never quite seen in the wild the ability to use the same function for different methods. Can you provide some use cases for even needing that to be built into the framework? It seems unlikely a handler to just log the method would be a use case.

I would suggest opening a new issue with your proposal and provide some great examples of what you would have to write in the real world without this helper so we can better understand the utility of it. As in, make the case in an issue to add it :)

On the surface, this PR doesn't add anything a user cannot already do with our route API, and I'm unsure what kind of handlers would exist that could handle both a GET, POST, and PUT all together without ended up with a switch internally to do different actions based on the method.

@UlisesGascon
Copy link
Member

Hi @getspooky! This request seems similar to #4181.

I agree with @dougwilson, the best option here is to use app.route but in case that you need a switch approach I recommend the following (DRY):

app.all('/', (req, res) => {
    const validMethods = ["GET", "POST"];
    const { method }  = req
    if (validMethods.contains(method)) {
        return res.send(`Current method: ${method}`)
    }
    res.status(405).send() //405 Method Not Allowed
})

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

Successfully merging this pull request may close these issues.

3 participants