firebase-functions-rate-limiter
firebase-functions-rate-limiter copied to clipboard
Jblew
Support firebase-admin v11 & v12
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):
firebase-adminv10 andfirebase-functionsv3firebase-adminv11 andfirebase-functionsv4firebase-adminv12 andfirebase-functionsv4firebase-adminv12 andfirebase-functionsv5
@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.
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?
@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.
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!
@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!
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.