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

Serialize objects of key/value pairs #47

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

gatkin
Copy link

@gatkin gatkin commented Oct 25, 2015

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

index.js Outdated

var cookieNames = Object.keys(cookies);
if(0 === cookieNames.length) {
return '';
Copy link
Contributor

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.

serializedCookies[i] = serializeNameValue(cookieNames[i], cookies[cookieNames[i]], serializeOptions);
}

return serializedCookies;
Copy link
Author

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.

@gatkin
Copy link
Author

gatkin commented Oct 26, 2015

What would you think about having separate methods for parsing/serializing the Set-Cookie response header and the Cookie request header? Something like parseSetCookie, serializeSetCookie, parseCookieRequest, serializeCookieRequest? If this is a worthwhile change, I can make an issue and work on a pr for this.

@jgornick
Copy link

jgornick commented Aug 4, 2016

To follow up on this, my workaround was to do something like:

const cookies = { foo: 'bar', cat: 'meow', dog: 'ruff' };
const serializedCookies = Object.keys(cookies).map(key => cookie.serialize(key, cookies[key]);

The serializedCookies is now an array of serialized key/value pairs (as string).

The single line of using Object.keys and Array.map made me think if there's really a need to overload the serialize method with this ability.

If anything, instead of overloading the method, I would maybe add a serializeObject method with a type of serializeObject :: Object -> Array

@TomVance
Copy link

Was looking around for something like this today, happy to take this over if it's something still of interest to people?

@export-mike
Copy link

@TomVance @jgornick @NedalEldeen @gatkin @dougwilson

I have wanted this feature recently and updated a new PR against the current state of the main branch.

#119

dougwilson pushed a commit to jridgewell/cookie that referenced this pull request Mar 30, 2022
@dougwilson dougwilson removed this from the 1.0 milestone Mar 30, 2022
@dougwilson
Copy link
Contributor

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.

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.

Update serialize to accept a object of key/values.
6 participants