Skip to content

clients(viewer): upgrade to firebase 9 #13115

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.

Sign up for GitHub

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

Jump to bottom
Merged
connorjclark merged 8 commits into from
Sep 24, 2021
Merged

clients(viewer): upgrade to firebase 9 #13115

connorjclark merged 8 commits into from
Sep 24, 2021

Conversation

@connorjclark
Copy link
Collaborator

@connorjclark connorjclark commented Sep 23, 2021

  • firebase 4.1.2 -> 9.0.2
  • get firebase from npm instead of cdn–saves additional origin connect, treeshakes (saves ~25 kB), and gives us easier types
  • some analytics features enabled in firebase, idk, didn't look into that
  • did some refactoring (deleting unused code, using async/await for readability)

@connorjclark connorjclark requested a review from a team as a code owner September 23, 2021 20:15
@connorjclark connorjclark requested review from patrickhulce and removed request for a team September 23, 2021 20:15
@google-cla google-cla bot added the cla: yes label Sep 23, 2021
patrickhulce
patrickhulce approved these changes Sep 23, 2021
View reviewed changes
lighthouse-viewer/app/src/firebase-auth.js
* Signs the user out.
* @return {Promise<void>}
*/
signOut() {
Copy link
Collaborator

@patrickhulce patrickhulce Sep 23, 2021

Choose a reason for hiding this comment

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

unused?

Copy link
Collaborator Author

@connorjclark connorjclark Sep 23, 2021

Choose a reason for hiding this comment

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

yes

@brendankenny
Copy link
Contributor

brendankenny commented Sep 23, 2021

  • some analytics features enabled in firebase, idk, didn't look into that

what does this mean?

@connorjclark
Copy link
Collaborator Author

connorjclark commented Sep 23, 2021

w/e measurementId (new property, but needs newer sdk) enabled. https://firebase.google.com/docs/analytics/get-started?platform=web

@brendankenny
Copy link
Contributor

brendankenny commented Sep 23, 2021

seems like we should turn it off?

@connorjclark
Copy link
Collaborator Author

connorjclark commented Sep 23, 2021

some analytics features enabled in firebase, idk, didn't look into that

OK, looked into it. it's not enabled. (firebase/analytics is not used)

@connorjclark connorjclark merged commit f07c84a into master Sep 24, 2021
@connorjclark connorjclark deleted the firebase-update branch September 24, 2021 18:10
@brendankenny brendankenny mentioned this pull request Oct 12, 2021
Closed
@brendankenny brendankenny mentioned this pull request Jan 4, 2022
Merged
@MaxMood96 MaxMood96 mentioned this pull request Nov 30, 2023
Open
@HijadelUFO HijadelUFO mentioned this pull request Dec 1, 2023
Open
@thexdesk thexdesk mentioned this pull request Dec 1, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@patrickhulce patrickhulce patrickhulce approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@patrickhulce patrickhulce

Labels

cla: yes waiting4reviewer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@connorjclark @brendankenny @patrickhulce @devtools-bot