Merge branch 'performance_monitoring' of github.com:angular/angularfi…

…re2 into performance_monitoring

.travis.yml

@@ -1,7 +1,7 @@
 language: node_js
 sudo: false
 node_js:
-- 'node'
+- 'lts/*'
 
 addons:
   chrome: stable

docs/firestore/collections.md

@@ -73,13 +73,13 @@ interface DocumentSnapshot {
 
 There are multiple ways of streaming collection data from Firestore. 
 
-### `valueChanges()`
+### `valueChanges({idField?: string})`
 
-**What is it?** - The current state of your collection. Returns an Observable of data as a synchronized array of JSON objects. All Snapshot metadata is stripped and just the method provides only the data.
+**What is it?** - The current state of your collection. Returns an Observable of data as a synchronized array of JSON objects. All Snapshot metadata is stripped and just the document data is included. Optionally, you can pass an options object with an `idField` key containing a string. If provided, the returned JSON objects will include their document ID mapped to a property with the name provided by `idField`.  
 
 **Why would you use it?** - When you just need a list of data. No document metadata is attached to the resulting array which makes it simple to render to a view.
 
-**When would you not use it?** - When you need a more complex data structure than an array or you need the `id` of each document to use data manipulation methods. This method assumes you either are saving the `id` to the document data or using a "readonly" approach.
+**When would you not use it?** - When you need a more complex data structure than an array.
 
 **Best practices** - Use this method to display data on a page. It's simple but effective. Use `.snapshotChanges()` once your needs become more complex.
 

karma.conf.js

@@ -30,13 +30,15 @@ module.exports = function(config) {
       'node_modules/firebase/firebase-database.js',
       'node_modules/firebase/firebase-firestore.js',
       'node_modules/firebase/firebase-functions.js',
+      'node_modules/firebase/firebase-performance.js',
       'node_modules/firebase/firebase-storage.js',
       'dist/packages-dist/bundles/core.umd.{js,map}',
       'dist/packages-dist/bundles/auth.umd.{js,map}',
       'dist/packages-dist/bundles/database.umd.{js,map}',
       'dist/packages-dist/bundles/firestore.umd.{js,map}',
       'dist/packages-dist/bundles/functions.umd.{js,map}',
       'dist/packages-dist/bundles/storage.umd.{js,map}',
+      'dist/packages-dist/bundles/performance.umd.{js,map}',
       'dist/packages-dist/bundles/database-deprecated.umd.{js,map}',
       'dist/packages-dist/bundles/test.umd.{js,map}',
     ],

package.json

@@ -35,8 +35,7 @@
     "@angular/core": ">=6.0.0 <8",
     "@angular/platform-browser": ">=6.0.0 <8",
     "@angular/platform-browser-dynamic": ">=6.0.0 <8",
-    "@firebase/performance": "file:firebase-performance-0.0.3.tgz",
-    "firebase": "^5.5.0",
+    "firebase": ">= 5.5.0 <7",
     "rxjs": "^6.0.0",
     "ws": "^3.3.2",
     "xhr2": "^0.1.4",
@@ -47,9 +46,9 @@
     "utf-8-validate": "~4.0.0"
   },
   "devDependencies": {
+    "@angular/animations": ">=6.0.0 <8",
     "@angular/compiler-cli": ">=6.0.0 <8",
     "@angular/platform-server": ">=6.0.0 <8",
-    "@angular/animations": ">=6.0.0 <8",
     "@types/jasmine": "^2.5.36",
     "@types/request": "0.0.30",
     "concurrently": "^2.2.0",

src/core/firebase.app.module.ts

@@ -1,7 +1,5 @@
 import { InjectionToken, NgModule, Optional } from '@angular/core';
-import { app, auth, database, firestore, functions, messaging, storage } from 'firebase/app';
-import { PerformanceController } from '@firebase/performance/dist/src/controllers/perf';
-
+import { auth, database, firestore, functions, messaging, storage } from 'firebase/app';
 // @ts-ignore (https://github.com/firebase/firebase-js-sdk/pull/1206)
 import firebase from 'firebase/app'; // once fixed can pull in as "default as firebase" above
 
@@ -21,16 +19,13 @@ export type FirebaseFunctions = functions.Functions;
 
 // Have to implement as we need to return a class from the provider, we should consider exporting
 // this in the firebase/app types as this is our highest risk of breaks
-export class FirebaseApp implements app.App {
+export class FirebaseApp {
     name: string;
     options: {};
     auth: () => FirebaseAuth;
-    // app.App database() doesn't take a databaseURL arg in the public types?
     database: (databaseURL?: string) => FirebaseDatabase;
-    // automaticDataCollectionEnabled is now private? _automaticDataCollectionEnabled?
-    // automaticDataCollectionEnabled: true,
     messaging: () => FirebaseMessaging;
-    performance: () => PerformanceController;
+    performance: () => any; // SEMVER: once >= 6 import performance.Performance
     storage: (storageBucket?: string) => FirebaseStorage;
     delete: () => Promise<void>;
     firestore: () => FirebaseFirestore;

src/firestore/collection/collection.spec.ts

@@ -1,7 +1,6 @@
 import { FirebaseApp, AngularFireModule } from '@angular/fire';
 import { AngularFirestore } from '../firestore';
 import { AngularFirestoreModule } from '../firestore.module';
-import { AngularFirestoreDocument } from '../document/document';
 import { AngularFirestoreCollection } from './collection';
 import { QueryFn } from '../interfaces';
 import { Observable, BehaviorSubject, Subscription } from 'rxjs';
@@ -30,7 +29,7 @@ describe('AngularFirestoreCollection', () => {
     TestBed.configureTestingModule({
       imports: [
         AngularFireModule.initializeApp(COMMON_CONFIG),
-        AngularFirestoreModule.enablePersistence()
+        AngularFirestoreModule.enablePersistence({synchronizeTabs: true})
       ]
     });
     inject([FirebaseApp, AngularFirestore], (_app: FirebaseApp, _afs: AngularFirestore) => {
@@ -39,8 +38,8 @@ describe('AngularFirestoreCollection', () => {
     })();
   });
 
-  afterEach(async (done) => {
-    await app.delete();
+  afterEach(done => {
+    app.delete();
     done();
   });
 
@@ -70,6 +69,19 @@ describe('AngularFirestoreCollection', () => {
 
     });
 
+    it('should optionally map the doc ID to the emitted data object', async (done: any) => {
+      const ITEMS = 1;
+      const { ref, stocks, names } = await collectionHarness(afs, ITEMS);
+      const idField = 'myCustomID';
+      const sub = stocks.valueChanges({idField}).subscribe(data => {
+        sub.unsubscribe();
+        const stock = data[0];
+        expect(stock[idField]).toBeDefined();
+        expect(stock).toEqual(jasmine.objectContaining(FAKE_STOCK_DATA));
+        deleteThemAll(names, ref).then(done).catch(fail);
+      })
+    });
+
     it('should handle multiple subscriptions (hot)', async (done: any) => {
       const ITEMS = 4;
       const { randomCollectionName, ref, stocks, names } = await collectionHarness(afs, ITEMS);

src/firestore/collection/collection.ts

@@ -103,13 +103,29 @@ export class AngularFirestoreCollection<T=DocumentData> {
 
   /**
    * Listen to all documents in the collection and its possible query as an Observable.
+   * 
+   * If the `idField` option is provided, document IDs are included and mapped to the
+   * provided `idField` property name.
+   * @param options
    */
-  valueChanges(): Observable<T[]> {
+  valueChanges(): Observable<T[]>
+  valueChanges({}): Observable<T[]>
+  valueChanges<K extends string>(options: {idField: K}): Observable<(T & { [T in K]: string })[]>
+  valueChanges<K extends string>(options: {idField?: K} = {}): Observable<T[]> {
     const fromCollectionRef$ = fromCollectionRef<T>(this.query);
     const scheduled$ = this.afs.scheduler.runOutsideAngular(fromCollectionRef$);
     return this.afs.scheduler.keepUnstableUntilFirst(scheduled$)
       .pipe(
-        map(actions => actions.payload.docs.map(a => a.data()))
+        map(actions => actions.payload.docs.map(a => {
+          if (options.idField) {
+            return { 
+              ...a.data() as Object, 
+              ...{ [options.idField]: a.id } 
+            } as T & { [T in K]: string };
+          } else {
+            return a.data()
+          }
+        }))
       );
   }
 

src/firestore/document/document.spec.ts

@@ -19,7 +19,7 @@ describe('AngularFirestoreDocument', () => {
     TestBed.configureTestingModule({
       imports: [
         AngularFireModule.initializeApp(COMMON_CONFIG),
-        AngularFirestoreModule.enablePersistence()
+        AngularFirestoreModule.enablePersistence({synchronizeTabs: true})
       ]
     });
     inject([FirebaseApp, AngularFirestore], (_app: FirebaseApp, _afs: AngularFirestore) => {
@@ -28,8 +28,8 @@ describe('AngularFirestoreDocument', () => {
     })();
   });
 
-  afterEach(async (done) => {
-    await app.delete();
+  afterEach(done => {
+    app.delete();
     done();
   });
 

src/firestore/firestore.spec.ts

@@ -23,7 +23,7 @@ describe('AngularFirestore', () => {
     TestBed.configureTestingModule({
       imports: [
         AngularFireModule.initializeApp(COMMON_CONFIG),
-        AngularFirestoreModule.enablePersistence({experimentalTabSynchronization: true})
+        AngularFirestoreModule.enablePersistence({synchronizeTabs: true})
       ]
     });
     inject([FirebaseApp, AngularFirestore], (_app: FirebaseApp, _afs: AngularFirestore) => {
@@ -116,7 +116,8 @@ describe('AngularFirestore with different app', () => {
   });
 
   afterEach(done => {
-    app.delete().then(done, done.fail);
+    app.delete();
+    done();
   });
 
   describe('<constructor>', () => {
@@ -155,6 +156,11 @@ describe('AngularFirestore without persistance', () => {
     })();
   });
 
+  afterEach(done => {
+    app.delete();
+    done();
+  });
+
   it('should not enable persistence', (done) => {
     afs.persistenceEnabled$.subscribe(isEnabled => {
       expect(isEnabled).toBe(false);

src/performance/package.json

@@ -22,8 +22,7 @@
     "@angular/core": "ANGULAR_VERSION",
     "@angular/platform-browser": "ANGULAR_VERSION",
     "@angular/platform-browser-dynamic": "ANGULAR_VERSION",
-    "@firebase/app": "FIREBASE_APP_VERSION",
-    "@firebase/messaging": "FIREBASE_MESSAGING_VERSION",
+    "firebase": "FIREBASE_VERSION",
     "rxjs": "RXJS_VERSION",
     "zone.js": "ZONEJS_VERSION"
   },

src/performance/performance.module.ts

@@ -1,7 +1,7 @@
 import { NgModule } from '@angular/core';
 import { AngularFirePerformance } from './performance';
 
-import 'firebase/perf';
+import 'firebase/performance';
 
 @NgModule({
   providers: [ AngularFirePerformance ]

src/performance/performance.spec.ts

@@ -1 +1,36 @@
-export {}
+import { TestBed, inject } from '@angular/core/testing';
+import { FirebaseApp, AngularFireModule } from '@angular/fire';
+import { AngularFirePerformance, AngularFirePerformanceModule } from '@angular/fire/performance';
+import { COMMON_CONFIG } from './test-config';
+
+describe('AngularFirePerformance', () => {
+  let app: FirebaseApp;
+  let afp: AngularFirePerformance;
+
+  beforeEach(() => {
+    TestBed.configureTestingModule({
+      imports: [
+        AngularFireModule.initializeApp(COMMON_CONFIG),
+        AngularFirePerformanceModule
+      ]
+    });
+    inject([FirebaseApp, AngularFirePerformance], (app_: FirebaseApp, _perf: AngularFirePerformance) => {
+      app = app_;
+      afp = _perf;
+    })();
+  });
+
+  afterEach(done => {
+    app.delete();
+    done();
+  });
+
+  it('should be exist', () => {
+    expect(afp instanceof AngularFirePerformance).toBe(true);
+  });
+
+  it('should have the Performance instance', () => {
+    expect(afp.performance).toBeDefined();
+  });
+
+});

src/performance/performance.ts

@@ -1,9 +1,9 @@
-import { Injectable, Inject, Optional, NgZone, ApplicationRef } from '@angular/core';
+import { Injectable, NgZone, ApplicationRef, InjectionToken, Inject, Optional } from '@angular/core';
 import { Observable } from 'rxjs';
-import { filter, tap, take } from 'rxjs/operators';
-import { FirebaseOptions, FirebaseAppConfig } from '@angular/fire';
-import { FirebaseOptionsToken, FirebaseNameOrConfigToken, _firebaseAppFactory } from '@angular/fire';
-import { PerformanceController } from '@firebase/performance/dist/src/controllers/perf';
+import { first, tap } from 'rxjs/operators';
+import { performance } from 'firebase/app';
+
+export const AUTOMATICALLY_TRACE_CORE_NG_METRICS = new InjectionToken<boolean>('angularfire2.performance.auto_trace');
 
 export type TraceOptions = {
   metrics: {[key:string]: number},
@@ -16,26 +16,25 @@ export type TraceOptions = {
 @Injectable()
 export class AngularFirePerformance {
 
-  performance: PerformanceController;
+  performance: performance.Performance;
 
   constructor(
-    @Inject(FirebaseOptionsToken) options:FirebaseOptions,
-    @Optional() @Inject(FirebaseNameOrConfigToken) nameOrConfig:string|FirebaseAppConfig|null|undefined,
+    @Optional() @Inject(AUTOMATICALLY_TRACE_CORE_NG_METRICS) automaticallyTraceCoreNgMetrics:boolean|null,
     appRef: ApplicationRef,
     private zone: NgZone
   ) {
 
-    this.performance = zone.runOutsideAngular(() => {
-      const app = _firebaseAppFactory(options, nameOrConfig);
-      return app.performance();
-    });
+    this.performance = zone.runOutsideAngular(() => performance());
 
-    // TODO detirmine more built in metrics
-    appRef.isStable.pipe(
-      this.traceComplete('isStable'),
-      filter(it => it),
-      take(1)
-    ).subscribe();
+    if (automaticallyTraceCoreNgMetrics != false) {
+
+      // TODO detirmine more built in metrics
+      appRef.isStable.pipe(
+        this.traceComplete('isStable'),
+        first(it => it)
+      ).subscribe();
+
+    }
 
   }
 

src/performance/test-config.ts

@@ -3,6 +3,8 @@ export const COMMON_CONFIG = {
   apiKey: "AIzaSyBVSy3YpkVGiKXbbxeK0qBnu3-MNZ9UIjA",
   authDomain: "angularfire2-test.firebaseapp.com",
   databaseURL: "https://angularfire2-test.firebaseio.com",
+  projectId: "angularfire2-test",
   storageBucket: "angularfire2-test.appspot.com",
-  messagingSenderId: "920323787688"
+  messagingSenderId: "920323787688",
+  appId: "1:920323787688:web:2253a0e5eb5b9a8b"
 };

src/root.spec.js

@@ -13,6 +13,7 @@ export * from './packages-dist/database/list/snapshot-changes.spec';
 export * from './packages-dist/database/list/state-changes.spec';
 export * from './packages-dist/database/list/audit-trail.spec';
 export * from './packages-dist/storage/storage.spec';
+export * from './packages-dist/performance/performance.spec';
 //export * from './packages-dist/messaging/messaging.spec';
 
 // // Since this a deprecated API, we run on it on manual tests only

src/storage/ref.ts

@@ -9,6 +9,7 @@ export interface AngularFireStorageReference {
   delete(): Observable<any>;
   child(path: string): any;
   updateMetatdata(meta: SettableMetadata): Observable<any>;
+  updateMetadata(meta: SettableMetadata): Observable<any>;
   put(data: any, metadata?: UploadMetadata | undefined): AngularFireUploadTask;
   putString(data: string, format?: string | undefined, metadata?: UploadMetadata | undefined): AngularFireUploadTask;
 }
@@ -33,6 +34,7 @@ export function createStorageRef(ref: Reference, scheduler: FirebaseZoneSchedule
     delete: () => from(ref.delete()),
     child: (path: string) => createStorageRef(ref.child(path), scheduler),
     updateMetatdata: (meta: SettableMetadata) => from(ref.updateMetadata(meta)),
+    updateMetadata: (meta: SettableMetadata) => from(ref.updateMetadata(meta)),
     put: (data: any, metadata?: UploadMetadata) => {
       const task = ref.put(data, metadata);
       return createUploadTask(task);

src/tsconfig.json

@@ -17,6 +17,7 @@
       "@angular/fire/functions": ["./functions"],
       "@angular/fire/storage": ["./storage"],
       "@angular/fire/messaging": ["./messaging"],
+      "@angular/fire/performance": ["./performance"],
       "@angular/fire/database-deprecated": ["./database-deprecated"]
     },
     "rootDir": ".",

tools/build.js

@@ -23,7 +23,7 @@ const GLOBALS = {
   'firebase/messaging': 'firebase',
   'firebase/firestore': 'firebase',
   'firebase/functions': 'firebase',
-  'firebase/perf': 'firebase',
+  'firebase/performance': 'firebase',
   'firebase/storage': 'firebase',
   '@angular/fire': 'angularfire2',
   '@angular/fire/auth': 'angularfire2.auth',