Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Commit

Permalink
Browse files Browse the repository at this point in the history
I initially tried to go to [email protected], the current
release, but it redoes the way that module imports work. It
seems straightforward to update our code to use the 'compat'
versions of the modules per the instructions at
https://firebase.google.com/docs/web/modular-upgrade, but
the third-party vuefire package doesn't seem to have been
updated yet (vuejs/vuefire#1128)
so we're stuck at version 8 for now.

Also switch from the deprecated @firebase/testing package to
@firebase/rules-unit-testing. Luckily, it seems to be a
drop-in replacement.
  • Loading branch information
derat committed Oct 26, 2021
1 parent 006ffcf commit 7118d5d
Show file tree
Hide file tree
Showing 9 changed files with 7,933 additions and 5,087 deletions.
2 changes: 1 addition & 1 deletion firestore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// https://firebase.google.com/docs/firestore/security/test-rules-emulator
// https://github.com/firebase/quickstart-nodejs/tree/master/firestore-emulator/typescript-quickstart

import * as firebase from '@firebase/testing';
import * as firebase from '@firebase/rules-unit-testing';

const projectId = `firestore-test-${Date.now()}`;

Expand Down
12,817 changes: 7,823 additions & 4,994 deletions package-lock.json

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"acorn": "^7.1.1",
"browserslist": "^4.17.2",
"core-js": "^3.4.4",
"firebase": "^7.14.5",
"firebase": "^8.10.0",
"firebaseui": "^4.0.0",
"humanize-duration": "^3.20.1",
"register-service-worker": "^1.6.2",
Expand All @@ -34,11 +34,11 @@
"vue-property-decorator": "^8.1.0",
"vue-router": "^3.0.7",
"vue-the-mask": "^0.11.1",
"vuefire": "^2.1.0",
"vuefire": "^2.2.5",
"vuetify": "^2.2.18"
},
"devDependencies": {
"@firebase/testing": "^0.15.1",
"@firebase/rules-unit-testing": "^1.3.15",
"@mdi/font": "^4.4.95",
"@types/humanize-duration": "^3.18.0",
"@types/jest": "^24.0.17",
Expand All @@ -58,7 +58,7 @@
"eslint": "^6.6.0",
"eslint-plugin-vue": "^6.2.2",
"eslint-plugin-vuetify": "^1.0.0-beta.6",
"firebase-tools": "^7.2.4",
"firebase-tools": "^9.21.0",
"flush-promises": "^1.0.2",
"jest": "^24.9.0",
"nightwatch": "^1.7.11",
Expand Down
9 changes: 5 additions & 4 deletions src/firebase/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
// This file exports methods that can be used to load different parts of
// Firebase asynchronously.

import type firebase from 'firebase';

// Loads the core Firebase library asynchronously.
// Outside code should only call this if it needs access to Firebase's weird
// constant values, e.g. firebase.auth.GoogleAuthProvider or
// firebase.firestore.FieldValue.delete().
// constant values, e.g. firebase.firestore.FieldValue.delete().
export function getFirebase() {
return import(/* webpackChunkName: "firebase-app" */ 'firebase/app');
}
Expand Down Expand Up @@ -52,9 +53,9 @@ export function getFirestore(): Promise<firebase.firestore.Firestore> {
.then(() => {
persistence = FirestorePersistence.ENABLED;
})
.catch(err => {
.catch((err) => {
persistence = FirestorePersistence.DISABLED;
import('@/log').then(log => {
import('@/log').then((log) => {
log.logError('firestore_persistence_failed', err);
});
});
Expand Down
16 changes: 9 additions & 7 deletions src/firebase/mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import Vue from 'vue';
import { deepCopy } from '@/testutil';

import type firebase from 'firebase';

type DocumentReference = firebase.firestore.DocumentReference;

// Firestore document data as property names to values.
Expand Down Expand Up @@ -144,7 +146,7 @@ class MockWriteBatch {
this._ops.push(new BatchUpdate(ref.path, props));
}
commit() {
return new Promise(resolve => {
return new Promise((resolve) => {
for (const op of this._ops) {
if (op instanceof BatchSet) {
MockFirebase.setDoc(op.path, op.data);
Expand Down Expand Up @@ -250,7 +252,7 @@ export const MockFirebase = new (class {
// Map of global mocks that should be passed to vue-test-utils's mount() or
// shallowMount().
mountMocks: Record<string, any> = {
$bind: function(name: string, ref: DocumentReference) {
$bind: function (name: string, ref: DocumentReference) {
if (!Object.prototype.hasOwnProperty.call(MockFirebase._docs, ref.path)) {
return Promise.reject(`No document at ${ref.path}`);
}
Expand All @@ -268,21 +270,21 @@ export const MockFirebase = new (class {
return Promise.resolve(data);
},

$unbind: function(name: string) {
$unbind: function (name: string) {
// Unregister the binding so the Vue's data property won't be updated.
const vm = this as Vue;
for (const path of Object.keys(MockFirebase._bindings)) {
MockFirebase._bindings[path] = MockFirebase._bindings[path].filter(
b => !(b.vm == vm && b.name == name)
(b) => !(b.vm == vm && b.name == name)
);
}
vm.$data[name] = null;
},
};
})();

export const MockEmailAuthProviderID = 'email';
export const MockGoogleAuthProviderID = 'google';
export const MockEmailAuthProviderID = 'password';
export const MockGoogleAuthProviderID = 'google.com';

jest.mock('firebase');
jest.mock('firebase/app', () => {
Expand All @@ -296,7 +298,7 @@ jest.mock('firebase/app', () => {
return MockFirebase.currentUser;
},
onAuthStateChanged: (observer: any) => {
new Promise(resolve => {
new Promise((resolve) => {
observer(MockFirebase.currentUser);
resolve();
});
Expand Down
8 changes: 5 additions & 3 deletions src/mixins/UserLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { Component, Watch } from 'vue-property-decorator';
import { User, Team } from '@/models';
import { getAuth, getFirebase, getFirestore } from '@/firebase';

import type firebase from 'firebase';

type DocumentReference = firebase.firestore.DocumentReference;

// The UserLoader mixin component automatically binds the Firestore user
Expand Down Expand Up @@ -75,13 +77,13 @@ export default class UserLoader extends Vue {
// Kick things off by loading the user doc.
this.userRef = firestore.collection('users').doc(this.user.uid);
this.$bind('userDoc', this.userRef)
.then(userSnap => {
.then((userSnap) => {
// If the user isn't on a team, then we're done. Otherwise,
// onTeamChanged will handle setting userReady after it loads the team
// doc.
if (userSnap['team'] === undefined) this.userLoaded = true;
})
.catch(err => {
.catch((err) => {
this.userLoadError = err;
});
}
Expand All @@ -98,7 +100,7 @@ export default class UserLoader extends Vue {
.then(() => {
this.userLoaded = true;
})
.catch(err => {
.catch((err) => {
this.userLoadError = err;
});
} else {
Expand Down
4 changes: 3 additions & 1 deletion src/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import type firebase from 'firebase';

// ClimbState describes whether and how a climber has climbed a route.
export enum ClimbState {
NOT_CLIMBED = 0,
Expand All @@ -20,7 +22,7 @@ export class ClimberInfo {
) {
this.initials = name
.split(/\s+/, 3)
.map(w => (w ? w[0] : ''))
.map((w) => (w ? w[0] : ''))
.join('')
.toUpperCase();
}
Expand Down
154 changes: 81 additions & 73 deletions src/views/Login.vue
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import { Component, Mixins } from 'vue-property-decorator';
import { logError, logInfo } from '@/log';
import { getAuth, getFirebase, getFirebaseUI, getFirestore } from '@/firebase';
import { getAuth, getFirebaseUI, getFirestore } from '@/firebase';
import Perf from '@/mixins/Perf';
Expand All @@ -46,84 +46,92 @@ export default class Login extends Mixins(Perf) {
completingLogin = false;
mounted() {
Promise.all([getAuth(), getFirebase(), getFirebaseUI()]).then(
([auth, firebase, firebaseui]) => {
// See https://github.com/firebase/firebaseui-web/issues/293.
let ui = firebaseui.auth.AuthUI.getInstance();
if (!ui) ui = new firebaseui.auth.AuthUI(auth);
Promise.all([getAuth(), getFirebaseUI()]).then(([auth, firebaseui]) => {
// See https://github.com/firebase/firebaseui-web/issues/293.
let ui = firebaseui.auth.AuthUI.getInstance();
if (!ui) ui = new firebaseui.auth.AuthUI(auth);
this.pendingRedirect = ui.isPendingRedirect();
if (!this.pendingRedirect) this.logReady('login_loaded');
this.pendingRedirect = ui.isPendingRedirect();
if (!this.pendingRedirect) this.logReady('login_loaded');
// As far as I can tell, there's no way to localize FirebaseUI's auth
// interface without loading a completely different version of the
// library from the CDN:
//
// https://github.com/firebase/firebaseui-web#localized-widget
// https://github.com/firebase/firebaseui-web/issues/9
// https://github.com/firebase/firebaseui-web/issues/242
//
// Best comment on that last bug:
//
// > > Each internationalized version is a separate build. Including all
// > > the versions in one file is not feasible.
//
// > Oh dear. Looks like you've used the wrong implementation architecture
// > to support i18n for SPA web apps!
//
// There *is* a |languageCode| field on firebase.auth.Auth, but I'm not
// sure what it does:
// https://firebase.google.com/docs/reference/js/firebase.auth.Auth.html#languagecode
ui.start('#firebaseui-auth-container', {
// Disable the account chooser, which is ugly and doesn't seem to work
// correctly with this logic (i.e. after signing out and trying to
// sign in with email, I just see a spinner):
// https://stackoverflow.com/q/37369929
credentialHelper: firebaseui.auth.CredentialHelper.NONE,
signInOptions: [
firebase.auth.GoogleAuthProvider.PROVIDER_ID,
firebase.auth.EmailAuthProvider.PROVIDER_ID,
],
callbacks: {
signInSuccessWithAuthResult: () => {
this.completingLogin = true;
// As far as I can tell, there's no way to localize FirebaseUI's auth
// interface without loading a completely different version of the
// library from the CDN:
//
// https://github.com/firebase/firebaseui-web#localized-widget
// https://github.com/firebase/firebaseui-web/issues/9
// https://github.com/firebase/firebaseui-web/issues/242
//
// Best comment on that last bug:
//
// > > Each internationalized version is a separate build. Including all
// > > the versions in one file is not feasible.
//
// > Oh dear. Looks like you've used the wrong implementation architecture
// > to support i18n for SPA web apps!
//
// There *is* a |languageCode| field on firebase.auth.Auth, but I'm not
// sure what it does:
// https://firebase.google.com/docs/reference/js/firebase.auth.Auth.html#languagecode
ui.start('#firebaseui-auth-container', {
// Disable the account chooser, which is ugly and doesn't seem to work
// correctly with this logic (i.e. after signing out and trying to
// sign in with email, I just see a spinner):
// https://stackoverflow.com/q/37369929
credentialHelper: firebaseui.auth.CredentialHelper.NONE,
signInOptions: [
// These correspond to the static variables
// firebase.auth.GoogleAuthProvider.PROVIDER_ID and
// firebase.auth.EmailAuthProvider.PROVIDER_ID (and I think also the
// ProviderId enum at
// https://firebase.google.com/docs/reference/js/auth.md#providerid).
// I can't figure out how to access any of these in [email protected]
// using the dynamic imports in @/firebase/index.ts. Worryingly, these
// seemed to formerly be 'google' and 'email', but if you use invalid
// strings you appear to get unclickable login buttons in the UI, so
// it seems easy to notice.
'google.com',
'password',
],
callbacks: {
signInSuccessWithAuthResult: () => {
this.completingLogin = true;
Promise.all([getAuth(), getFirestore()]).then(([auth, db]) => {
const user = auth.currentUser;
if (!user) throw new Error('Not logged in');
const ref = db.collection('users').doc(user.uid);
ref.get().then((snap) => {
if (snap.exists) {
// If the user has logged in before, send them to the routes
// view.
this.$router.replace('routes');
} else {
// Otherwise, create the doc using their display name and
// send them to the profile view. Note that the name is null
// when using the email provider.
const name = user.displayName || 'Unknown Climber';
logInfo('create_user', { name });
ref
.set({ name }, { merge: true })
.then(() => {
this.$router.replace('profile');
})
.catch((err) => {
this.$emit('error-msg', `Failed creating user: ${err}`);
logError('create_user_failed', err);
});
}
});
Promise.all([getAuth(), getFirestore()]).then(([auth, db]) => {
const user = auth.currentUser;
if (!user) throw new Error('Not logged in');
const ref = db.collection('users').doc(user.uid);
ref.get().then((snap) => {
if (snap.exists) {
// If the user has logged in before, send them to the routes
// view.
this.$router.replace('routes');
} else {
// Otherwise, create the doc using their display name and
// send them to the profile view. Note that the name is null
// when using the email provider.
const name = user.displayName || 'Unknown Climber';
logInfo('create_user', { name });
ref
.set({ name }, { merge: true })
.then(() => {
this.$router.replace('profile');
})
.catch((err) => {
this.$emit('error-msg', `Failed creating user: ${err}`);
logError('create_user_failed', err);
});
}
});
});
// Don't redirect automatically; we handle that above.
return false;
},
uiShown: () => this.recordEvent('loginShown'),
// Don't redirect automatically; we handle that above.
return false;
},
});
}
);
uiShown: () => this.recordEvent('loginShown'),
},
});
});
}
}
</script>
Expand Down
2 changes: 2 additions & 0 deletions src/views/Profile.vue
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,8 @@ import Perf from '@/mixins/Perf';
import Spinner from '@/components/Spinner.vue';
import UserLoader from '@/mixins/UserLoader';
import type firebase from 'firebase';
// Removes excessive/weird whitespace from |name|.
function cleanName(name: string): string {
return name.replace(/\s+/gm, ' ').trim();
Expand Down

0 comments on commit 7118d5d

Please sign in to comment.