SupposedlySam / firefuel

A Firebase Cloud Firestore architecture to jump-start your project
MIT License
8 stars 1 forks source link

Question: offline support? #62

Open ezmegy opened 2 years ago

ezmegy commented 2 years ago

Hey guys,

Awesome plugin, I only stumbled on it and tried recently but I think it's a brilliant tool on top of Firestore that can do much of the heavy-lifting in a smooth and coherent manner, great job!

I wanted to check if you plan on supporting offline usage (to the extent Firestore does)?

At the moment, when you're offline and try creating a new document or updating an existing (and cached) one will hang because of the consistent await usage. E.g. the create function in firefuel_collection.dart will hang on line 29 and the createById on line 39 etc.

Annoyingly enough the official docs do not suggest a best practice; at least I haven't found something like that yet. One could omit the awaits altogether (most of the official examples do I think) or roll promises. I don't think there's a third option, is there? Here's a good summary for reference.

Either way, just wanted check with you.

Cheers

SupposedlySam commented 2 years ago

Hey @ezmegy, thanks for the kind words!

As for offline support, this isn't something we focused on while creating the package but we wouldn't mind taking a look at it for you!

Is there any way you can provide us with some basic reproduction steps?

Just something as simple as:

  1. Load app with internet
  2. Pull down data
  3. Turn off internet
  4. Interact with xyz

but maybe with a little more detail to the methods you're using from firefuel, etc.

We'll see if we can reproduce and work with you to get it up and running!

ezmegy commented 2 years ago

Hi @SupposedlySam,

Thanks for the quick reply! Sounds great, I'll try and help if I can.

Using the example app the repro steps could be:

  1. Open app
  2. Increment counter
  3. Turn off internet
  4. Increment counter again -> will hang: It won't do anything on the UI and if you debug it, the update function in firefuel_collection.dart on line 206 will not return (so you don't get to 208) as long as the internet is turned off.
  5. Optionally: Turn on internet back on (without quitting the app first) -> will continue execution on line 208.

Some points to note:

To sum it up, I think the situation with offline usage is the following:

READING is working as expected: If the document has been cached it works both with and without Firefuel/awaits. If the document has not been cached fails both with and without Firefuel/awaits.

CREATING a new document: hangs with awaits, works without them.

UPDATING an existing document: if the document has been cached before it hangs with awaits but works without them, if the document has not been cached before it fails both with and without awaits as expected.

Edit: after re-reading this, I found the term node super confusing, so removed that and used documents/collections instead.

SupposedlySam commented 2 years ago

Thank you for such a detailed response! I love the depth you went into here and I think what you've written is extremely clear.

I did a little searching and found this video from a few years ago that says the same thing about "writes" as you mentioned above: https://youtu.be/XrltP8bOHT0?t=677.

Our usage of await is the same as their usage of then and causes the UI to hang. I have a couple of thoughts here.

  1. As an immediate workaround (not dependent on a package change), you can update your code to not await the result of any write calls in firefuel if your app doesn't have access to internet.
  2. We can create a story from this issue to accept a boolean value (isOffline -- or similar) which no longer awaits the value when performing any writes to firestore.

I toyed with the idea of checking for internet inside of firefuel but I'm not sure about adding another dependency to the package. Allowing a boolean value to be passed in will give everyone the ability to choose their own way of checking that internet exists.

Looking at the example app, I think the reason it currently hangs is because we're waiting on the response from the repository (aka firestore) before we update the state. If we were to check for internet connection inside the example app and no longer await the response before updating the state I think it would work offline.

Thoughts?

ezmegy commented 2 years ago

Sounds great!

I pretty much agree with everything you said 😃

Example app: I think with the isOffline/Online flag you could actually just keep the outer await, couldn't you?

So for example if we look at increment in counter_collection.dart on line 19 something like: await update(docId: docId, value: incrementedCounter, isOnline: isConnected); could work if the update function in firefuel_collection.dart, line 206 does this:

if (isOnline) {
  await ref.doc(docId.docId).update(value.toJson());
} else {
  ref.doc(docId.docId).update(value.toJson());
}
...

But if we don't drop the await inside the update function (w/o the isOffline flag), just the outer in the increment function, that wouldn't work. (Not sure now if this is different from what you meant or is exactly the same 😊 )

SupposedlySam commented 2 years ago

Hmm. I was thinking it would. If you invoke any async function and don't await its result it will act like a synchronous function and continue execution.

Of course, if there were any side-effects, those would happen at some point in the future when the function actually does return.

For instance, in the example you propose above:

await update(docId: docId, value: incrementedCounter, isOnline: false);

is the exact same as:

update(docId: docId, value: incrementedCounter, isOnline: true);

isn't it?

Any time an await is used, it pauses execution for the function awaiting its result and continues execution on the next line once it's finally done. However, if somewhere up the chain you decide not to await it, then the function that fired it off won't be waiting for anything. It just happily continues as if the function already completed.

Note: the example above doesn't do anything with the result returned (if any) so it's the most simple example. If we were to NEED the value that was returned...I'm not quite sure what to do. The only thing I can think of would be to not rely on an auto-gen ID and instead create one using the generateDocId method on a FirefuelCollection and use that id as a reference while offline.

ezmegy commented 2 years ago

Counter Hmm 😃

I agree with most of your explanation on async vs sync but synchronous functions can hang too, right? So if we (don't await) just call an async function that does not return we would still be hanging at that point, wouldn't we?

I believe that's what's happening here. Could be still missing something of course. Hooked up the example app though to a Firebase project and tested it; was working as expected/described before:

Képernyőfotó 2022-08-11 - 11 13 18

Képernyőfotó 2022-08-11 - 11 17 25

Now it would be very different of course if we returned whatever Firestore gives us directly. So for example with update like this:

Képernyőfotó 2022-08-11 - 11 36 24

Because in this case we lift the responsibility of awaiting/not awaiting up one level, back to the caller.

With firestore functions that return void like update() or set() it's easily done (and maybe preferable?). Those which return snapshots the convenience provided by firefuel comes exactly from intercepting that and returning the data class object instead, so I would keep doing that and probably still add the isOnline flag + one case with the await for the true and one without await for the false scenario...?

SupposedlySam commented 2 years ago

@ezmegy I wanted to pull down the code and run through it myself to make sure it was working the way I expected it to.

I created a new folder called example-offline on the jonahwalker/docs/offline-example branch. Just run git checkout jonahwalker/docs/offline-example in the repo to get access to the new folder. It's already connected to a firestore sample project so you don't need to connect your own.

Note: I was testing it with Android so the ios side of things might not be set up 100%.

The only thing I changed in this example is to remove the await on the update function on both the increment and decrement functions and added a print statements below them with counterLog: prefixes so you can filter them out in the debug console.

It does move past the update function immediately (even added in a StopWatch around the call to make sure). However, there does seem to be a problem that might not be able to be handled by the firefuel package. I updated the package code in the firefuel_mixin to run a StopWatch around the callback in the guard function. The first execution was really fast, but the second one (and after) were extremely slow.

I/flutter ( 1022): counterLog: Incrementing to Counter(value: 21)
I/flutter ( 1022): counterLog: executed callback in 14 milliseconds
I/flutter ( 1022): counterLog: Incrementing to Counter(value: 22)
I/flutter ( 1022): counterLog: executed callback in 26626 milliseconds

I'll look into it some more later, but a 26 second wait is not acceptable IMO.