fix: Remove `router` dependency as it comes from Express that is marked as peerDependency now
Remove router dependency and mark express as peer dependency. This way we get rid of the security warnings when we npm install this package. We also allow the use of the Router that comes with whatever express version the host application has.
Removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected], npm/[email protected]
OMG thank you for this! I have not reviewed it yet, but I literally opened a tab in my terminal with all the right files to fix this last week and just never got back to it. I will fully review it in the morning, but just wanted to drop a note of appreciation before then.
EDIT: Oh wait, took a first glance and am not sure this is enough. The tests are failing because of some regexp changes, which is what I had been looking at last week. I know this is a good change, but I am not sure it will be enough to support both latest versions of 4.x and also 5.
I guess I should add, for the vuln report, we obviously have to open those CVE's, but it only impacts a subset of route formats and none of the most common ones. So while it is important to update, as long as you read the CVE and ensure your app is not vulnerable to the ReDOS then you should be fine on your express version.
If you want to read about the security issues we discovered while getting the project back on it's feet you can read about that here: https://expressjs.com/2024/10/22/security-audit-milestone-achievement.html
The failing tests should be fixed by PR #66
@wesleytodd friendly ping on this one, including the test fix in #66
Upgrading to Express v5 is a larger effort, and requires us to maintain 2 branches for this project. The reasons why I think it requires us to have 2 branches:
- A lot of
if... else...forks in thelibfolder will make the project unmaintainable - Express v5 requires another set of dependencies, especially with regards to
path-to-regexpandrouter - The path routes matching syntax in the tests need to be different for the regex routes that involve a wildcard
*
cc @wesleytodd for comments
I will close this PR for now.