-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix database:remove bug #3186
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
Fix database:remove bug #3186
Conversation
src/utils.ts
Outdated
| urlObj.hostname.includes("firebaseio.com") || | ||
| urlObj.hostname.includes("firebasedatabase.app") | ||
| ) { | ||
| if (urlObj.hostname.includes("firebase")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that FIREBASE_REALTIME_URL=https://firebaseio-staging.com will work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just list all three full domain names to be on the safe side. myfirebaseapp.example.com should be filtered out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to detect emulator specific url instead? It might be cleaner.
Are they always "localhost"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, in theory the emulator can listen on all sorts of hosts and ports. Common configurations include 127.0.0.1, localhost, but it can be changed by the user to LAN or even public IP addresses and domains, and I've seen those in the wild. We had a similar discussion re: RTDB emulator and the conclusion so far is not to try and detect emulator URLs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SG. I will just check for firebaseio or firebasedatabase if u think firebase is too general.
src/utils.ts
Outdated
| urlObj.hostname.includes("firebaseio.com") || | ||
| urlObj.hostname.includes("firebasedatabase.app") | ||
| ) { | ||
| if (urlObj.hostname.includes("firebase")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just list all three full domain names to be on the safe side. myfirebaseapp.example.com should be filtered out.
src/utils.ts
Outdated
| urlObj.hostname.includes("firebaseio.com") || | ||
| urlObj.hostname.includes("firebasedatabase.app") | ||
| ) { | ||
| if (urlObj.hostname.includes("firebase")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
In
listRemove.ts,const paths = Object.keys(res.body);fails ifres.bodyis null.This is causing errors when listing a path already deleted.
This bug was introduced in #2828.