Skip to content
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

FR: Offline first support for PWAs (RTDB) #17

Open
Hollerweger opened this issue May 18, 2017 · 101 comments
Open

FR: Offline first support for PWAs (RTDB) #17

Hollerweger opened this issue May 18, 2017 · 101 comments

Comments

@Hollerweger
Copy link

While the Firebase JS SDK has support for offline scenarios when the web app goes from online to offline it lacks offline first support.
Offline first is a crucial part of PWAs and should be supported by the Firebase JS SDK directly.

@google-oss-bot
Copy link
Contributor

Hey there! I couldn't figure out what this issue is about, so I've labeled it for a human to triage. Hang tight.

@jshcrowthe
Copy link
Contributor

I have some ideas of what I think you mean by "offline first support," but the term itself is rather vague especially when viewed in the context of the entire JS SDK as each part of firebase (auth, database, storage, messaging) would have different approaches to "offline first support."

That said, I like what this issue calls attention to, and it is something that I'd love to pursue. Can you help me understand what specific things were difficult for you in doing offline first development?

@Hollerweger
Copy link
Author

I was thinking about a database persistence similar to what is supported with the Firebase iOS and Androd SDKs available even when the web app is reopened in a new browser tab offline.
Right now i need to implement my own offline persistence layer on top of Firebase to support offline first scenarios.
There was even a Firebase I/O session last year regarding PWAs and offline first. For this demo Polymer with an index db mirror was used on top because the functionality was not provided by the Firebase JS SDK itself. https://www.youtube.com/watch?v=SobXoh4rb58
With this approach I'm limited on the offline functionality of Polymer without the ability to directly query the Firebase database.
Would be great if such an index db mirror could be part of Firebase JS SDK itself.

@knpwrs
Copy link
Contributor

knpwrs commented May 18, 2017

This is something I have talked to Firebase support about in the past and I was actually just about to open my own issue until I saw this one. When I think about Firebase (at least, my usage of Firebase) and offline functionality I think of storage.

I think what would make the most sense would be to refactor the current implementation to support storage adapters. The current implementation could become the default, "in-memory" adapter. Other community-developed or officially supported adapters could be published as well. IndexedDB is an obvious choice, it's what PouchDB uses by default. A less obvious adapter I would like to implement for use in Electron would be a sqlite adapter.

Just spitballing here, but there could also be a proxy adapter to use two adapters together. For instance, I could use the in-memory adapter along with my sqlite adapter for performance purposes.

The Firebase SDKs were only just recently open sourced. Would these types of features be welcome for pull requests?

@jthegedus
Copy link

I was looking at achieving this type of functionality with Firebase and Redux-Offline. If the Firebase JS SDK was to be made modular with defaults it seems that it should do so in other layers of the SDK than just storage to achieve all the Offline-first criteria as specified in Redux-Offline EG: like exposing functions for implementing custom reconciliation of optimistic update failures/rollbacks.

@TarikHuber
Copy link

Making the data Offline-first available using react-redux and firebase is no big deal. Here is a working example: https://github.com/TarikHuber/react-most-wanted
But: the data is offline available only for the client. The firebase database listeners don't know that you already have most of the data in your local storage. It would be great if firebase itself would manage Offline-first. Maybe they could figure out how to then just load the data that is missing in the local storage and not all of it like it's done with a running app that has connection. For example: you loaded 10 tasks in your application and go offline or close the application. After you reconnect firebase uses hes own cache not only to give you the already loaded 10 tasks but to also just load 2 tasks that where added afterwards and edits to the existing 10.

@jthegedus
Copy link

It's no big deal except you have to manage Redux in addition to Firebase. You have no control over when Firebase syncs to the server, you're restricted by Firebases local cache limits and persisting the cache isn't trivial. And Redux certainly overlaps with what the Firebase SDK does for offline. All could be mitigated should Firebase support a few modules/adapters for Offline-first in a similar method to how Redux-Offline defines. I'm just suggesting that we use Redux-Offline as a guide for what parts could be made modular.

@jshcrowthe
Copy link
Contributor

@knpwrs I think this is something that we could totally accept as a PR! Love to have your contributions. The notion of different storage adapters is also an interesting idea that I'd love to see more details on.

