feat: Add callback for configuring Vary header#318
feat: Add callback for configuring Vary header#318gl-jkeys wants to merge 6 commits intoexpressjs:masterfrom
Conversation
|
I enabled the CI to run in this PR 👍 |
|
Hey @gl-jkeys 👋, first of all, thank you for being an awesome contributor.
It makes more sense if that README change is also included in this PR. Could you please take care of that? (Note to self: I was unable to run the tests due to my own PC issue, I will check the tests again sometime later soon). |
|
Hey @IamLizu, I made the changes you requested; I also modified the PR to unconditionally set the Vary: Origin header for OPTIONS requests. (It seems unlikely, in hindsight since this PR was opened, that OPTIONS requests would be cached without respect to Origin.) I also simplified the tests to match the updated implementation. |
|
Hey @gl-jkeys sorry I missed this one. I will run the tests in my pc and get back to you. |
IamLizu
left a comment
There was a problem hiding this comment.
LGTM 👍
I am in favor of having the option to not set Vary header where it could help avoiding unncessary cache fragmentation.
Fixes #317
This PR adds a callback (
shouldSetVaryHeader) to the options which is used to determine when and how to configure the Vary header.To preserve backward compatibility with all existing
corsusages, this callback returnstrueby default.If this PR is accepted, I will add documentation to the README on how to use this PR's functionality.