feat(useFirestore): support delay for autoDispose, fixes #2252

[Zehir]

Description

Allow to add a delay on the autoDispose option to allow user to load the same document on a new page just after the document was previously loaded and disposed. This will allow to use the firestore document cache and avoid a Firestore read that cost around €0.06 for 100 000 document's reads and some network usage.

Additional context

We previously discuss about this feature on the issue #2252

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[kiyopikko]

I want the following tests 🙏

1. collection or doc dispose without option
2. autoDispose: false
3. autoDispose: timeout
4. autoDispose: timeout close immediately when new listener comes

[Zehir]

@kiyopikko I added tests and fix one bug

[Zehir]

Do you think we should make a setting for the dispose on read during timeout ?

[kiyopikko]

oh sorry, I imagined B of the following picture, but this PR is C...?

if true, D can happen so this code may not be very effective(and may be less predictable to users).

[Zehir]

Yes it's implemented like C but the Firebase SDK is re-using the data from the cache so it's like B.
Doing B will be way more complex than C for not many benenfits.

Yes D can happen that why I think we should add a setting for that that is not enabled by default ?
And maybe a read limit count, like you close if if you get more than 3 reads. If your data is updated that many time you will still have to read it anyways so it's not worth it to keep updating a data that you don't need. In the D part of the graph, between OLD die and NEW come you don't need the data. So if your data is updated that many time you probably don't want them updating between thoses 2 times

[kiyopikko]

It seems too much configuration so I think this would be sufficient just to be able to set a timeout. (removing this lines)

1. I'm wondering if how many users want to do this optimization.
2. That option is complex and difficult to explain.

Simple and configurable is trade-off, useFirestore is one of the vueuse utility collection, I don't know if it's a good idea to have such a variety of options. Maybe a library like vue-use-firestore(fictional) will support that.

This is just my opinion and I'm just a contributor.
Only the vueuse members have the philosophy. It seems better to let them be the judge

[Zehir]

Or we add a control option like in useTimestamp to expose the stop option ?

[kiyopikko]

@Zehir

Or we add a control option like in useTimestamp to expose the stop option ?

how we access that option in component? (even in unmouted component)

---

I think it needs more discussion but the below bug should be resolved soon. Can you (or can I) make a PR to fix only this?

https://github.com/vueuse/vueuse/blob/main/packages/firebase/useFirestore/index.ts#L99

[Zehir]

In the useTimestamp have an option that modify the returned data. Instead of having only the data we will have an object with the data in a data key, a stop method, and anything else we add down the road.

Yes I will make a PR to fix the issue

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Zehir]

@kiyopikko any chances to get this merge ?

[kiyopikko]

@Zehir

As I wrote in #2276 (comment), please remove autoDisposingReadLimit?

It seems too much configuration so I think this would be sufficient just to be able to set a timeout. (removing this lines)

[Zehir]

@kiyopikko I removed the autoDisposingReadLimit option

[kiyopikko]

LGTM!

[Zehir]

Anybody can merge this PR please ?