Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use new Store interface from express-rate-limit v6 #40

Open
gamemaker1 opened this issue Jan 18, 2022 · 4 comments
Open

Use new Store interface from express-rate-limit v6 #40

gamemaker1 opened this issue Jan 18, 2022 · 4 comments

Comments

@gamemaker1
Copy link

gamemaker1 commented Jan 18, 2022

Hi @okv,

I recently helped out with a Typescript rewrite for the express-rate-limit package, which also introduced a couple of changes to the Store interface:

As the external tests show, the store still works (without any changes) with v6 of express-rate-limit. However, implementing the newer Store interface allows for the following:

I would be happy to open/help out with a PR that implements the new Store interface.

Also, I see that #27 is still open and there is no @types/rate-limit-mongo package. Would you like this PR to also:

  1. Rewrite the library in Typescript, OR
  2. Add a type definition file (index.d.ts like what @gtmsingh has suggested in Typescript declaration #27 (comment)) so that Typescript users don't encounter errors while using this library.

Please let me know your thoughts on the above.

Thanks,
Vedant

// cc @nfriedly

@okv
Copy link
Contributor

okv commented Jan 19, 2022

Hey @gamemaker1, to be honest I'm not a big TypeScript fan. But everything except it sounds awesome 😃
I would keep the code in JS, but index.d.ts could be useful. Any assistance with reviewing #28 is highly appreciated.
I will try to take a closer look at all this next weekend.

@gamemaker1
Copy link
Author

Great, I'll start working on a PR this weekend! I'll also take a look at #28.

@FxllenCode
Copy link

Is this issue stale? Not to necrobump but it would be nice to support v6 👍

@gamemaker1
Copy link
Author

@FxllenCode yes, this issue is stale since I have not gotten the time to convert the store to by yet :(

Although ypu could send in a PR as per the discussion above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants