session icon indicating copy to clipboard operation
session copied to clipboard

backlog for [email protected]

Open bjohansebas opened this issue 1 year ago • 9 comments

I’m opening this issue to discuss a potential version 2 of express-session. Here are some things that could be included in this version 2:

  • Drop support for Node.js <18.
  • Code modernization (Migrate to new ES syntax)
  • Add type definition files (.d.ts).
  • Use cookie v1, which introduces breaking changes.
  • Add support for HTTP/2 (there’s a PR that partially addresses this https://github.com/expressjs/session/pull/908).
  • Fix the res.end error: https://github.com/expressjs/session/pull/751.
  • Do not remove the legacy session: https://github.com/expressjs/session/pull/419
  • https://github.com/expressjs/session/pull/977.
  • Provide an asynchronous API: https://github.com/expressjs/session/issues/952 https://github.com/fastify/session/pull/78.
  • Improve debug mode: https://github.com/expressjs/session/pull/708 https://github.com/expressjs/session/pull/986.

These are the issues and PRs I’ve identified as relevant for a version 2. I’d appreciate any feedback to move forward with modernizing this package.

cc: @expressjs/session-collaborators @UlisesGascon @expressjs/express-tc

bjohansebas avatar Dec 06 '24 18:12 bjohansebas

#992

kamalyusuf avatar Dec 12 '24 04:12 kamalyusuf

This sounds great! Let use this issue as a backlog. I will create a branch v2 where we can land PRs

UlisesGascon avatar Dec 20 '24 15:12 UlisesGascon

Branch created: https://github.com/expressjs/session/tree/v2

UlisesGascon avatar Dec 20 '24 15:12 UlisesGascon

@bjohansebas seems like we have already a milestone https://github.com/expressjs/session/milestone/1 for v2.x . Maybe we can rescue some ideas from there and use re-use the milestone. WDYT?

UlisesGascon avatar Dec 20 '24 15:12 UlisesGascon

Sure, I hadn't noticed that this Milestone for v2 already existed. Happy to review it when I have access to my computer.

bjohansebas avatar Dec 20 '24 16:12 bjohansebas

Hi. I'm in a project where we need couple of new features for session management, which express-session doesn't yet have. There are PRs or issues about both of those. Namely, rotating the token in cookie and requiring user to login after some time. To be able to add those to this project, I assume the v2 branch is the way to go (i.e., I assume no new features will be merged to v1).

To look how viable developing the library would be, I looked into how complicated it would be to make API for the cookie handling part of the code. I moved all the cookie handling behind a "token management api". As a result, it seems it could be a good way to support multiple different wants (e.g., custom HTTP header instead of a cookie). If interested, here is the code.

So, to get actually stuff done, I think I could do bit of maintenance for this repo (note, I'm just random developer and user of this library). I was thinking the following steps:

  1. PR to remove deprecated APIs (signed cookies and cookie parser, if I remember right).
  2. PR to rewrite source code in Typescript, if there are no objections to use Typescript in place of just adding type definition files (.d.ts). This should fix the first 3 points in the issue.
  3. Then I could look into PR to change the API and move cookie management to a separate class. This would impact the configuration interface, I think.

How you feel about these plans? Are there some other opinions? Is someone actively working on those?

raphendyr avatar Mar 13 '25 09:03 raphendyr

It's great that you want to help. I'm going to respond to some of your points.

I assume no new features will be merged to v1

I would say it will be like this, at least from my side. I’m not sure what the other members think, but if we have plans to release a version 2, then it’s something we’ll be focused on launching in the coming months

PR to remove deprecated APIs (signed cookies and cookie parser, if I remember right).

Those PRs already exist, please review them. We need people to review the PRs so we can merge the existing ones (see https://github.com/expressjs/session/pull/1026, https://github.com/expressjs/session/pull/1025).

PR to rewrite source code in Typescript, if there are no objections to use Typescript in place of just adding type definition files (.d.ts). This should fix the first 3 points in the issue.

There are no intentions to rewrite the library in TypeScript, what we are going to do is integrate the types directly into the library, at least that's what has been discussed so far (see https://github.com/expressjs/typescript-wg/issues/1, https://github.com/expressjs/express/pull/6382)

Then I could look into PR to change the API and move cookie management to a separate class. This would impact the configuration interface, I think.

Is there any PR or issue that indicates this? I don't remember very well, although I partially support the idea, it depends on how the PR is approached. If there's no issue, please open one, I would love to read the proposal.

Is someone actively working on those?

Ulises and I are working on this, but if anyone else wants to help, it would be really amazing.

bjohansebas avatar Mar 13 '25 22:03 bjohansebas

Amazing to hear.

Those PRs already exist, please review them. We need people to review the PRs so we can merge the existing ones (see https://github.com/expressjs/session/pull/1026, https://github.com/expressjs/session/pull/1025).

Perfect. Sorry that I missed those. Any how, I looked into these and they look good. I added few minor comments.

There are no intentions to rewrite the library in TypeScript, what we are going to do is integrate the types directly into the library, at least that's what has been discussed so far (see https://github.com/expressjs/typescript-wg/issues/1, https://github.com/expressjs/express/pull/6382)

Thanks. This was a good discussion. I think all my points were mentioned there.

As a next thing, I look creating a similar PR here about the d.ts files. And I need to experiment how JSDoc types and d.ts files can co-exists, should they and how those help provide type annotations to VSCode for example.

( Note to self, see #723 ).

Then I could look into PR to change the API and move cookie management to a separate class. This would impact the configuration interface, I think.

Is there any PR or issue that indicates this? I don't remember very well, although I partially support the idea, it depends on how the PR is approached. If there's no issue, please open one, I would love to read the proposal.

The expressjs/discussions#161 is a long issue about using express-session in API without cookies. And there is a kind of duplicate need in expressjs/discussions#185, #543, #567. Similar need in expressjs/discussions#317. Also related #158. Earliest custom header issue expressjs/discussions#77 with PR expressjs/discussions#79. The angle of overriding get and set cookie in #468.

In addition, there are issues about expiring the token or rotating it and so on in #425, #688, #583. Related expressjs/discussions#2.

Max session duration when rotating the cookie in #557.

I did a prototype of the rewrite (linked above, but it's ok): https://github.com/raphendyr/express-session/commit/85c9bd9a3088c636f379e2a029cbff4eba4b12d1

Thanks for everything!

raphendyr avatar Mar 14 '25 15:03 raphendyr

I set aside some time this week to work on the repo and review/merge the current PRs. I'm very happy to see this level of engagement recently! ❤️

UlisesGascon avatar Mar 16 '25 18:03 UlisesGascon