chore: upgrade to express v5.x#95
Conversation
Updates package.json range to include express v5.x Resolves: okta#89
Fix e2e test format to support express 5 Fix test to support okta sign in page that does not display until the user submits their username
|
I have signed and submitted the cla to cla@okta.com. Should be all set! |
|
@Tiuipuv Looking into this some more, I think for this change to make sense this needs to be a breaking change that moves As written, this could cause |
|
Yeah that's fair. Your core lib works with either, but your test harness depends on a specific version. I was thinking the lockfile has you covered there, but I could see it being dangerous/misleading for those who install w/o using the lockfile, or just maintaining the lockfile in general. What are your thoughts on dropping express v4 and bumping this lib to 6.0.0? |
I'm in favor of this. This is pretty much what I was picturing |
|
Ok cool, I'll work on that. Do you want me to update the Release status table in README.md to v6? |
|
Yes, please |
BREAKING CHANGE: Set package to only use express v5 Add a migration guide in the README.
|
Alright, should be set. Restricted it down to just v5. Added a small blurb about migrating (almost no-one is affected unless they use removed features in the custom callback handlers), and also backfilled the migration guide for older versions based on what I saw in your git releases + old commit history. |
|
Yeah makes sense. What are your thoughts on including the readme migration guide from this (v3 to v6) branch on yours? Adjusting the comment for v5 to v6 of course. Following semver is quite helpful to have the migration guide to make sure a semver major is not breaking for my/a consumers app. Also, it might be helpful for other contributors to add the await password field existence for e2e tests, as many tenants are configured to require a username submit first before the password field is shown on the dom. Thanks for all your help and review on this! |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Currently, the package does not support express v5.
Issue Number: #89
What is the new behavior?
This allows installers to
npm update expressin their app to v5.X, resolving issues for all consumers who are working on (or have completed) the migration to the new version of express.Does this PR introduce a breaking change?
Other information
Full breaking change checks were performed by running
npx codemod@latest @expressjs/v5-migration-recipe(0 library files needed modification). Checked for remaining issues (not codemod fixable) from official express upgrade tutorial here.After updating the e2e test server harness, Tests have fully passed for both v4.x and v5.x of express. Included 1 bug fix for authentication screens that require username to be submitted before the password prompt becomes available.
Reviewers