From e18da0eb76b2ce48a3f38ac86bd9a27a4038e45b Mon Sep 17 00:00:00 2001 From: Jeff Cross Date: Fri, 28 Oct 2016 16:27:12 -0700 Subject: [PATCH] fix(Database): use Zone scheduler for object and list factories Since Firebase is adding WebSocket listeners before the Angular Zone exists, all callbacks when data is received are being executed outside of Angular's zone and are not triggering change detection. This fix uses the existing ZoneScheduler, which is already used in auth.onAuth, to cause the observables created by af.list() and af.object() to be emitted inside the current Zone at the time of calling .list() or .object(). See angular/angular#4603 for progress on making RxJS more zone-aware. Fixes #637 --- src/database/firebase_list_factory.spec.ts | 20 ++++++++++++++++++++ src/database/firebase_list_factory.ts | 5 ++++- src/database/firebase_object_factory.spec.ts | 19 +++++++++++++++++++ src/database/firebase_object_factory.ts | 6 +++++- 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/database/firebase_list_factory.spec.ts b/src/database/firebase_list_factory.spec.ts index 142517be1..0f7ce23cc 100644 --- a/src/database/firebase_list_factory.spec.ts +++ b/src/database/firebase_list_factory.spec.ts @@ -541,6 +541,26 @@ describe('FirebaseListFactory', () => { }); }); + it('should emit values in the observable creation zone', (done: any) => { + Zone.current.fork({ + name: 'newZone' + }) + .run(() => { + // Creating a new observable so that the current zone is captured. + subscription = FirebaseListFactory(`${rootDatabaseUrl}/questions`) + .filter(d => d + .map(v => v.$value) + .indexOf('in-the-zone') > -1) + .subscribe(data => { + expect(Zone.current.name).toBe('newZone'); + done(); + }); + }); + + ref.remove(() => { + ref.push('in-the-zone'); + }); + }); }); }); diff --git a/src/database/firebase_list_factory.ts b/src/database/firebase_list_factory.ts index 39fbcf0bc..723525180 100644 --- a/src/database/firebase_list_factory.ts +++ b/src/database/firebase_list_factory.ts @@ -2,6 +2,7 @@ import * as firebase from 'firebase'; import { AFUnwrappedDataSnapshot } from '../interfaces'; import { FirebaseListObservable } from './firebase_list_observable'; import { Observer } from 'rxjs/Observer'; +import { observeOn } from 'rxjs/operator/observeOn'; import { observeQuery } from './query_observable'; import { Query, FirebaseListFactoryOpts } from '../interfaces'; import * as utils from '../utils'; @@ -152,7 +153,9 @@ function firebaseListObservable(ref: firebase.database.Reference | firebase.data ref.off('child_changed', chgFn); } }); - return listObs; + + // TODO: should be in the subscription zone instead + return observeOn.call(listObs, new utils.ZoneScheduler(Zone.current)); } export function onChildAdded(arr:any[], child:any, prevKey:string): any[] { diff --git a/src/database/firebase_object_factory.spec.ts b/src/database/firebase_object_factory.spec.ts index 499329525..7d677d793 100644 --- a/src/database/firebase_object_factory.spec.ts +++ b/src/database/firebase_object_factory.spec.ts @@ -136,5 +136,24 @@ describe('FirebaseObjectFactory', () => { subscription.unsubscribe(); expect(firebaseSpy).toHaveBeenCalled(); }); + + it('should emit values in the observable creation zone', (done: any) => { + Zone.current.fork({ + name: 'newZone' + }) + .run(() => { + // Creating a new observable so that the current zone is captured. + subscription = FirebaseObjectFactory(`${rootDatabaseUrl}/questions/${i}`) + .filter(d => d.$value === 'in-the-zone') + .subscribe(data => { + expect(Zone.current.name).toBe('newZone'); + done(); + }); + }); + + ref.remove(() => { + ref.set('in-the-zone'); + }); + }); }); }); diff --git a/src/database/firebase_object_factory.ts b/src/database/firebase_object_factory.ts index 9c2a5371c..85a8b9eb8 100644 --- a/src/database/firebase_object_factory.ts +++ b/src/database/firebase_object_factory.ts @@ -1,5 +1,6 @@ import { FirebaseObjectObservable } from './index'; import { Observer } from 'rxjs/Observer'; +import { observeOn } from 'rxjs/operator/observeOn'; import * as firebase from 'firebase'; import * as utils from '../utils'; import { Query } from '../interfaces'; @@ -17,7 +18,7 @@ export function FirebaseObjectFactory ( isRef: () => ref = absoluteUrlOrDbRef }); - return new FirebaseObjectObservable((obs: Observer) => { + const objectObservable = new FirebaseObjectObservable((obs: Observer) => { let fn = ref.on('value', (snapshot: firebase.database.DataSnapshot) => { obs.next(preserveSnapshot ? snapshot : utils.unwrapMapFn(snapshot)) }, err => { @@ -26,4 +27,7 @@ export function FirebaseObjectFactory ( return () => ref.off('value', fn); }, ref); + + // TODO: should be in the subscription zone instead + return observeOn.call(objectObservable, new utils.ZoneScheduler(Zone.current)); }