findAll reloading is not respected and the model hook resolves before all data is fetched

[roomman]

Hi, I’m struggling to maintain a route’s loading state while findAll resolves.

I have a user model. When a given user logs in their user is loaded into the Store via findRecord. There is an admin page where all users are loaded via findAll. Right now, when I navigate to that page, I immediately see one user (my own) and after a delay the other users “pop” into the template.

Here’s the current implementation:

import Route from '@ember/routing/route';
import { inject as service } from '@ember/service';

export default class AuthenticatedAdminUsersRoute extends Route {
  @service store;

  async model() {
    const users = await this.store.findAll('user', {
      adapterOptions: {
        isRealtime: true,
      },
      reload: true,
    });
    return users;
  }
}

As instructed by the API docs for Ember Data, I’ve included the reload option. I had hoped this would be enough to keep things in a loading state until the promise had resolved. However, in the current implementation of this adapter, reload isn’t being respected. I believe this is because of the use of onSnapshot in firestoreDataManager.findAll(), whereby data is grabbed from the cache. Reverting to the use of getDocs delivers the right result (for my use case).

Is my implementation wrong - is there already a way to achieve the desired result? If not, would it be possible for the adapter use of getDocs in cases where options.reload is true?

[roomman]

Thinking on this further, I wonder if onSnapshot could evaluate fromCache in the snapshot's metadata and respond accordingly? Something like:

public findAll(colRef: CollectionReference, shouldReload: boolean): Promise<QuerySnapshot> {
  return new Promise((resolve, reject) => {
    // Use `onSnapshot` instead of `getDocs` because the former fetches from cache if there's
    // already an existing listener for it
    const unsubscribe = onSnapshot(colRef, { includeMetadataChanges: shouldReload }, (querySnapshot) => {
      if (shouldReload && querySnapshot.metadata.fromCache) {
        return;
      }
      unsubscribe();
      resolve(querySnapshot);
    }, (error) => {
      reject(error);
    });
  });
}

[mikkopaderes]

Hmm, good point on findAll issue. Generally, I'd avoid findAll and use query instead because it's not a good practice in Firestore to fetch a whole collection cost-wise. It's also the reason why I don't support realtime for it.

However, not being able to refetch it because reload is disable is definitely a bug which I can look into.

[roomman]

Thanks @mikkopaderes 👍🏻

Ultimately (I'm working with a small number of records now), I will also use query for large sets but I do have some cases (a dozen or so categories, for example) where findAll has a role.

If you were to use includeMetadataChanges, onSnapshot will return the cached data with metadata.fromCache being true. Then a second snapshot with the remaining records will land in a second snapshot, when the metadata.fromCache is toggled.

You don't support real-time for findAll? 🤯 Can you clarify? I didn't get this from the docs, I assumed the findAllRealtime meant you did, and I'm sure I've seen real-time behaviour while developing?

As always, thanks for this addon, it's been a charm to work with 🙏🏻

[mikkopaderes]

You don't support real-time for findAll? 🤯 Can you clarify? I didn't get this from the docs, I assumed the findAllRealtime meant you did, and I'm sure I've seen real-time behaviour while developing?

Will read to other messages later on and respond to it if any, but just to answer this quickly. It's DEFINITELY supported, I'm not sure why I even said that it wasn't, I just probably zoned out for a bit 😅

[roomman]

Amazing 🤩

[mikkopaderes]

I have a user model. When a given user logs in their user is loaded into the Store via findRecord. There is an admin page where all users are loaded via findAll. Right now, when I navigate to that page, I immediately see one user (my own) and after a delay the other users “pop” into the template.

I understand and just as you've said, it grabs from the cache as designed by Firestore as indicated in the footnote here. What I didn't know before was that if you have the onSnapshot listener to the doc, it will also be considered cached record when doing onSnapshot for a collection. I thought that it only considers cache when you have the same onSnapshot target as collection+collection or doc+doc not collection+doc—very interesting.

To solve this with native Firestore, you'd have to fetch the collection using getDocs which you've also said above. The problem now lies with the adapter is that I designed it to always use onSnapshot even if just a one time fetch to take advantage of the caching, but unfortunately, like I said above, I wasn't aware that it overlaps between collection and doc.

I will look into it and see that whenever we don't provide isRealtime, we always use getDoc or getDocs.

[mikkopaderes]

And if you need something immediately, I believe you can extend the adapter into your own app and change this line into await getDocs(colRef) and that should work for your case until I provide a change to this from the addon itself.

[roomman]

I will look into it and see that whenever we don't provide isRealtime, we always use getDoc or getDocs.

My ideal scenario would allow isRealtime (I want document changes to be reflected locally) and provide a way to override the cache / await the full set of records, so I get the initial loading state. This would apply to findAll and query operations (just tested query behaves in the same way - i.e. I get an initial snapshot with one user then a second for all users that match my query.)

I think this issue addresses the mechanics of doing it 'realtime'.

[mikkopaderes]

I see, I understand now. The solution looks interesting but I will have to think about it and see if there's a better way to implement it. I don't just want to add a new property to adapterOptions that says fetchFromCache: false or something because that's not flexible enough and is a maintenance burden. The approach that I generally go for is how to expose the Firestore API completely but wrap it in Ember Data StoreService.

In the meantime, I imagine that this could be a workaround for now:

@service('-firebase')
firebase;

function model() {
  return new Promise((resolve) => {
    const db = this.firebase.firestore();
    const unsubscribe = onSnapshot(collection(db, 'users'), { includeMetadataChanges: true }, (querySnapshot) => {
      if (querySnapshot.metadata.fromCache) {
        return;
      }

      resolve();
    });
  }).then(() => {
    return this.store.findAll('users', {
      adapterOptions: {
        isRealtime: true,
      },
    });
  }).finally((model) => {
    unsubscribe();
  });
}

What this does is that you setup a listener that makes sure to get the latest from the server and have that as the cache when you do store.findAll afterwards. Then you clean it up via unsubscribe() as the listener will now be managed by the internals of findAll.