angular / angularfire

Angular + Firebase = ❤️
https://firebaseopensource.com/projects/angular/angularfire2
MIT License
7.66k stars 2.19k forks source link

snapshotChanges delivers data twice from cache after updating to "@angular/fire": "^6.1.0" and "firebase": "^8.0.2" #2662

Closed lukfel closed 3 years ago

lukfel commented 3 years ago

Version info

Angular: "~11.0.0",

Firebase: "^8.0.2",

AngularFire: "^6.1.0",

How to reproduce these conditions

1) I import the AngularFirestoreModule and set enablePersistence() for caching.

app.module.ts

...
import { AngularFirestoreModule } from '@angular/fire/firestore';

@NgModule({
  imports: [
    ...,
    AngularFirestoreModule.enablePersistence()
  ]
})
export class AppModule{}

2) I create a service that allows to receive the required data from Firestore with an optional query. Additionally I log how many documents are loaded and where they are loaded from (network or cache).

firestore.service.ts

  // Load data from firestore with optional query
  public readAll$(query?: QueryFn): Observable {
    return this.getCollection(query).snapshotChanges().pipe(map(actions =>
      actions.map(action => {
        this.getLoadingInfos(action.payload.doc.metadata.fromCache);
        return data;
      }),
    ));
  }

  // Log whether data was loaded from cache or network
  private getLoadingInfos(fromCache: boolean): void {
    console.log('LOAD ' + this.collectionName + '-Docs from ' + (fromCache ? 'CACHE' : 'NETWORK'));
  }

3) Then I´ve a start page where I load and display all active user documents (currently two).

start.component.ts

  ngOnInit(): void {
    this.users$ = this.firebaseService.readAll$(ref => ref.where('active', '==', true).limit(10));
  }

start.component.html

  <div *ngIf="users$ | async as users">
    <user-list [users]="users"></user-list>
  </div>

4) I also have a profile page where upvoted users can be viewed (currently two).

profile.component.ts

  ngOnInit(): void {
    this.upvotedUsers$ = this._userDB.readAll$(ref => ref.where('id', 'in', user.upvotedIds));
  }

profile.component.html

<div *ngIf="user$ | async as user">

  <mat-tab-group mat-stretch-tabs>
    <mat-tab label="Tab 1"></mat-tab>

    <mat-tab label="Tab 2">
      <!-- lazy load data with ng-template -->
      <ng-template matTabContent>
        <br>
        <div *ngIf="upvotedUsers$ | async as upvotedUsers; else upvotedUsersNotFound">
          <user-list [users]="upvotedUsers"></user-list>
        </div>
        <ng-template #upvotedUsersNotFound>nothing found</ng-template>
      </ng-template>
    </mat-tab>
  </mat-tab-group>

</div>

Sample data and security rules

Debug output

However, since updating from "@angular/fire": "^6.0.3" to "@angular/fire": "^6.1.0" and from "firebase": "^7.22.0" to "firebase": "^8.0.2" the caching behaviour seems to have changed.

On the start page, the two users are loaded twice every time I load the page (see gif below). In the console output you can even see that it first loads two and then jumps to 4 shortly later (this was always correct before the update). start

The next problem the updates introduced was on the profile page when lazy loading data in the Angular Material tabs. Now, data seems to loaded again (as expected), but stacked on the previous data (see gif below). profile

Expected behavior

To load the required data once from cache if the data has already been loaded from network before.

Actual behavior

Data is loaded twice from cache and apparently stacked when lazy loaded.

jamesdaniels commented 3 years ago

Firebase 8.0.2 has a bug, upgrade to 8.0.3 if it's out yet. Otherwise roll back.

See #2660

jamesdaniels commented 3 years ago

If you're still experiencing after getting off of 8.0.2 LMK and I'll dig in.

lukfel commented 3 years ago

@jamesdaniels Thanks for the reply. However, I also tried 8.0.1 and 8.0.0 and they also had the same behaviour. The last working version I tried was 7.22.0.

lukfel commented 3 years ago

@jamesdaniels I just updated to firebase 8.1.1. Docs are still loaded twice from cache. However, the second issue on the profile page with the lazy loaded docs was resolved.

jamesdaniels commented 3 years ago

Caching is the expected behavior with Firestore. I haven't seen anything unexpected outside of Firebase 8.0.2, That said snapshotChanges isn't reporting metadata as I expected in Angular 6.1. I have addressed this on @canary and debating cutting in 6.1.2 or with the next API additions in a 6.2 release.

ahmedogela commented 3 years ago

Still an issue. It gets snapshot with type: added and fromCache: true, then emits the same snapshot type: modified and fromCache: false. Why does it emit again with type modified and why it get from server while there is no modification!? Persistence enabled *** It seems to be a firebase issue that it happens even using the firebase API directly. The expected behavior to get from cache first and only emit if there is a real modification.

jamesdaniels commented 3 years ago

This is expected behavior. The doc is modified because it's metadata is now different. This is to provide you the most flexibility. If you don't want the additional emits, you can filter out using something like distinctUntilChanged((a,b) => a.payload.isEqual(b.payload))

jamesdaniels commented 3 years ago

Though metadata will probably still trip that, so deep equality on the data()?

ahmedogela commented 3 years ago

I can adjust the behavior for my case, but my concerns about the cost of extra reads, so does it count as extra reads that the second emit doesn't come fromCache?

jamesdaniels commented 3 years ago

I don't believe it counts as a read, until there's a legit update, because it's just confirming that the data on your cache is up to date & there's been no new data transferred.

A read costs $0.0000006 though, so TBH I haven't been paying much attention to the "constant" on the number of reads I perform in my apps, only the exponent.

jornetsimon commented 3 years ago

I don't believe it counts as a read, until there's a legit update, because it's just confirming that the data on your cache is up to date & there's been no new data transferred.

I didn't encounter any documentation about this. Is there any? The official statement is that listeners should typically not get triggered when the validated data from the server is identical. So this behavior is kinda confusing/alarming.

A read costs $0.0000006 though, so TBH I haven't been paying much attention to the "constant" on the number of reads I perform in my apps, only the exponent.

Your rationale is fine, but when scaling up this can be a great cost generating detail.