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
Serialize objects of key/value pairs #47
base: master
Are you sure you want to change the base?
Conversation
index.js
Outdated
|
||
var cookieNames = Object.keys(cookies); | ||
if(0 === cookieNames.length) { | ||
return ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps return undefined
for this case? Not too clear, but just feel like setting the Set-Cookie
header to the result would make more sense if undefined
was returned, since that would just not send the header instead of send an empty header.
Conflicts: test/serialize.js
serializedCookies[i] = serializeNameValue(cookieNames[i], cookies[cookieNames[i]], serializeOptions); | ||
} | ||
|
||
return serializedCookies; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if given a single key-value pair in the object, this returns an array of serialized cookie strings. I thought this might make the function more consistent in that if you pass an object, you always get back an array of strings rather than returning just a single string if you give only a single key-value pair in the object and an array if you give multiple key-value pairs in the object.
I still left the behavior that when given a name string and a value string, the function returns a single serialized cookie string so this does not break anyone's code that uses the old version of this function.
What would you think about having separate methods for parsing/serializing the Set-Cookie response header and the Cookie request header? Something like |
To follow up on this, my workaround was to do something like:
The The single line of using If anything, instead of overloading the method, I would maybe add a |
Was looking around for something like this today, happy to take this over if it's something still of interest to people? |
@TomVance @jgornick @NedalEldeen @gatkin @dougwilson I have wanted this feature recently and updated a new PR against the current state of the main branch. |
Hi @gatkin I know this is old, but I'm trying to rebase and merge your changes. It seems I don't have the ability to push the rebase to your branch. If you're still around and can check the box on the right-hand side in this PR to allow me to push, that would be awesome. If not, no problem. I'll still get the changes landed, they just won't appear as the PR is merged. |
Updates the serialize method to accept objects of key-value pairs.
If the object of key/value pairs contains multiple pairs, then allow the optional options object to specify only an encoding. If the object of key/value pairs contains only one pair, then the options object may specify cookie parameters as well as an encoding.Resolves #30