Add parse method for set-cookie

[blakeembrey]

Other part of #200, follow up to #214. Adds a deserialize method that mirrors the serialize method.

The current signature of serialize is (key, val, options) which doesn't lend itself to being mirrored particularly nicely. In this PR I've opted to support an object setCookie with key and value properties, and kept the old signature for backward compatibility. Alternatively, I could return a tuple, but options isn't the cleanest already with both values that get used for set-cookie and encode which is a function used for value.

[blakeembrey]

Alternatively, could avoid changing this interface, but it's nice to have parity with serialize(deserialize("")).

[wesleytodd]

High level comment here, but I feel like having these "generic" names like stringify and serialize don't do a great job of showing users what they are intended to be used for. Part of the ongoing confusion in this package has been that folks thought stringify could be used with set-cookie. Now I am concerned that the new name will mean folks are confused on which is for the cookie and which is for set-cookie.

I think there are two ways I could see us improving this:

1. A naming convention like encodeSetCookie/encodeCookie. Downside of longer names and having to alias & deprecate encode (maybe more?). Upside, has a clean path to landing here without major disruption and clarifies the intent.
2. Namespacing. There are a few ways I can think of (cookie.set.encode/cookie.get.encode, cookie.encode.set/cookie.encode.get, names to be bikeshed), but I think I am more in favor of option one, so don't want to spend too much time on this one unless other folks really like this idea.

I read through some of the details, but other than small nitpicks I might make I don't have a lot to add. Leaving as a comment so I don't accidentally block, but I am pretty concerned with the point I raised above.

[jonchurch]

For how my brain works, the API here is hard to hold onto.

I can't keep straight what methods are meant for what header type

After reviewing the code, and not looking at it right now, let me try to recall.

parse/stringify is for Cookie header, and serialize/deserialize is for Set-Cookie header?
(checked after I wrote that, phew, got it right!)

That's my biggest nit here.

What about exploring aliasing to prevent needing a major ala:

export {
  parse,
  stringify,
  parse as parseCookie,
  stringify stringifyCookie,
  serialize,
  deserialize,
  serialize as serializeSetCookie,
  deserialize as deserializeSetCookie
 };

Or otherwise namespacing to make it clearer

I think standardizing on a single verb like parse/serialize and tacking on the header type would make it a more straightforward DX as well. is there a real difference between parse/deserialize besides the type of header they operate on and the algorithm they use?

[blakeembrey]

Agree with the naming, I just stuck to the existing scheme. The existing methods were parse and serialize 🤷

I can rename in a follow up PR or this one, whichever is preferred. Though I'd go with parseCookie and parseSetCookie - not sure why I'd use two different naming schemes there?

is there a real difference between parse/deserialize besides the type of header they operate on and the algorithm they use?

I'm not sure how to answer this, but no, aside from the entire algorithm they are meant to be mirrors of each other. Ideally you can do stringify(parse("")) and serialize(deserialize("")).

[jonchurch]

Suggested change

	Since the value of a cookie has a limited character set (and must be a simple string), these functions are be used to transform values into strings suitable for a cookies value.
	Since the value of a cookie has a limited character set (and must be a simple string), these functions are used to transform values into strings suitable for a cookies value.

[jonchurch]

is there a real difference between parse/deserialize besides the type of header they operate on and the algorithm they use?

was just a politer way to say "it doesnt make any sense to me that they're not the same name"

I know it's not a choice you made hehe

[blakeembrey]

was just a politer way to say "it doesnt make any sense to me that they're not the same name"

Haha cool, it's fine to say that, it's reasonable. Do you think they should both be verbose? E.g. parseCookie, parseSetCookie and stringifyCookie, stringifySetCookie? That also makes the most sense to me, although the existing name is serialize?

I'm doing parse and stringify due to existing APIs like JSON using that naming. Open to going another route entirely.

[jonchurch]

Im down with the change.

I do want to see the naming of the methods cleaned up, but this PR is 👍 as is

[blakeembrey]

@jonchurch Methods have been renamed, here's the backward compatibility exports (no version with stringify was released).

[jonchurch]

blake and I synced offline, i am removing the block and will let him sort out if he wants to ignore invalid values in attributes, or handle it another way.

[blakeembrey]

The spec itself has us ignoring invalid fields, e.g.

If the remainder of attribute-value contains a non-DIGIT character, ignore the cookie-av.

That said, we're not specifically following the spec in other places. For examples, it's more lenient on no validation for domain or path, or it removes prefixed . on domain, etc.

I'm leaning most toward following the spec, e.g. ignoring invalid max-age and expires but being more lenient on string fields. Especially since this is a "real" parser and not just collecting raw fields.

I do like the idea of an escape hatch allow users to potentially parse a raw array + serialize that without all the validation, though that's an entirely different implementation.