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

Enable multiple cookie-sessions #172

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

ZacTredger
Copy link

Enables the user to create arbitrarily many cookie-sessions, whose options can differ. The API design was motivated by ensuring backwards compatibility with v2 (more details in the Discussion below).

Fixes:

The new features are pretty comprehensively tested, and I'm using them in production. But I'm still happy to discuss alternative APIs/implementations. I've also got a sister fork of Definitely Typed with updated typings. I'll create a PR there when this gets merged.

I'm requesting to merge into master because I noticed that you have some WIP changes on develop.

Example

A fairly complex example taken from the new tests to illustrate the new features:

app = connect()
app.use(session([
  {
    signed: false,
  }, {
    signed: false,
    name: 'secondary',
    sessionName: 'secondary',
  }
]))
app.use(function (req, _res, next) {
  req.session.name = req.sessionOptions.name
  req.sessions.secondary.name = req.sessionsOptions.secondary.name
  next()
})
app.use('/', function (req, res, _next) {
  res.end(req.session.name + req.sessions.secondary.name)
})

request(app)
  .get('/')
  .expect(shouldHaveCookie('session'))
  .expect(shouldHaveCookie('secondary'))
  .expect(200, 'sessionsecondary', done)
})

Discussion

I tend to discuss the "why", including rejected alternative designs, in my commit messages. But I've basically reproduced that commentary here for convenience.

Enable setting multiple cookie-sessions

Introduces a new option, 'sessionName', which is the key under Express.Request.sessions where the session can be accessed. 'session' is the default sessionName, and sessions with sessionName 'session' are still accessible at Express.Request.session. This API was chosen to avoid breaking existing applications.

Rejected alternative APIs:

  • The name property acts as the session accessor on req.sessions. Either obviating the need for the sessionName option, or perhaps allowing the sessionName to default to name. This would be a breaking change in all apps that provide a non-default value for name. I considered working around this by always making the first cookie-session created the one accessible at req.session, but this felt hacky and potentially confusing.
  • Each session is accessed by adding its sessionName as a property on req. I feared this would pollute the namespace, risking name collisions with other modules.

Call cookieSession with multiple options objects

Users can now pass several, or an array of, options objects to cookieSession. One middleware function will always be returned that creates all the sessions. This means that users using multiple cookie-sessions in their app never need to call cookieSession multiple times. cookieSession now also checks that the sessions have unique names and sessionNames to avoid tricky overwriting bugs.

I rejected an alternative approach, in which cookieSession returns an array of middleware functions. You can pass Express an array of middleware exactly like passing a single functions, so it wouldn't break any user's apps in those cases. But we can't assume that all users are passing the function straight to Express.

Introduces a new option, 'sessionName', which is the key under
`Express.Request.sessions` where the session can be accessed. 'session' is the
default sessionName, and sessions with sessionName 'session' are still
accessible at `Express.Request.session`. This API was chosen to avoid breaking
existing applications.

Rejected alternative APIs:
- The `name` property acts as the session accessor on `req.sessions`. Either
obviating the need for the `sessionName` option, or perhaps allowing the
`sessionName` to default to `name`. This would be a breaking change in all
apps that provide a non-default value for `name`. I considered working around
this by always making the first cookie-session created the one accessible at
`req.session`, but this felt hacky and potentially confusing.
- Each session is accessed by adding its sessionName as a property on `req`.
I feared this would pollute the namespace, risking name collisions with other
modules.
Users can now pass several, or an array of, options objects to `cookieSession`.
One middleware function will always be returned that creates all the sessions.
This means that users using multiple cookie-sessions in their app never need to
call `cookieSession` multiple times. `cookieSession` now also checks that the
sessions have unique names and sessionNames to avoid tricky overwriting bugs.

I rejected an alternative approach, in which `cookieSession` returns an array of
middleware functions. You can pass Express an array of middleware exactly like
passing a single functions, so it wouldn't break any user's apps in those cases.
But we can't assume that all users are passing the function straight to Express.
Bumps actions/checkout from v2 to v3
Uses latests Ubuntu version for tests

Because actions using Node12 are being deprecated:
https://github.blog/changelog/2022-09-22-github-actions-all-actions-will-begin-running-on-node16-instead-of-node12/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants