Skip to content

Conversation

@ascorbic
Copy link
Contributor

@ascorbic ascorbic commented Dec 9, 2024

Changes

Adds support for session data expiry. This includes:

  • a new ttl config option
  • support for a ttl argument to set
  • support for session.flash(), which expires the data after the next request

Testing

Added unit tests

Docs

@changeset-bot
Copy link

changeset-bot bot commented Dec 9, 2024

⚠️ No Changeset found

Latest commit: a55fedb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) docs pr labels Dec 9, 2024
@ematipico
Copy link
Member

Can you update the RFC with the relative information, so we can review the PR?

@ascorbic
Copy link
Contributor Author

ascorbic commented Dec 9, 2024

@ematipico the RFC has been updated

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Code-wise, I would add more test cases, especially for middleware and rewrites. I also left some questions in the RFC around the .flash.

Comment on lines +2 to +3
Astro.session.flash('flash-value', `Flashed value at ${new Date().toISOString()}`);
return Astro.redirect('/');
Copy link
Member

Choose a reason for hiding this comment

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

Can we add more cases, such as:

  • usage with Astro.rewrite
  • usage with the next("/some-route") (the other form of rewrite): see if the value changes when the next middleware function read from .flash
  • usage with multiple middleware functions e.g. sequence(fn1, fn2)

this.#data ??= new Map();
this.#data.set(key, value);
const lifetime = ttl ?? this.#config.ttl;
const expires = typeof lifetime === 'number' ? Date.now() + lifetime * 1000 : lifetime;
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a comment that explains the 1000?

@ascorbic ascorbic merged commit f45962d into astro-dot-session Dec 10, 2024
4 checks passed
@ascorbic ascorbic deleted the session-ttl branch December 10, 2024 15:58
ematipico added a commit that referenced this pull request Dec 18, 2024
* wip: experimental sessions

* feat: adds session options (#12450)

* feat: add session config

* chore: add session config docs

* Fix

* Expand doc

* Handle schema

* Remove example

* Format

* Lock

* Fix schema

* Update packages/astro/src/types/public/config.ts

Co-authored-by: Sarah Rainsberger <[email protected]>

* Update packages/astro/src/types/public/config.ts

Co-authored-by: Sarah Rainsberger <[email protected]>

* Add link to Sessions RFC in config.ts

* Move session into experimental

---------

Co-authored-by: Sarah Rainsberger <[email protected]>

* Lock

* feat: prototype session support (#12471)

* feat: add session object

* Add tests and fix logic

* Fixes

* Allow string as cookie option

* wip: implement sessions (#12478)

* feat: implement sessions

* Add middleware

* Action middleware test

* Support URLs

* Remove comment

* Changes from review

* Update test

* Ensure test file is run

* ci: changeset base

* ci: exit from changeset pre mode

* Lockfile

* Update base

* fix: use virtual import for storage drivers (#12520)

* fix: use virtual import for storage drivers

* Don't try to resolve anythign in build

* Fix test

* Polyfill node:url

* Handle custom drivers directly

* No need for path

* Update packages/astro/src/core/session.ts

Co-authored-by: Emanuele Stoppa <[email protected]>

---------

Co-authored-by: Emanuele Stoppa <[email protected]>

* Fix jsdoc

* fix: set default storage path

* Update changeset config for now

* Revert config workaround

* Lock

* Remove unneeded ts-expect-error directive

* fix: [sessions] import storage driver in manifest (#12654)

* wip

* wip

* Export manifest in middleware

* Changeset conf

* Pass session to edge middleware

* Support initial session data

* Persist edge session on redirect

* Remove middleware-related changes

* Refactor

* Remove vite plugin

* Format

* Simplify import

* Handle missing config

* Handle async resolution

* Lockfile

* feat(sessions): implement ttl and flash (#12693)

* feat(sessions): implement ttl and flash

* chore: add unit tests

* Make set arg an object

* Add more tests

* Add test fixtures

* Add comment

* Remove session.flash for now (#12745)

* Changeset

* Apply suggestions from code review

Co-authored-by: Sarah Rainsberger <[email protected]>

---------

Co-authored-by: Sarah Rainsberger <[email protected]>
Co-authored-by: Emanuele Stoppa <[email protected]>
Co-authored-by: Sarah Rainsberger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs pr pkg: astro Related to the core `astro` package (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants