`findAll` reloading is not respected and the model hook resolves before all data is fetched
roomman opened this issue · 9 comments
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?
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);
});
});
}
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.
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 🙏🏻
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 😅
Amazing 🤩
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
.
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.
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'.
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
.