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

Export a function that takes a res and sets the headers #15

Closed
satazor opened this issue Nov 24, 2017 · 5 comments
Closed

Export a function that takes a res and sets the headers #15

satazor opened this issue Nov 24, 2017 · 5 comments

Comments

@satazor
Copy link

satazor commented Nov 24, 2017

There are cases where we simply want to set the nocache headers in a handler, depending on some condition. Using this as a middleware on those cases is not as convenient. We can get around it but it's ugly:

// If no cache-control headers are still defined, default to no-cache
if (res.get('cache-control')) {
    nocache()(null, res, () => {});
}

// some more code

..instead it would be cool to do:

// If no cache-control headers are still defined, default to no-cache
if (res.get('cache-control')) {
    nocache.setHeaders(res);
}

// some more code

(the name of the method can obviously be more appropriate).

Would you be willing to provide this functionality?

@EvanHahn
Copy link
Member

Conditional application of Express middleware is a problem that's plagued Helmet and Express in general. Would something like this work for you?

app.get('/some-route',
  function (req, res, next) {
    if (res.get('cache-control')) {
      nocache()(null, res, next);
   } else {
     next();
   },
   function (req, res) {
     res.send('this is the response');
   }
);

@satazor
Copy link
Author

satazor commented Nov 24, 2017

Your solution is perfectly valid, I was just hoping to make it less clunky.

@EvanHahn
Copy link
Member

I don't want to add something to Helmet (conditional middleware) that I feel should really be added to Express. (I've tried a few times to write a generalized conditional middleware package but have been unable to find something much less clunky than my solution above.)

Is it okay if I close this issue?

@satazor
Copy link
Author

satazor commented Nov 24, 2017

👍

@EvanHahn
Copy link
Member

Feel free to open a new issue if you run into anything else!

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

No branches or pull requests

2 participants