Skip to content

ValueChanges not returning all data. #1747

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robert-king opened this issue Jun 24, 2018 · 10 comments
Closed

ValueChanges not returning all data. #1747

robert-king opened this issue Jun 24, 2018 · 10 comments

Comments

@robert-king
Copy link

robert-king commented Jun 24, 2018

Version info

"@agm/core": "^1.0.0-beta.2",
"@angular/animations": "6.0.5",
"@angular/cdk": "^5.2.5",
"@angular/common": "6.0.5",
"@angular/compiler": "6.0.5",
"@angular/core": "6.0.5",
"@angular/forms": "6.0.5",
"@angular/http": "6.0.5",
"@angular/material": "^5.2.5",
"@angular/platform-browser": "6.0.5",
"@angular/platform-browser-dynamic": "6.0.5",
"@angular/router": "6.0.5",
"@angular/service-worker": "6.0.5",
"angular-instantsearch": "^2.0.2",
"angularfire2": "^5.0.0-rc.10",

return this.usersCollection.valueChanges().subscribe((users) => {
  console.log('got users', users);
})

I have two users, however it logs a users array of just one user, then after that, it logs an array with two users. This is dangerous because If I pipe valueChanges() to first() or take(1), then I will be missing content forever.

I assume this happens because one of the users is me, and since i've already logged in, my user record is in the cache, so it returns that while fetching the rest of the user collection. Perhaps that's expected behaviour, if so, then it should be better documented to be careful piping valueChanges to .first() or to .take(1)

@davideast
Copy link
Collaborator

@robert-king Can you create a stackblitz recreating the problem so I can debug?

@robert-king
Copy link
Author

@trentjones21
Copy link

trentjones21 commented Jul 2, 2018

I can confirm that this is a problem. It would be nice if there was a way to disable the cache completely for an individual query. Something like

db.collection('users').valueChanges({ source: 'server' });

This appears to be how it works when using firestore directly. I can't seem to find a way to do it with angularfire2 though. Here is the relevant firestore documentation:

https://firebase.google.com/docs/reference/js/firebase.firestore.GetOptions

var docRef = db.collection("cities").doc("SF");

// Valid options for source are 'server', 'cache', or
// 'default'. See https://firebase.google.com/docs/reference/js/firebase.firestore.GetOptions
// for more information.
var getOptions = {
    source: 'cache'
};

// Get a document, forcing the SDK to fetch from the offline cache.
docRef.get(getOptions).then(function(doc) {
    // Document was found in the cache. If no cached document exists,
    // an error will be returned to the 'catch' block below.
    console.log("Cached document data:", doc.data());
}).catch(function(error) {
    console.log("Error getting cached document:", error);
});

@robert-king
Copy link
Author

It would be cool to show progress too - e.g. show some things quickly from the cache, but have a loading spinner to show while more results are being fetched from the server.

@mikhailmelnik
Copy link

Spent a few hours yesterday debugging same thing, ended up with the following snippet illustrating the issue:

afs.collection('profiles').valueChanges().pipe(take(1), tap(console.log)).subscribe(() =>
  afs.doc('profiles/SOME_ID').valueChanges().pipe(tap(console.log), delay(2000)).subscribe(() =>
    afs.collection('profiles').valueChanges().pipe(tap(console.log)).subscribe()));

with output [N], {...}, [1], [N] where N/1 is the reported collection length, so if I add take(1) to the last line which was my original intention I would never receive the full collection.

As a workaround started using underlying firestore which allows to set a source:

afs.firestore.collection('profiles').get({source: 'server'}).then(...)

Would be great to have a parameter for the valueChanges() call as @trentjones21 have suggested.

@TyagiAnkit40
Copy link

I have the same issue. Did anyone find a workaround on how to disable caching when using valuechanges or onSnapShot?

@davideast
Copy link
Collaborator

davideast commented Aug 13, 2018

@robert-king @trentjones21 @TyagiAnkit40 Just put in a PR to address this. You can specify the { includeMetadataChanges: true } config and then filter out any snap.meta.fromCache: true objects.

Let me know if any of you are up for testing the test release.

@TyagiAnkit40
Copy link

I can test. Currently, I am using an ugly workaround by calling get directly inside get collection which I want to avoid.

this.db.collection('OrderBom', ref => {
let query : firebase.firestore.Query = ref;
query = query.where('orderNo', '==', orderNo).where('inEntityLC', '==', entity);
query.get({source: 'server'}).then(result=>{
var arr=[];
result.forEach((doc) => {
arr.push(doc.data());
});
this.createOrderTab(arr);
})
return query;
})

@TyagiAnkit40
Copy link

TyagiAnkit40 commented Aug 13, 2018

Will it be better if we can directly specify {source : 'server'} instead of reading metadata? That way, the subscribe to valuechanges / snapshot will only fire once. Today, subscribe fires two times, one for cached data and another for server data. Even today, I can look at 'fromCache' property and identify whether it's cached data or loaded from server. Not sure how including metadata will be different.

@davideast
Copy link
Collaborator

We will add the ability to filter client events with #1819. However, in the OP's use case you should not use .take(1) on a stream. Stream behave differently than a data request like a .get() call. If you need the set of data call ref.get().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants