Skip to content

Commit

Permalink
fix(Database): use Zone scheduler for object and list factories
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jeffbcross committed Nov 2, 2016
1 parent 2c0a57f commit e18da0e
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 2 deletions.
20 changes: 20 additions & 0 deletions src/database/firebase_list_factory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
});

Expand Down
5 changes: 4 additions & 1 deletion src/database/firebase_list_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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[] {
Expand Down
19 changes: 19 additions & 0 deletions src/database/firebase_object_factory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
});
6 changes: 5 additions & 1 deletion src/database/firebase_object_factory.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -17,7 +18,7 @@ export function FirebaseObjectFactory (
isRef: () => ref = <firebase.database.Reference>absoluteUrlOrDbRef
});

return new FirebaseObjectObservable((obs: Observer<any>) => {
const objectObservable = new FirebaseObjectObservable((obs: Observer<any>) => {
let fn = ref.on('value', (snapshot: firebase.database.DataSnapshot) => {
obs.next(preserveSnapshot ? snapshot : utils.unwrapMapFn(snapshot))
}, err => {
Expand All @@ -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));
}

1 comment on commit e18da0e

@daslicht
Copy link

Choose a reason for hiding this comment

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

Is that the fix for this issue :
#637

How do I use this, please, or is it already present in the latest angular 2?
cheers Marc

Please sign in to comment.