-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
Extend res.links() to allow adding multiple links with the same rel #2729 #4885
Conversation
Hi @dougwilson, |
Hi @andvea no need to do anything. We'll get it fixed up as it get close to landing. Sorry for any confusion there. An issue like that which is not caused by your PR can typically be ignored as it won't be held against your PR or anything :) |
Great, wish you good work! |
eb10dba
to
340be0f
Compare
a1ff1b0
to
ec23568
Compare
Since we are now targeting v5 we can use nice things like template literals. Just made a few small tweaks but it should be true to the original and if we get a few approvals (I posted in our slack asking for some eyes) then I will land this for the next release. |
Ah, I was continuing down the list of PRs and I saw this similar one: https://github.com/expressjs/express/pull/2738/files While it has no description about this, it supports something which I am realizing now we are missing: multiple params in the link. https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Link I am thinking I should rebase that commit on top of this and clean it up to include both. |
re: combining with the related PR method overloading via type checking. sure. might want to clean up / optimize how we are building strings here as well |
There are two parts you could be referring to, so just to clarify, I want the value to be either a string or an object with an Is this being too paranoid? |
After sleeping on it, and also taking a little poll on bluesky, I think it is not too paranoid and should not land with this PR. So I marked that one with the updated labels and will go forward with this as is. |
This pr fixes #2729
I've done the following: