Add Snap.Cookie with a Cookie type using lenses

[kaol]

This also implements SameSite for cookies.

The change should be mostly transparent for code using the old
Cookie type. The functions that use Cookie now use a IsCookie
typeclass instead.

This change will break code that does not actually use the Cookie value but just passes it along to isNothing or the equivalent. rqCookies is no longer a field so any use of it as a record selector breaks as well but I'm not sure if that's even a concern with user code. I'm hoping that neither is too much of an issue.

Any feedback on how I implemented Snap.Cookie? It was quite lightweight and I saw no reason to split any of it to an internal module and make re-exports.

If Snap would have a major version update with code breakage then I would suggest merging Snap.Cookie to Snap.Internal.Http.Types and make the former just offer re-exports.

[tom-audm]

No opinion on the actual feature here, but can we please not include lens as a dependency in snap-core? It's a massive set of additional transitive dependencies.

https://ro-che.info/ccc/23

[kaol]

The main snap package already depends on lens, does snap-core get used without it a lot?

Anyhow, it would be quite possible to drop the dependency and add some module Snap.Cookie.Lens to snap which would do the makeLenses ''Cookie call instead of having it in Snap.Cookie.

[tom-audm]

The main snap package already depends on lens, does snap-core get used without it a lot?

Not that Hackage download numbers are super reliable (because of CDNs, etc) but according to those numbers, 20% of the usage of snap-core (76084 downloads) is without snap (61009 downloads). Anecdotally also, I know multiple people who use snap-core by itself as a more lightweight tool.

[mightybyte]

Agree with @tom-audm. Depending on lens in snap-core is a no-go.

I'm not quite sure what I think about the tradeoff between two Cookie types and preserving backwards compatibility. @gregorycollins any thoughts?

[gregorycollins]

Sorry for taking so long to get to this, I'm on pandemic paternity leave and am not even looking at computers right now.

I'm definitely in favour of adding this feature. Lens as a dep for snap-core gets a downvote from me though. I'll get to that. Here's the thing, we screwed this up: in almost all of our code we have been careful to not expose our datatype constructors in public interfaces, so that the underlying datatypes can be changed without breaking user code. Somehow we missed this for Cookie, and now there's a new-ish cookie feature, and ugh.

Can we have this as an action plan?

• remove Default instance and dependency
• 👍 to "remove old Cookie datatype, hide constructor, move to a new module"
• 👎 to "lens as a snap-core dep", adding get + set functions instead of lens TH
• this will require a major version bump :(

[gregorycollins]

I don't love Default as part of my war against "typeclasses without laws". My argument is usually: "why is writing def preferable to emptyCookie :: Cookie"?