Skip to content

Revamp environment check for firebase app #1757

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

Merged
merged 2 commits into from
May 7, 2019
Merged

Revamp environment check for firebase app #1757

merged 2 commits into from
May 7, 2019

Conversation

Feiyang1
Copy link
Member

@Feiyang1 Feiyang1 commented May 7, 2019

  1. Proper browser env detection. Fixed Angular with SSR firebase bug #1754
  2. Delay Nodejs detection and warning util initializeApp, so people have a chance to suppress it by setting logLevel in @firebase/logger

Copy link
Contributor

@hsubox76 hsubox76 left a comment

Choose a reason for hiding this comment

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

Especially because we're expecting people to have problems with Firebase Lite at first, I feel like we should make these warnings more discoverable, or make loglevel settings more discoverable somehow. I don't want users to have to come file an issue before they can be told to turn up loglevel to see if anything is wrong.

@Feiyang1
Copy link
Member Author

Feiyang1 commented May 7, 2019

Warning messages are shown by default, so they don't need to do anything unless they want to disable it. Is it your concern?
The FirebaseLite message can't be suppressed because it will execute before people have a chance to set log level. It is desirable because having firebase imported twice is always the wrong thing to do.

Copy link
Contributor

@hsubox76 hsubox76 left a comment

Choose a reason for hiding this comment

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

Oh, sorry, I thought it was the opposite, that warn loglevel had to be turned on. Looks good then.

@Feiyang1 Feiyang1 merged commit f6afa45 into master May 7, 2019
@Feiyang1 Feiyang1 deleted the fei-fixself branch May 7, 2019 16:36
@firebase firebase locked and limited conversation to collaborators Oct 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Angular with SSR firebase bug
2 participants