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 app.request for set get, post #4181

Closed
wants to merge 1 commit into from

Conversation

bluelovers
Copy link

No description provided.

@dougwilson
Copy link
Contributor

Hello @bluelovers ! Thank you for the pull request. Can you explain what it does and what it would achieve other the existing APIs and the use-case for this new method? This will help us better understand the purpose to evaluate it.

@UlisesGascon
Copy link
Member

UlisesGascon commented Feb 22, 2020

Hi @bluelovers!

Thanks for your PR 🤗

As far I know you can use .all to fix this requirement using a filter, let me show you a simple example 🤔

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
})

I don't think app.request will add an extra value to the Express Core, but maybe it is a great opportunity for you to create an independent module in NPM for that as external middleware 👍.

@dougwilson
Copy link
Contributor

I'm going to close this since we asked for more information and never got any, and the PR remains in a failing state.

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