search

Jblew / firebase-functions-rate-limiter

Js/ts library that allows you to set per-time, per-user or per-anything limits for calling Firebase cloud functions
MIT License
100 stars 15 forks source link

Support firebase-admin v11 & v12 #43

Open omgovich opened 1 year ago

omgovich commented 1 year ago

Motivation / Context

Hi! Thanks for the nice library!

My app (I believe I'm not alone) is working on the latest versions of firebase-admin and firebase-functions. The current firebase-functions-rate-limiter config is incompatible with them.

Description

In firebase-admin v11 & v12 Timestamp must be imported from another place (it is also there in v10, so it's back-compatible). Using HttpsError from firebase-functions v3 when the app is running on v4 is also causing errors.

I moved firebase-admin and firebase-functions from dependencies to peerDependencies and made the package work with prev and new versions of firebase packages: no matter which one is installed in a developer's project.

Using peerDependencies for that is the more correct way: check how all React libraries are configured. They support different major versions of React and don't limit developer to use the one that is declared in the package dependencies.

Testing Instructions / How This Has Been Tested

Tested locally with npm run testall with several package combinations installed (changed versions in devDependencies and reinstalled node_modules):

yorch commented 1 year ago

Any chance this could be merged soon?

omgovich commented 1 year ago

@yorch I published my fork on NPM as @omgovich/firebase-functions-rate-limiter

You can install and use it while the PR is not merged.

Jblew commented 1 year ago

Excellent work! 🚀 I am sorry for a period of unavailability. To explain it a little bit: I have graduated as a medical doctor two years ago and had to finally complete 13 month ,180h/mo internship at a hospital. Now I am back to software development and will take care of my projects :)

Unfortunately I cannot merge your work due to a conflict Can you please either (1) allow me editing this PR or (2) resolve the conflicts yourself?

omgovich commented 1 year ago

@Jblew Totally understand. Congratulations!)

I'm currently on vacation and will be able to fix conflicts after May 10th.

P.S. If you have time you could try resolving them — I checked the PR: seems like you are allowed to push.

omgovich commented 1 year ago

@Jblew I fixed all conflicts 🤟

spicemix commented 1 year ago

Just noting that firebase-admin@10 has a security issue, which I can't address fully until this is merged thank you!

And congrats on your MD!

M-a-c commented 9 months ago

@Jblew congrats on the MD as @spicemix said, this seems to be ready to merge, would be really nice to fix the security errors on the package 👍 , looking forward to the updates!

omgovich commented 2 months ago

Hello everyone! I have updated this PR and added support for firebase-admin v12. While the PR remains unmerged, my fork on NPM has all changes and is available for installation.

omgovich commented 2 weeks ago

firebase-functions v5 are now supported P.S. Available in v4.2 of my fork