Skip to content

Conversation

@parenthetical
Copy link

@parenthetical parenthetical commented Jan 25, 2023

This extends the Cookie module with a class for setting cookies and adds some documentation to the module.

HasSetCookie is implemented on Snap and MonadJSM/Client m. The Haddocks warn that HasSetCookie and HasCookies do not necessarily implement a state monad. While this is the case for Client m, in Snap setting cookies affects the HTTP response while asking cookies looks at the request. I did this for pragmatic reasons (not breaking old code, not requiring a wrapper type inside Snap to distinguish the two sources of cookies).

A few helper functions are added to encourage Base64 use by default: setCookie and askCookie work do Base64 encoding/decoding of cookies.

  • Based work on latest develop branch
  • Followed the contribution guide
  • Looked for lint in my changes with hlint . (lint found code you did not write can be left alone)
  • Run the test suite: $(nix-build -A selftest --no-out-link) Failed on some unrelated points?
  • Updated the changelog
  • (Optional) Run CI tests locally: nix-build release.nix -A build.x86_64-linux --no-out-link (or x86_64-darwin on macOS)

Closes #977.

@madeline-os madeline-os self-requested a review January 26, 2023 15:05
Copy link
Collaborator

@madeline-os madeline-os left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just tiny requests

{-# LANGUAGE UndecidableInstances #-}
{-# LANGUAGE OverloadedStrings #-}
{-|
Description:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation 💜




-- TODO: Make generic over both frontend and backend.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo in code >:|

can you make an issue instead?

askCookies = fmap (parseCookies . encodeUtf8) $ DOM.getCookie =<< askDocument

-- | Store a cookie which will be Base64 encoded.
setCookie :: (HasSetCookie m) => SetCookie -> m ()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we name this encodeAndSetCookie? I don't want it to be surprising that this function re-encodes your cookie value for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants