Skip to content

Conversation

@Hoppi164
Copy link
Contributor

Description

Bug Fix
When using firebase emulators Realtime Database throws an error due to app.options not existing.
I've found adding an extra check to the database constructor stops this error, and results in the emulator starting up correctly.

Code sample

Instead of
if (app && app.options.databaseURL.startsWith('http:'))
I've added
if ( app && app.options && app.options.databaseURL && app.options.databaseURL.startsWith('http:') )

Note: I've deliberately not used the modern ES6 optional chaining.
This syntax would be much cleaner, but is not supported by your lowest node version.
app?.options?.databaseURL?.startsWith('http:')

Copy link
Contributor

@taeold taeold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your great contribution!

Can you add an entry in the CHANGELOG to describe the bug fix?

@Hoppi164 Hoppi164 requested a review from taeold January 14, 2022 13:04
Copy link
Contributor

@taeold taeold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more change and we should 🚢

Co-authored-by: Daniel Lee <taeold@gmail.com>
@Hoppi164
Copy link
Contributor Author

No problem.
Let me know if there's anything else you need me to change :)

@taeold
Copy link
Contributor

taeold commented Jan 17, 2022

@Hoppi164 It looks like the linter is failing - can you run npm run lint or npm run format to fix the tests?

@Hoppi164
Copy link
Contributor Author

Hi @taeold I've run format:fix and removed the extra spacing i had on the if statement which the linter didn't like.
I've pushed this as another commit.

(note: there was a few other lint warnings in unrelated files, i've not corrected these as it seems out-of-scope for this PR)

@Hoppi164 Hoppi164 requested a review from taeold January 19, 2022 10:45
@taeold
Copy link
Contributor

taeold commented Jan 19, 2022

lgtm - thank you for your contribution (and your patience!)

@Hoppi164
Copy link
Contributor Author

No problem at all!
and thanks for all your help 😄
Let me know if there's anything else you need me to do..

@taeold taeold merged commit 7645a69 into firebase:master Jan 20, 2022
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

Successfully merging this pull request may close these issues.

2 participants