kleydon / prisma-session-store

Express session store for Prisma
MIT License
116 stars 18 forks source link

Dependency Update and Fixes #31

Closed wSedlacek closed 3 years ago

wSedlacek commented 3 years ago

Description

This updates the dependencies and fixes conflicts introduced with their updates.

fixes: #30

wSedlacek commented 3 years ago

@kleydon When you get a chance, take look at this. It fixes some typing changes and updates the packages. Some of these changes are breaking due to the fact that @types/express and @types/express-session are not dependencies.

We should probably discuss locking our package versions at some point. We should also figure out what is the best way to assure compatible @types/express-session and @types/express is used. Typically I would use peerDependencies but I am not sure that makes sense for types.

kleydon commented 3 years ago

@wSedlacek Given that there are breaking changes here: What do you think about addressing the expires -> expiresAt consistency issue, in conjunction? Do you think it would it make sense to "lump" these dislocations into one set of breaking changes - or do you think it would confuse the issue / complicate things?

wSedlacek commented 3 years ago

As far as locking, I mean for Prisma and express-session. They are currently set to * which was good when Prisma was updating so frequently without breaking changes but at this point older versions of Prisma won't work and future versions may or may not work so it might be time to be more strict about the versioning.

Supporting older typings seems fruitless. Our primary two dependencies, Prisma and Express-session, shouldn't have a ton of other dependencies in the way that a framework like angular or react would so it is quite unlikely that someone would be stuck on an older version of one of those dependencies and still want the latest version of our library. And even if we do want to support this I think it should be done though multiple branches that each correspond to the version of the library they are tracking. Ie our version 2.14.x would be tracking Prisma 2.14.x and express-session 1.7.x and then on the next prism release we would update our express-session dependency as well and support two separate branches for X months. (take a look at the angular release model and how they track the type script supported version for my inspiration)

In any case, supporting multiple versions of dependant packages is a lot of work and if we are going to pursue that then we would likely want at least one other contributor. I don't think it's in the card for us right now.

As far as assuring type dependencies versions, I haven't seen any libraries do this before so we likely will need to do some research on how to do this properly. I don't have anymore insight into this at this time, but maybe we should open an issue and mark it as help wanted to further discuss it with the community. Using peerDependencies uses the same version in downstream application and provides a warning to the developer if they don't match. Where as a dependency would use a locked version for our package but allow other packages in the project to use different versions. It could potentially cause conflicts I believe.

Running test against updated dependencies could be done via dependbot or something similar that makes PR to update your dependencies. I believe by default it is setup for just security updates but there might be a way to change that to any version update.

As far as the breaking change, this change purely relates to the dependencies and anyone updating will need to update their typing packages for express and express-session. No changes in their code should be necessary, at least not for our library, but @types/express may need to be removed completely and installed again OR the package-lock file may need to be regenerated. Given the nature of this particular breaking change, I don't think breaking changes involving code changes needed by the developer are warranted. I more or less want to wait until we need to have the developer change their prisma.schema for something else, and this particular change doesn't require that.

wSedlacek commented 3 years ago

I am going to go ahead and release these changes.

kleydon commented 3 years ago

:tada: This PR is included in version 2.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

kleydon commented 3 years ago

@wSedlacek Thanks for doing the release.