meta: add express 5 as peer dependency

[InSantoshMahto]

Related Issues

What Does This PR Do?

This PR will resolve the peer dependency error fix for express v5

Added

Changed

Removed

Caveats/Problems/Issues

Checklist

• The issues that this PR fixes/closes have been mentioned above.
• What this PR adds/changes/removes has been explained.
• All tests (npm test) pass.
• All added/modified code has been commented, and
  methods/classes/constants/types have been annotated with TSDoc comments.
• If a new feature has been added or a bug has been fixed, tests have been
  added for the same.

[gamemaker1]

Hi @InSantoshMahto,

Thanks for taking the time to make the PR :)

However, Express v5 is still in beta - I think it would make more sense to update the peer dependency once v5.0.0 is officially released and tested with this library.

[gamemaker1]

Also, why does this change involve 57 files? Did you need to change something to make it work with Express v5?

Could you please undo the rename of the source/ folder and deletion of comments?

[InSantoshMahto]

Also, why does this change involve 57 files? Did you need to change something to make it work with Express v5?

I just rename the source directory to src to look the same as other projects. and remove some comments

[InSantoshMahto]

Hi @InSantoshMahto,

Thanks for taking the time to make the PR :)

However, Express v5 is still in beta - I think it would make more sense to update the peer dependency once v5.0.0 is officially released and tested with this library.

Can we make it available express-rate-limit@next so that we can use with express@next for V5

[gamemaker1]

Can we make it available express-rate-limit@next so that we can use with express@next for V5

I think we could do that. AFAIK usage with Express v5 should be possible even if the peer dependency is not updated though - npm only warns if peer dependencies are not met. Have you tested the library and checked if it works with the newer version without any changes?

[gamemaker1]

I just rename the source directory to src to look the same as other projects. and remove some comments

Could you please undo those changes? The comments are helpful for new contributors and I prefer keeping full form directory names in projects. Also, the tests are failing because of the incomplete rename.

[InSantoshMahto]

I just rename the source directory to src to look the same as other projects. and remove some comments

Could you please undo those changes? The comments are helpful for new contributors and I prefer keeping full form directory names in projects. Also, the tests are failing because of the incomplete rename.

sure

[InSantoshMahto]

Can we make it available express-rate-limit@next so that we can use with express@next for V5

I think we could do that. AFAIK usage with Express v5 should be possible even if the peer dependency is not updated though - npm only warns if peer dependencies are not met. Have you tested the library and checked if it works with the newer version without any changes?

Yes. work perfectly with waring while using npm. But while using pnpm I faced the issue. it break my docker build.

[gamemaker1]

But while using pnpm I faced the issue. it break my docker build.

What error do you receive?

In a test project, running pnpm install express@5.0.0-beta.1 gives me the following output:

Packages: +13 -12
+++++++++++++------------

Progress: resolved 295, reused 268, downloaded 8, added 13, done

dependencies:
- express 4.17.3
+ express 5.0.0-beta.1
- express-rate-limit 6.3.0
+ express-rate-limit 6.3.0

 WARN  Issues with peer dependencies found
.
└─┬ express-rate-limit
  └── ✕ unmet peer express@^4: found 5.0.0-beta.1

[InSantoshMahto]

But while using pnpm I faced the issue. it break my docker build.

What error do you receive?

In a test project, running pnpm install express@5.0.0-beta.1 gives me the following output:

Packages: +13 -12
+++++++++++++------------

Progress: resolved 295, reused 268, downloaded 8, added 13, done

dependencies:
- express 4.17.3
+ express 5.0.0-beta.1
- express-rate-limit 6.3.0
+ express-rate-limit 6.3.0

 WARN  Issues with peer dependencies found
.
└─┬ express-rate-limit
  └── ✕ unmet peer express@^4: found 5.0.0-beta.1

yes I also received error

[gamemaker1]

This is just a warning, and shouldn't result in a failure during installation.

Anyways, I don't see an issue in publishing a new version under the next tag. @nfriedly what do you think?

[nfriedly]

I think express 5.x compatibility is fine to declare in a regular semver minor release.

I would like to see us run our automated tests against both 4.x and 5.x then - but I think that could be as simple as adding a few lines to test/external/run-all-tests - basically, after the two npm test lines, do npm install express@5 and then npm test again.

[gamemaker1]

@InSantoshMahto could you please check the "allow maintainer edits" box on the right hand side on this page so I can push a commit updating the tests? Thanks!

[InSantoshMahto]

@InSantoshMahto could you please check the "allow maintainer edits" box on the right hand side on this page so I can push a commit updating the tests? Thanks!

Hey @gamemaker1 Already checked

[gamemaker1]

Ohk, thank you

[gamemaker1]

Looks good to me!

[gamemaker1]

@nfriedly Do take a look at the changes, the tests now run with express 4 and 5

[nfriedly]

I like it!

it might be good to add a couple of echo "running test $foo"; npm ls express sort of comments to run-all-tests, but that's more cosmetic than anything else.

[gamemaker1]

Sure, will do

[gamemaker1]

Added some colors 🎆

[gamemaker1]

Merging after fixing a merge conflict, will release a minor version by tomorrow :D

[gamemaker1]

Released as part of v6.4.0