In addition, I'd encourage everyone, for all Feature Requests, to make sure you are signed up for the Firebase Alpha Program where you can keep up on all the upcoming features and products.

@FluorescentHallucinogen

@jshcrowthe What exactly features and products Firebase Alpha Program offers at this time? I've completed and submitted the form 3 days ago. I wonder how long to wait the admission into the Alpha Program?

@mbleigh
Copy link

mbleigh commented May 29, 2017 via email

@jshcrowthe
Copy link
Contributor

jshcrowthe commented May 30, 2017

+1 to what @mbleigh said.

Lets try and keep this thread on topic though 😄 . Further questions on the Alpha program would be better directed to our support or discussion channels (link here: https://firebase.google.com/support/)

@jsayol
Copy link
Contributor

jsayol commented Jun 3, 2017

Adding some form of local storage in our apps to cache the data retrieved from the Firebase database (like most of us are doing, I guess) works quite well to enable offline use even when cold-starting an app, but the main problem I see with not having persistence built into the SDK like it is in the Android or iOS ones is this: when an app starts, the SDK has no idea what data is stored locally so, when it attaches listeners, the hash field is empty and the server responds with all the data. Every single time. That means that data usage with the JS SDK is significantly higher than with the native ones.

I understand building a reliable and truly cross-browser local storage solution into the Firebase SDK is no easy feat, maybe even impossible, but it doesn't need to be perfect. It just needs to be better than not having it, even if only in some situations. It could be implemented gradually, first for whatever browsers have the best IndexedDB or SQLite support and then slowly with others, if possible.

Another possible solution, albeit a radically different approach to what is being currently used in the other platforms, would be for the SDK user to pass whatever data it has for a certain database location before attaching a listener.

This might be better explained with an example: let's say we already know what the data at /messages contains, because we had it locally stored somehow:

let data = {
  "-Kgx5lyGUg7w9nnNAKss": {
    "from": "Bob",
    "text": "Hey there"
  },
  "-Kgx9en1kPcRyy1uk7j7": {
    "from": "Alice",
    "text": "Sup?"
  }
};

So there would be a way to "bootstrap" the data at that location before attaching a listener, letting the SDK know what we know:

firebase.database().ref('messages').bootstrap(data).on('child_added', snap => { /* */ });

This leaves some open questions, though: should the SDK always accept that data, or should it ignore it when it is positive the data it has is fresh? (maybe because there's already an active listener on that path).

This would only be a temporary solution anyway, since it puts most of the burden on the developer using the library (figuring out how to store the data locally, passing it to the SDK, etc.) Not my favorite approach but it would certainly be a step up.

@jsayol
Copy link
Contributor

jsayol commented Jun 4, 2017

Some more thoughts: a possible solution to add local storage support would be to use localForage, maybe wrapping it like ionic-storage is doing. This would allow to use whatever the best solution is in every scenario/browser.

@knpwrs's idea of storage adapters also seems quite interesting. Skimming through the code, it seems like there's already support for in-memory, LocalStorage, and SessionStorage. So it seems it would be a matter of implementing a way to allow the user to provide their own adapter with a compatible API. I would also consider this a temporary solution though, like the bootstrapping I mentioned in my previous comment. [EDIT] I was looking in the wrong place, nevermind what I said here[/EDIT] The ultimate goal should be for the SDK to handle all this for the user, the same way it's done in the native SDKs.

@jthegedus
Copy link

Having a storage mechanism like redux-persist would allow for complete browser/native coverage. Then the user would only have to specify an environment flag for the correct storage adapter to be used.

@knpwrs
Copy link
Contributor

knpwrs commented Jun 4, 2017

@jsayol I've been looking at how PouchDB stores data offline. They've taken the approach of basing their storage around LevelUP. From that point you can plug in different backends such as MemDOWN (in-memory), level.js (IndexedDB), or even something like SQLdown (sqlite3, PostgreSQL, and MySQL). There's even an Abstract LevelDOWN project which can be used to implement compatible backends. Basing storage around LevelUP could be potentially interesting because then we inherit a large offering of various storage backends. By default we could use MemDOWN and offer the ability to use different backends such as level.js.

@FluorescentHallucinogen
Copy link

I believe Firabase developers already have a solution. That's why @jshcrowthe suggests to sign up Firebase Alpha Program.

@jshcrowthe, @mbleigh, isn't it?

@jshcrowthe
Copy link
Contributor

@FluorescentHallucinogen, the Alpha Program is a great way to work with developers who are willing to donate their time to help make Firebase an awesome platform. There is really good discussion going on, so the invitation is to make sure we get to work with all of you in that space as well!

@jshcrowthe
Copy link
Contributor

jshcrowthe commented Jun 7, 2017

AFAICT this discussion primarily emphasizes two things

  1. Providing offline access to data
  2. Leveraging offline data on boot to prevent unneeded network traffic

The notion of pluggable storage adapters is something that I think is a cool idea and I'd love to see a demo implementation of this in context of the SDK. This could be separate from minimizing the network traffic as the amount of network traffic would be no different than what it is today. Once we had an agreed upon implementation of persistence, reducing network overhead is just the next logical step.

In the iOS SDK (Github Repo: https://github.com/firebase/firebase-ios-sdk) we are synchronizing only the delta between the local device and server state. In principle we could port that same functionality over to web, and then integrate it with the persistence layer discussed above.

@jsayol / @knpwrs I'd love to see a sample implementation of the storage adapters concept, sounds like a solid strategy to allow for flexible browser/environment requirements.

@jsayol
Copy link
Contributor

jsayol commented Jun 8, 2017

In the iOS SDK (Github Repo: https://github.com/firebase/firebase-ios-sdk) we are synchronizing only the delta between the local device and server state.

That's very interesting. You mean that if the hashes don't match when attaching a listener, only the difference is synchronized? If so that's pretty cool, and quite different from the web SDK where the whole thing is resent in that situation.

How is it implemented? Do you traverse the tree checking the hashes at every node to figure out what's up to date and what isn't? I'm trying to locate the relevant code in the iOS repo but I can't seem to find it (and not being familiar with ObjC doesn't help either 😄).

@knpwrs
Copy link
Contributor

knpwrs commented Jun 8, 2017

@jshcrowthe Time permitting I may be able to get something done. What do you think about utilizing LevelUP as suggested in my previous comment? Obviously assumes a compatible data model. If it's not compatible then we'd need to design our own adapters.

@jshcrowthe
Copy link
Contributor

@knpwrs I looked at LevelUP and it seems like a really solid library, however I don't know that we need all that it provides. With the database already being quite large, adding another large persistence library is probably a hard sell (I just ran LevelUP through a quick webpack build, 103kb min).

Same story goes for something like LocalForage (although this one is admittedly lighter coming in at around ~25kb).

IMO I'd start w/ just the raw primitives until we need the abstraction (we are going to have to build our own abstraction layer already to allow it to be pluggable).

@jshcrowthe
Copy link
Contributor

jshcrowthe commented Jun 8, 2017

@jsayol so we currently are using a hash function that can be found here:

https://github.com/firebase/firebase-js-sdk/blob/master/src/database/js-client/core/Repo.js#L204-L236

This hash is a "simple" hash of the data in the node. We then send that hash to the server when we call listen in the PersistentConnection (see https://github.com/firebase/firebase-js-sdk/blob/master/src/database/js-client/core/PersistentConnection.js#L183)

By leveraging "compound" hashing (which is a hash of key ranges instead of the entire node, iOS implementation found here: https://github.com/firebase/firebase-ios-sdk/blob/master/Firebase/Database/Core/FCompoundHash.m) we could minimize traffic over the wire. We would just need to implement the ability to merge the range updates that we receive with what we already have in memory. (see https://github.com/firebase/firebase-ios-sdk/blob/master/Firebase/Database/Core/FRangeMerge.m)

All that said, I think the right first step is to allow for persistent offline through IndexedDB (or an adapter structure), and then work towards this.

@jsayol
Copy link
Contributor

jsayol commented Jun 8, 2017

Thanks for the links @jshcrowthe!

I knew about the hash function (a few months ago it took me a while of digging through minified code to figure that one out :P) but I had no idea about the whole compound hashing implementation. I'll definitely look into it!

I agree with you though, none of it will be very useful without persistence so let's focus on that first. I think a solid first approach would be to simply use IndexedDB, since that would cover most use-cases. (Safari's implementation of IndexedDB is known to have issues though, so it might be worth looking into WebSQL too. Maybe. I don't know.)
If we also want to support Node.js then we'd have to look into other options too, but having direct access to the file system opens a whole lot of other possibilities there.

You raised a valid point in a previous comment about bundle size. Ideally we'd keep this change as small as possible but if it ends up getting too large for comfort it could just be implemented into its own sub-module, as an optional feature to be added by the user if they want to use persistence. Something like this:

const firebase = require('firebase/app');
require('firebase/database');
require('firebase/db-persistence');

I'll start looking into how IndexedDB could fit in into the current implementation. Off the top of my head, we'd have to build a system to consistently synchronize the contents of the MemoryStorage with what's being persisted, and probably ensure we're not hitting persistence too often during read or write bursts to avoid performance issues. This synchronization could happen after a certain time of inactivity on the database, like for example 10 seconds, with a maximum interval of time between operations to minimize the risk of ending up with stale data in the event that the app would crash or suddenly be shut down somehow.

Thoughts?

P.S.: I still think the storage adapter idea is an interesting one that can be added later, but basic browser persistence should be provided by the SDK out of the box anyway.

@PierBover
Copy link

To me the best case scenario for true offline support would be if it was completely transparent. Downloaded data would be available from persistent storage, and new data would be written to persistent storage and synched automatically once the device is online again.

I'd like to add (since I haven't seen this mentioned) that using localStorage on mobile is not very persistent since it can be deleted at any time by the OS. This would be much more inconvenient on Cordova, React Native, NativeScript, apps. Users expect more persistence from an app than a website.

I agree that right approach would be having an API and write adapters on separate modules (official or third party) to reduce bloating on the main SDK.

@PierBover PierBover mentioned this issue Jun 10, 2017
@paulpv
Copy link

paulpv commented Jan 20, 2018

@callagga Yes, I do get two onSnapshot when using .onSnapshot({ includeQueryMetadataChanges: true }, (querySnapshot) => { ... }), and one when not using { includeQueryMetadataChanges: true }

@PierBover
Copy link

I'm looking into solving offline again for a crossplatform web app (mobile, desktop, Chrome OS).

So, persistence hasn't been solved yet for the RTDB, right?

I also took a look at Firestore but I saw this in the docs:

For the web, offline persistence is an experimental feature that is supported only by the Chrome, Safari, and Firefox web browsers.

Which doesn't inspire much confidence to be honest... and knowing Edge is not supported is a deal breaker for us which represents about 25% of our users.

Is there work being done on offline persistence for Firestore for the web or is this feature going to remain "experimental"?

@mikelehen
Copy link
Contributor

@PierBover Firestore web persistence is definitely under active development. Right now we're focused on implementing multi-tab support. The Edge issue is unfortunate. If Microsoft implements https://developer.microsoft.com/en-us/microsoft-edge/platform/status/indexeddbarraysandmultientrysupport/ then we should be able to support Edge easily (feel free to add some upvotes to their roadmap :-)). Barring that, we're going to have to rework how we store / index our persisted data in order to work around the limitation.

@PierBover
Copy link

Thanks for your fast answer @mikelehen !

For my use case multi-tab support is not a priority, but Edge support is.

And what about offline persistence for the RTDB @jsayol ?

@jacwright
Copy link

I am also hoping for RTDB persistence. I have a custom solution using IndexedDB and will be using https://github.com/dabblewriter/tab-election for multi-tab support so that only one tab commits updates to the database. You could use that (or something like it) and have a single tab be the one with all the watches. Sorry I can't help contribute more to this!

@PierBover
Copy link

Thanks @jacwright my app will not run in multiple tabs. We are working on our own native wrappers using the web engine of each OS (Android, iOS, Mac, Windows).

@jacwright
Copy link

I am currently caching data myself and I wonder if we could expose the hashing in Firebase to at least allow it to be taken advantage of outside the persistence feature. I would really like to take advantage of the bandwidth savings and am happy to control the caching myself.

@awardrop
Copy link

Folks, I have been looking into this for some time now and the answer is clear but not what we want to hear.

Browsers "parse" JS and thus we rely on browser technology for local storage as we do not store data on disc.

Android / OS compile onto the disc if necessary so the access to disc storage and light databases like sqlite is possible.

Browser local cache or localdb or indexeddb only allow 4mb of data per app before recycling old data... So ... Offline first would mean user cannot create more than 4mb of data. This is not realistic in the long term.

You could choose to only make certain tables offline first.... But even then it's unclear if they might one day loose old data and sync will ruin your cloud store as well

The only option is to make your site into an online portal with more functionality and allow offline for your android app... Both feed the same node API.

I am waiting for a miracle in browsers but don't loose any sleep on it
Hope this helps

@jacwright
Copy link

Don't lose heart @awardrop!

The browser limit is 50% of remaining disk space (https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API/Browser_storage_limits_and_eviction_criteria) which is much more than 4mb in most cases. And in the cases it is not, you can let the user know to clean up disk space for offline functionality when getting the QuotaExceededError.

In addition, if firebase is ejected because it has been unused for awhile this is certainly ok. The main reasons to cache the data locally are:

  1. increase the speed of startup and data-load
  2. allow for offline usage
  3. decrease bandwidth usage and costs

If you have to fetch data from the server because it has been ejected since last fetch, that is certainly an acceptable situation. Offline storage enhances an app, but the data still exists elsewhere so you're not in trouble if it times out. You're only in trouble if you only store it locally and not anywhere else.

I'm storing all my data locally in indexeddb and syncing it to firebase. I would love to take advantage of the bandwidth savings persistence mode provides, but I realized yesterday that if I timestamp all the saves I can use a query to only get updates newer than last fetch (and I can store the last timestamp in local storage or indexeddb).

I think most people on this thread of come up with workarounds or other solutions for this problem and are past the point where they need persistence. But newcomers will certainly benefit from the feature (and we all will on our next app) so I still feel there is a lot of value to it. I just wish I could commit time to solving it rather than onlyt contributing to the discussion.

@PierBover
Copy link

What we ended up doing in previous projects is to abstract the persistent storage medium depending on the platform for Cordova, Electron, or UWP web apps. Basically we ended up saving .json files to disk (which are truly persistent) with the complete Vuex state of our Vue app.

It's rudimentary... but it works. We have a couple thousands users with very bad connectivity that remain offline like 80% of the time.

@awardrop
Copy link

@jacwright very true... I didn't mean to sound pessimistic :D.

The only thing I can't wrap my head around is the security layer when working with on disc files @pierbober... Was wondering if you could share some pearls of wisdom down on us mere mortals about how you managed to make the local database on client side secure (given its fully under their control potentially)... It's the last piece of my puzzle

@PierBover
Copy link

Was wondering if you could share some pearls of wisdom down on us mere mortals about how you managed to make the local database on client side secure

I think you may be overestimating what we did... I'd like to clarify that we do not have the entire DB there, only the state of our app from Vuex (like Redux for Vue).

On iOS, Android, and Chrome OS the user has no access to the JSON files we are saving.

On Mac and Windows (via Electron or UWP) we simply encrypt the files before writing to disk and decode them in memory.

@motin
Copy link

motin commented Sep 18, 2018

Is this thread about implementing offline persistence or true offline first capabilities? Ie, is the target to be able to add and query items in a locally persisted Firestore-compatible database without requiring any network activity or even a cloud Firestore database setup?
With PouchDB, this is completely possible since remote syncing is optional and can be added at a later stage.

@mikelehen
Copy link
Contributor

I believe this was talking about adding offline support to the Realtime Database SDK (not the Cloud Firestore SDK, which already supports web offline).

FWIW- In theory you could use the Cloud Firestore SDK 100% offline by calling firebase.firestore().disableNetwork() but it's not really optimized for this use case right now, so performance may degrade over time.

@Ross-Rawlins
Copy link

Is there a solid answer for how to use offline persistance with real time Db yet? I had to upgrade my angular app and the old angularfire2-offline has been depricated. And to move over from RTDb to firestore I cant do just yet.

@jacwright
Copy link

I have come up with a system to do it myself adding a modified field on every record. Then using indexeddb and the time offset I keep the local records and the remote records in sync whenever online. This reduces the traffic by only grabbing everything that has been modified since I last asked (I store the last modified timestamp received in an indexeddb store too). It's been working well, but having built-in support like Firestore does would be much nicer. It took me awhile to put this together and puts certain requirements on my data structure.

@jsayol
Copy link
Contributor

jsayol commented Nov 8, 2018

Hi everyone. I started looking into this again to see if we can move it forward. I’m gonna put some thoughts into writing here, along with some of the decisions I’ve made so far. Please feel free to comment about any of it and ask any questions you have.

(cc @mikelehen @schmidt-sebastian)

Data storage

There was some concern about how data is stored internally. My current implementation follows the same approach as in the iOS sdk, where an entry is created for each value with its full path as the key. As mentioned, this results in many entries with long keys and usually small values. That's fine in the iOS sdk because it uses LevelDB, which uses prefix compression on the keys.

I've looked into it and both Blink (Chrome, Opera) and WebKit (Safari, Chrome iOS) use LevelDB as the underlying engine for IndexedDB so we're probably good there, although we should still do some profiling to be sure the performance is acceptable. I couldn't find any information about Gecko (Firefox) nor EdgeHTML (Edge).

As for other platforms, like React Native, I think we should offload the decision on how to store it to their specific StorageAdapter implementation. The "core" could still keep using these deep keys while, depending on what's most efficient in that platform, the specific storage implementation might decide to group several entries that share a common prefix into a single value. When retrieving data from storage, the persistence manager already makes a single request to the storage adapter to get all the entries whose keys begin with a certain prefix (the path we want) so it should be fairly trivial to use an alternative approach in each adapter.

That aside, I don't see any easy alternative to the current implementation. Even though IndexedDB is our best option as a default, it's really not ideal to store arbitrary JSON-like data and that imposes certain limitations. We can't just dump the whole thing into a single entry since that would be very inefficient, so data needs to be split somehow. But we don't really know how to split that data other than by its deepest key. In the long run we could implement some heuristic to determine where to split/group (for example, by determining which paths are commonly written to or read from, among other things) but in the meantime our best approach is to just keep using deep keys. Obviously I’m open to any suggestions here, of course.

Multi-tab access

This was another concern back when we were discussing this last year. Since it has already been solved in Firestore, I think the best approach here will be to mirror their solution. To keep things simple, though, I will probably begin by not allowing persistence to be enabled in more than one tab. This can be achieved by using a minimal implementation of what's currently being used in Firestore, using LocalStorage to coordinate between tabs. Once an initial version is stable and working we can look into adding proper multi-tab support later on.

Cache policy

To prevent the cache from growing too big (and thus increasing the chances of the browser nuking all persisted data) I implemented a “Least Recently Used” cache policy, with pruning triggered when the persisted storage reaches a certain size.

With IndexedDB, though, there’s no direct way to obtain the size of the database so I ended up following this approach as an approximation:

  • If it's a number, count 8 bytes.
  • If it's a string, count its length in bytes.
  • If it's a boolean, count half a byte.
  • If it's an array, loop its elements and apply the same algorithm recursively.
  • Also, for each entry add half the length of the key to the count.

I’ll need to do some testing to see if it’s a good approximation or whether it needs some tweaking. Any suggestions as to how to make it more accurate are definitely welcome.

packaging

When I implemented most of this last year I put persistence into its own separate module, in order to limit the impact this change would have on the size of the database bundle. Since the internal structure of the SDK has changed quite a bit these last few months, when I adapted my changes into it I opted to just put it into the database package for now. That also seems to be the approach followed by Firestore, but if you think it would still be a good idea to have it separate (something like @firebase/database-persist) let me know. This can be decided further down the road, though. There’s plenty of work to do before this is of any concern.

@mikelehen
Copy link
Contributor

mikelehen commented Nov 14, 2018

Hey @jsayol,

Good luck with this!

Data Storage
If you wanted to pursue an approach to avoid many entries with small values, you could take a look at what we do on Android. Each row contains a tree node (possibly containing an entire subtree). If the node would be too large, then we split it into multiple rows. This works pretty well, though breaks down if you have large lists of very small nodes. The list will be too big to fit in a single node, but the individual nodes are very small, leading to poor storage efficiency. Anyway, for more details see https://github.com/firebase/firebase-android-sdk/blob/c9f213cac589595580a04a89784a44e0ff19c39b/firebase-database/src/main/java/com/google/firebase/database/android/SqlPersistenceStorageEngine.java#L70 and https://github.com/firebase/firebase-android-sdk/blob/master/firebase-database/src/main/java/com/google/firebase/database/android/SqlPersistenceStorageEngine.java#L842. But you may as well stick with what you're doing initially and see if it's a problem.

Multi-tab access
The key to the Firestore approach is a "lock" in IndexedDb. Every tab assigns itself an ID and one tab writes its ID to the "lock" object store in IndexedDb. Then in every subsequent transaction it verifies that it still holds the lock before doing any writes. To guard against crashed tabs holding the lock there's also a timestamp in the lock and other tabs can take over the lock if it's too stale. Even with multi-tab we use a similar approach to nominate one tab as the "primary."

Cache policy
That sounds very reasonable. We do something similar in Android: https://github.com/firebase/firebase-android-sdk/blob/c9f213cac589595580a04a89784a44e0ff19c39b/firebase-database/src/main/java/com/google/firebase/database/core/utilities/NodeSizeEstimator.java#L34

@jsayol
Copy link
Contributor

jsayol commented Nov 24, 2018

(Update at the bottom)


Hi there.

I just remembered that back when I was implementing this last year I ran into an issue that could become a potential breaking change.

Some background: when a new listener is attached with the current implementation, either via .on() or .once(), then SyncTree.addEventRegistration() returns a list of events to be raised immediately (synchronously) based on the data that can already be found on the in-memory cache.

By adding persistence into the mix, now addEventRegistration() needs to account for the possibility that it might need to access the disk cache, which is asynchronous. This means that now it needs to return a Promise that resolves to that list of events instead.

With that in mind, take the following code from one of the current Transaction tests:

it('New value is immediately visible.', function() {
  const node = getRandomNode() as Reference;
  node.child('foo').transaction(function() {
    return 42;
  });

  let val = null;
  node.child('foo').on('value', function(snap) {
    val = snap.val();
  });
  expect(val).to.equal(42);
});

I've already modified several other tests that were directly inspecting the list of raised internal events synchronously, since that is not a real use case. But this example is different since people's code might be currently relying on this behavior. It's uncommon, but I'd say it's perfectly valid to do that if you know the value you're looking for has already been cached.

If you don't want to introduce a breaking change here, which is perfectly understandable, the only alternative I see would be to revert addEventRegistration() back to synchronously returning the list of events to be raised based only on the in-memory cache, and then asynchronously raise any events based on the disk cache. From a practical point of view, it would behave just as if new data had come from the server after attaching the listener.

Would that be an acceptable solution?


Update: I went ahead and made these changes, since it actually seemed like the only option really. I guess I needed to put it in writing to realize ¯\_(ツ)_/¯

@mikelehen
Copy link
Contributor

Yep! Agreed that's the only option. 👍

@alexnu
Copy link

alexnu commented Jan 26, 2019

Just wanted to say I'm still hoping for this feature. I'm quite happy with RTDB and don't see any other reason to migrate to Firestore. My only alternative is to implement my own caching system which won't be as good as official support.

@jsayol I'm cheering for you! 👏

@alexanderwhatley
Copy link

Any progress on this feature? Would be really useful for my website.

@tomlarkworthy
Copy link

@jsayol do you have a copy of what you have so far? I am interested in seeing how much work it is. I would personally skip trying to store the data efficiently in indexDB, for my personal needs very little data beyond the user record needs to be stored, but startup time is paramount. A store ineffecient offline first implementation would be better than nothing!

@jdcoldsmith
Copy link

I know my company would really benefit from a feature like this! I hope it can see the light of day.

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

No branches or pull requests