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

v8 beta release, last tasks and discussions #620

Closed
vvo opened this issue Nov 18, 2023 · 6 comments
Closed

v8 beta release, last tasks and discussions #620

vvo opened this issue Nov 18, 2023 · 6 comments
Assignees

Comments

@vvo
Copy link
Owner

vvo commented Nov 18, 2023

Hi there, @brc-dd and @renchris. First, thank you for all your contributions and help and everyone who chimed in on #586.

Here's the status: I published iron-session@8.0.0-beta.6, please give it a try:

pnpm add iron-session@8.0.0-beta.6

Usage: https://github.com/vvo/iron-session/tree/v8#usage

Notable changes and questions to finish this release:

  1. There's a single method now: getIronSession(req, res, sessionOptions) and getIronSession(cookieStore, sessionOptions
  2. @brc-dd Can you provide explanations why we need mergeHeaders and createResponse? For now I removed them until I understand why they are needed in the lib itself
  3. @renchris Can you provide explanations why we need OverridableOptions? I don't understand the usecase yet, please be specific 🙏
  4. For now, I removed deno, bun, lagon, koa examples to focus on Next.js. We may re-introduce such examples but they are adding noise to the repository. I think we could have them in different repositories afterwards (perhaps we move to an org)
  5. I added many examples to the Next.js one, give it a try with pnpm dev at the root of the repo
  6. I will detail more work for us to collaborate once we fix the previous items, and we release it to latest

Thank you again, looking forward to discussing here now and reach a shippable state super soon.

@vvo vvo assigned vvo and brc-dd Nov 18, 2023
@vvo vvo mentioned this issue Nov 18, 2023
13 tasks
@vvo
Copy link
Owner Author

vvo commented Nov 18, 2023

cc @devjmetivier as for the OverridableOptions question. Thanks.

@brc-dd
Copy link
Collaborator

brc-dd commented Nov 18, 2023

Can you provide explanations why we need mergeHeaders and createResponse

These aren't required in the next.js example because we are passing cookies() there. If we pass something like:

const res = new Response()

getIronSession(req, res, ...)

then cookies are appended to the original res object. Later if someone wants to, say, specify body based on the session data, they will need something like createResponse(res, { body: 'the user is authenticated' }).

If we aren't planning on supporting other frameworks, then I guess it's fine to not export them.

@vvo
Copy link
Owner Author

vvo commented Nov 20, 2023

iron-session v8 is live now 👍 https://github.com/vvo/iron-session

@vvo vvo closed this as completed Nov 20, 2023
@renchris
Copy link
Contributor

Hey. I see this is closed now, however, for the use case of Overridable Options is that:

We can set our options for our Iron Session object during

  1. Initiation
const getServerActionSession = async () => {
  const session = getServerActionIronSession<IronSessionData>(sessionOptions, cookies())
  return session
}
  1. Function call .save() or .destroy()
export const submitCookieToStorageServerAction = async (cookie: string) => {
  const session = await getServerActionSession()
  session.cookieVariable = cookie
  const newCookieOption = { cookieOptions: { maxAge: undefined } }
  await session.save(newCookieOption)
}

We need options for the developer to pass in Cookie options that are different than the defaultOptions at the time of Iron Session object initiation and/or later at the time of .save()/.destroy() function call.

const defaultOptions: Required<OverridableOptions> = {
  ttl: fourteenDaysInSeconds,
  cookieOptions: { httpOnly: true, secure: true, sameSite: 'lax', path: '/' },
}
function mergeOptions(
  userSessionOptions: IronSessionOptions,
  overrides?: OverridableOptions,
): Required<IronSessionOptions> {
  const options: Required<IronSessionOptions> = {
    ...defaultOptions,
    ...userSessionOptions,
    ...overrides,
    cookieOptions: {
      ...defaultOptions.cookieOptions,
      ...userSessionOptions.cookieOptions,
      ...overrides?.cookieOptions,
    },
  };

We need the type OverridableOptions to ensure ttl is passed as our Iron Session core code expects and the cookieOptions are passed to the Cookie handler as the NextJS cookies() function expects

type OverridableOptions = Pick<IronSessionOptions, "cookieOptions" | "ttl">;
The final type definition for `CookieOptions` ends up to be
`'domain' | 'path' | 'secure' | 'sameSite' | 'name' | 'value' | 'expires' | 'httpOnly' | 'maxAge' | 'priority'`

Note these code examples refer to the codebase before the most recent Friday release changes. Congratulations on the release!

@vvo
Copy link
Owner Author

vvo commented Nov 21, 2023

Thank you @renchris, for the detailed explanation.
Do you have any real-world example in mind for when you would need to update ttl and/or cookie options during a save()? I am not saying they are none but I like to start from the problem (? changing the cookie name, why?) and then move to the solution (save accepts options)

What I have done in the end: I created updateConfig that you can call anytime to update any options:

readonly updateConfig: (newSessionOptions: SessionOptions) => void;

This should be enough, correct?

@vvo
Copy link
Owner Author

vvo commented Nov 21, 2023

Also @renchris I mentioned you here: https://twitter.com/vvoyer/status/1726521507457986758 👍

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

No branches or pull requests

3 participants