🐛 FirestoreListView renders (incomplete) local cache before remote db result

sinnaj-r commented 10 months ago

Is there an existing issue for this?

[X] I have searched the existing issues and found no duplicates.

What plugin is this bug for?

Firebase UI Firestore

What platform(s) does this bug affect?

Web

List of dependencies used.

flutter pub deps -s list

``` Our dependency list is very long, and contains private packages, therefore I only include the relevant firebase packages here: - firebase_core 2.21.0 - firebase_core_platform_interface ^5.0.0 - firebase_core_web ^2.8.1 - flutter any - meta ^1.8.0 - firebase_auth 4.12.1 - firebase_auth_platform_interface ^7.0.3 - firebase_auth_web ^5.8.6 - firebase_core ^2.21.0 - firebase_core_platform_interface ^5.0.0 - flutter any - meta ^1.8.0 - firebase_messaging 14.7.3 - firebase_core ^2.21.0 - firebase_core_platform_interface ^5.0.0 - firebase_messaging_platform_interface ^4.5.12 - firebase_messaging_web ^3.5.12 - flutter any - meta ^1.8.0 - firebase_crashlytics 3.4.3 - firebase_core ^2.21.0 - firebase_core_platform_interface ^5.0.0 - firebase_crashlytics_platform_interface ^3.6.11 - flutter any - stack_trace ^1.10.0 - cloud_firestore 4.12.2 - cloud_firestore_platform_interface ^6.0.3 - cloud_firestore_web ^3.8.3 - collection ^1.0.0 - firebase_core ^2.21.0 - firebase_core_platform_interface ^5.0.0 - flutter any - meta ^1.8.0 - firebase_ui_firestore ^1.5.7 ```

Steps to reproduce

Do not enable Persistance on Web (default is -- as far as I know -- a small memory cache)
Have .snapshots()-Stream on a single firestore document (lets call it docA ); Render this document e.g. with a StreamBuilder.
Next to that use the FirestoreListView to render a list of documents (including docA ). The database should contain enough documents, to allow for multiple pages.
Scroll down the FirestoreListView to trigger the "fetch more" functionality.
Now the FirestoreListView will render a list only containing docA for a very short amount of time and then render the full list. This results in the scroll position beeing lost and the whole list apparently jumping to the top.

Expected Behavior

I would expect the List not jumping. And thereby, as far as I understood the bug, I would expect the list not rendering a single cached item inbetween list fetches. I would also be fine with disabling any caching behavior on web, if there's a way to do that.

Actual Behavior

(see steps to reproduce)

Additional Information

I briefly looked into the FirestoreListView code (packages/firebase_ui_firestore/lib/src/query_builder.dart) . If I'm not mistaken, the problems lies in the query.snapshots().listen()-Callback in lines 176 et seq. Here any data is "rendered", as long as its smaller then expectedDocsCount, which of course includes the one item eagerly returned by the cache.

danagbemava-nc commented 10 months ago

Labeling based on the report above, I keep hitting https://github.com/firebase/FirebaseUI-Flutter/issues/239

sinnaj-r commented 10 months ago

Labeling based on the report above, I keep hitting #239

Are you able to reproduce the bug now that #239 is fixed ? Otherwise I can gladly provide more information.

russellwheatley commented 9 months ago

Scratched my head on this issue for a while, it turns out, this appears to be a bug in the firebase-js-sdk. I will open a bug report on that repo and link to this issue 👍

russellwheatley commented 9 months ago

Opened issue here if you wish to track: https://github.com/firebase/firebase-js-sdk/issues/8004

adifyr commented 9 months ago

Hi. Do you have any updates on this? I am utilizing a FirestoreListView while also listening to a specific document using StreamProvider on Riverpod. And I'm facing this issue.

The JS sdk isue got closed as it was concluded that the issue wasn't there. What's the issue exactly? And is there an interim solution in the meantime?

russellwheatley commented 9 months ago

the interim solution is to update your cache settings so persistenceEnabled is set to true:

  FirebaseFirestore.instance.settings = const Settings(
    persistenceEnabled: true,
  );

I'm trying to figure out the optimal solution to this problem in the underlying implementation.

russellwheatley commented 7 months ago

@sinnaj-r - I actually think the solution to this problem is to set persistence:

  FirebaseFirestore.instance.settings = const Settings(
    persistenceEnabled: true,
  );

That way, you're matching the persistence that you see on the native SDKs which have persistence enabled by default. FlutteFire has also fixed the hot restart bug so it no longer crashes.

sinnaj-r commented 7 months ago

@sinnaj-r - I actually think the solution to this problem is to set persistence:
  FirebaseFirestore.instance.settings = const Settings(
    persistenceEnabled: true,
  );
That way, you're matching the persistence that you see on the native SDKs which have persistence enabled by default. FlutteFire has also fixed the hot restart bug so it no longer crashes.

Thank you for the response; unfortunately, at least in our use case, activating persistence is not something we can do because our application is used by many "office"-type PCs with very limited resources that can not deal with the app taking up several hundreds of megabytes (or more) of RAM -- which is what we observed after enabling persistence.

russellwheatley commented 7 months ago

Have you tried limiting cache size:

FirebaseFirestore.instance.settings = const Settings(
    persistenceEnabled: true,
    // Limit cache size to whatever you want
    cacheSizeBytes: 10000,
  );

github-actions[bot] commented 2 weeks ago

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time. Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

pierre-su commented 1 week ago

Have you tried limiting cache size:

FirebaseFirestore.instance.settings = const Settings(
    persistenceEnabled: true,
  // Limit cache size to whatever you want
    cacheSizeBytes: 10000,
  );

I experimented with various ways to limit the cache size, but the RAM usage remains too high, as shown in the images below. As @sinnaj-r mentioned, enabling persistence is not a viable option, and setting cacheByteSize has no significant impact.

Does anyone have suggestions for alternative approaches?

Images: The screenshots are in German, but they all display "Arbeitsspeichernutzung" (RAM usage). For example, "Arbeitsspeichernutzung: 279 MB" translates to "RAM usage: 279 MB."

Disabled caching: cache_disabled Cache set to 1 MB: cache_1_mb Cache set to 10 MB: cache_10mb_ Cache set to 100 MB: cache_100mb Cache set to null: cache_null Cache set to Settings.CACHE_SIZE_UNLIMITED: cache_unlimited