Upgrade to Express v5 #2685
Upgrade to Express v5 #2685
Conversation
| @@ -50,7 +50,7 @@ const defaultGlobalMiddleware = new Map([ | |||
| ['cors', cors({ origin: config.allowedCORSOrigins })], | |||
| ['logger', logger('dev')], | |||
| ['express.json', express.json()], | |||
| ['express.urlencoded', express.urlencoded({ extended: false })], | |||
| ['express.urlencoded', express.urlencoded()], | |||
There was a problem hiding this comment.
{ extended: false } is now the default https://expressjs.com/en/guide/migrating-5.html#express.urlencoded
There was a problem hiding this comment.
Great stuff Carlos, I missed it in the code view for some reason 🤦
cprecioso
requested review from
infomiho and
sodic
and removed request for
infomiho
waspc/data/Generator/templates/server/src/auth/providers/config/email.ts
Show resolved
Hide resolved
waspc/data/Generator/templates/server/src/middleware/globalMiddleware.ts
Show resolved
Hide resolved
| @@ -17,7 +17,7 @@ | |||
| "devDependencies": { | |||
| "@tailwindcss/forms": "^0.5.10", | |||
| "@tailwindcss/typography": "^0.5.16", | |||
| "@types/express": "^4.17.13", | |||
| "@types/express": "^5.0.0", | |||
There was a problem hiding this comment.
We should update this in open-saas as well: https://github.com/wasp-lang/open-saas/blob/main/template/app/package.json#L35
There was a problem hiding this comment.
AFAIU we do that with the RC when we prepare for a release, right?
There was a problem hiding this comment.
You can do the change now and merge it before release or wait for the RC to do the change, I guess we don't have a rule about that 🤷
Context: Filip did this change #2691 and this PR wasp-lang/open-saas#419
There was a problem hiding this comment.
@cprecioso what did you decide to do?
There was a problem hiding this comment.
I'll prepare the PR now
EDIT: I think it's quite a small change actually and the compiler will guide us, better to do it when we update OpenSaas than to have such a simple PR left open for weeks
|
@infomiho if everything is fine, we should be good to go with merging (tho @Martinsos removed it from the sprint so idk) |
Oh we removed the PR only because there was issue already (with estimate and all), so to not have it duplicate! So it is still in the sprint, no worries. |
Description
Fixes #2593
Fixes #1632
Updates Express to v5
We're not impacted by most of the changes, only one thing:
We had a helper for that in our codebase (
handleRejection), which is now unnecessary. I was going to remove it but our code is dependent on this function in order to correctly typereqandres.So I renamed it to
defineHandlerand made it a no-op, while updating its types to Express v5, so that we can keep it as it was.Marked as a breaking change because users might had
reqandrescalls that used some of the removed methods (as explained in the migration guide).Select what type of change this PR introduces:
Update Waspc ChangeLog and version if needed
If you did a bug fix, new feature, or breaking change, that affects waspc, make sure you satisfy the following:
Update example apps if needed
If you did code changes and added a new feature or modified an existing feature, make sure you satisfy the following:
waspc/examples/todoAppas needed (updated modified feature or added new feature) and manually checked it works correctly.waspc/headless-test/examples/todoAppand its e2e tests as needed (updated modified feature and its tests or added new feature and new tests for it).