[cloud_firestore] Added Flutter web support

[amrfarid140]

Description

• Created cloud_firestore_platform_interface to host A common platform interface for the cloud_firestore plugin.
• Created cloud_firestore_web to host the web implementation for Firestore
• Changed cloud_firestore implementation to delegate to method_channel_xyz in cloud_firestore_platform_interface when necessary
• Relocated tests from cloud_firestore to cloud_firestore_platform_interface

This PR is created for gathering feedback on the approach and check mergeability.

Todo items
This PR is created for gathering feedback on the approach. The remaining steps are

• create PR and merge cloud_firestore_platform_interface
• create PR and merge cloud_firestore_web
• update cloud_firestore to use published version of cloud_firestore_web & cloud_firestore_platform_interface
• Add unit tests for cloud_firestore_web
• Add unit tests to verify that cloud_firestore delegates method calls to delegate
• Add unit tests for cloud_firestore_platform_interface
• Address PR comments and feedback
• Delay FirebasePlatform._instance initialisation until it's called for the first time (lazy init)
• Handle encoding/decoding DocumentReference
• Fix Query method delegation (currently broken ☹️)
• Add support for Transaction
• Add support for WriteBatch
• Web & mobile specific implementation for GeoPoint
• Web & mobile specific implementation for Blob
• Fix settings bug mentioned here [cloud_firestore] Added Flutter web support #1670 (comment)

Related Issues

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

• I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
• If the pull request affects only one plugin, the PR title starts with the name of the plugin in brackets (e.g. [cloud_firestore])
• My PR includes unit or integration tests for all changed/updated/fixed behaviors (See [Contributor Guide]).
• All existing and new tests are passing.
• I updated/added relevant documentation (doc comments with ///).
• The analyzer (flutter analyze) does not report any problems on my PR.
• I read and followed the [Flutter Style Guide].
• I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy].
• I updated CHANGELOG.md to add a description of the change.
• I signed the [CLA].
• I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
• No, this is not a breaking change.

[collinjackson]

Looping in @ditman @hterkelsen who have been working on similar functionality in #1633

[ditman]

Wow, thanks for doing this! I'm taking a look!

[amrfarid140]

No probs @ditman 👍 . It might fail builds on CI as packages such as cloud_firestore_web & cloud_firestore_platform_interface are not published yet. If we change it to relative paths instead of versions then it works. I am using this fork on my personal app at the moment.

The only thing that I am not unsure about is Transaction delegation work. As mentioned in the PR details, I am still in the process of adding tests for the new functionality.

Let me know if you have anything.

[ditman]

@amrfarid140 this is fantastic, I've been working with firestore for a while, and I appreciate the amount of work you put here!

I've added a bunch of comments but I didn't find anything major, (maybe other than the Transaction stuff, but I read that you're still working on getting that part tested).

I think we can get this merged with just a little bit of work. Let me know if you're interested in work on this PR some more!

[ditman]

It might fail builds on CI as packages such as cloud_firestore_web & cloud_firestore_platform_interface are not published yet.

@amrfarid140 Yes, there's also some failures on analyze and format that need to be fixed in the PR, those are related with the coding style (but for example, dartfmt is an automated fix).

The toughest part to fix analyze is going to be (probably) adding documentation to every public member.

The only thing that I am not unsure about is Transaction delegation work. As mentioned in the PR details, I am still in the process of adding tests for the new functionality.

What I did in PR was to leave the tests in the core package of the plugin, slightly modified to run with the new interface and make sure everything works (similar to the changes you have done when you moved the tests from the core to the platform_interface package.

(I wasn't close to working in the Web plugin, so I didn't have a good story to test the web version!)

(BTW, feel free to email me directly: dit at google if you need anything from me, I have time to help you with this).

[ditman]

@amrfarid140 I cloned your fork, and modified it a little bit to run the example of the cloud_firestore package and it loaded! However when clicking on the + icon I got an exception.

The error comes from the jsify util in package:firebase.

It seems that the web types (maybe: 'DocumentReference', 'FieldValue', 'Blob', 'GeoPoint') need to extend the Firestore package types as well, look at _jsify implementations here.

Uncaught (in promise) Error: Invalid argument (dartObject): Could not convert: Instance of 'FieldValue'
    at Object.throw_ [as throw] (errors.dart:196)
    at Object.jsify (utils.dart:120)
    at utils.dart:94
    at IdentityMap.from.forEach (linked_hash_map.dart:23)
    at Object.jsify (utils.dart:93)
    at firestore.DocumentReference._fromJsObject.set (firestore.dart:348)
    at firestore_web.DocumentReferenceWeb.new.setData (document_reference_web.dart:11)
    at cloud_firestore.DocumentReference.__.setData (document_reference.dart:47)
    at cloud_firestore.CollectionReference.__.add (collection_reference.dart:50)
    at add.next (<anonymous>)
    at runBody (async_patch.dart:86)
    at Object._async [as async] (async_patch.dart:125)
    at cloud_firestore.CollectionReference.__.add (collection_reference.dart:48)
    at main.MyHomePage.new._addMessage (main.dart:65)
    at _addMessage.next (<anonymous>)
    at runBody (async_patch.dart:86)
    at Object._async [as async] (async_patch.dart:125)
    at main.MyHomePage.new.[_addMessage] (main.dart:64)
    at _InkResponseState.new.[_handleTap] (ink_well.dart:705)

So close!

[erparker]

@ditman ah ha! that did it!
Adding this to index.html did the trick.

<script>
  // Your web app's Firebase configuration
  var firebaseConfig = {
    ...
  };
  // Initialize Firebase
  firebase.initializeApp(firebaseConfig);
</script>

I had that code in my MyApp() widget code to start the firebase instance, but I guess it needs to be inside script tags. Not sure how I missed that detail, but that did the trick

thanks again!

[ditman]

@erparker Glad you got it working! Programmatic initialization should be doable, but for now we're recommending the script tag method (which is what is also recommended by the Firebase console as well, so at least in that part we're identical to the JS SDK).

[erparker]

One other strange thing I noticed - after adding firestore web and re-deploying my site, Chrome wouldn't display it and kept asking me if I wanted to install the PWA version of the app instead (which was just a blank screen as well). I removed firestore web from the project, re-deployed, and things were fine again.
I'm stumped once again on this one. No clue if it's related or not, and I don't really know much about progressive web apps, just thought I'd pass that along

[ditman]

@erparker there was some weird interplay between Flutter web's service worker and the firestore requests that got solved a few days ago. Can you try upgrading your flutter channel to master (if you're not there already), cleaning your build and rebuilding your app again?

Note, that in that blank-screen case, you should look for clues/errors in the Javascript DevTools console, if any.

[erparker]

Things are working again. I was already on latest Flutter, Flutter 1.15.3-pre.58 • channel master , what i did was remove some code that was generated with this project from a couple Flutter versions ago. This is what I took out

<script>
    if ('serviceWorker' in navigator) {
      window.addEventListener('load', function () {
        navigator.serviceWorker.register('/flutter_service_worker.js');
      });
    }

</script>

I removed it on a whim since that issue you linked talked about a service worker, and it got things moving again. To be honest, I don't know what the service worker does, so no idea if removing it was a bad thing ¯_(ツ)_/¯ but things are running again with firestore_web in the project

[Levi-Lesches]

¯_(ツ)_/¯

You dropped something: \

I don't know what the service worker does

Service workers help manage the website even when the client is offline, along with other stuff browser-related.

[ditman]

Merged and published cloud_firestore 0.13.2 with web support! Thanks @amrfarid140 for the super hard work!!

[bean5]

Really? I'm going to test this out!

[ditman]

All you need to depend on your App's pubspec, with a fairly recent version of flutter, is:

cloud_firestore: ^0.13.2

No need to specify dependencies on cloud_firestore_web, the platform interface, or anything else.

[ditman]

For a quick test, the example app of the cloud_firestore package should be runnable with flutter run -d chrome

[bean5]

So not yet on flutter's stable channel, but dev channel?

[ditman]

@bean5 I normally use master, but anything >=1.12.13+hotfix.4 will do. IIRC beta is probably enough (Answering your question: dev should also work).

[amrfarid140]

It's amazing to see this getting merged 😄 . Thanks so much @ditman and @collinjackson for your feedback and effort in this as well.

[bean5]

So...I thought this meant that firebase_auth would be supported for web as well. Is that correct or can you use firestore without auth?

[amrfarid140]

@bean5 you can use firestore without using auth. Also, Auth supports web as well.

[ditman]

Note: In a separate PR I've updated cloud_firestore and cloud_firestore_web documentation with the latest setup instructions, and marked "Web?" as supported in the main flutterfire contents matrix.

[bean5]

I got it working. My firebase_auth package was pretty old and I was using old methods of serving web. Flutter web has come a long way since I ran into this issue.

Thanks for the great fix!

[erparker]

Anybody else start getting this error when running their flutter web app with cloud_firestore_web?
I didn't change anything beyond doing flutter upgrade the other day, so I'm not sure why this started happening.

I'm using this in my pubspec

cloud_firestore: ^0.13.2+1
cloud_firestore_web: ^0.1.0+2

When I run the app I just got a blank Chrome page and this in the javascript console:

Error: Unsupported operation: Platform._operatingSystem
    at Object.throw_ [as throw] (errors.dart:196)
    at Function._operatingSystem (io_patch.dart:241)
    at Function.get operatingSystem [as operatingSystem] (platform_impl.dart:62)
    at get _operatingSystem (platform.dart:73)
    at Function.desc.get [as _operatingSystem] (utils.dart:77)
    at get isIOS (platform.dart:141)
    at Function.desc.get [as isIOS] (utils.dart:77)
    at get firebaseDefaultAppName (default_app_name_io.dart:9)
    at Object.desc.get [as firebaseDefaultAppName] (utils.dart:77)
    at get defaultAppName (firebase_app.dart:19)
    at Function.desc.get [as defaultAppName] (utils.dart:77)
    at get instance (firebase_app.dart:41)
    at Function.desc.get [as instance] (utils.dart:77)
    at new cloud_firestore_web.FirestoreWeb.new (cloud_firestore_web.dart:33)
    at Function.registerWith (cloud_firestore_web.dart:26)
    at Object.registerPlugins (generated_plugin_registrant.dart:20)
    at main$ (web_entrypoint.dart:6)
    at main$.next (<anonymous>)

It sort of looks like instantiating a new cloud firestore will call out to see what the operating system is, which is not allowed on flutter web.

here's flutter --version

Flutter 1.15.4-pre.48 • channel master • https://github.com/flutter/flutter.git
Framework • revision 8abf54f5e3 (16 hours ago) • 2020-02-12 20:45:36 -0800
Engine • revision e0ebaea590
Tools • Dart 2.8.0 (build 2.8.0-dev.9.0 e4c39721c4)

Any ideas?

[harryterkelsen]

@erparker Very odd, it seems like you are getting the configurable import for Dart VM even though you're running on web. How exactly are you running your app? BTW you don't need the cloud_firestore_web dependency, it should be pulled in automatically already.

[erparker]

@hterkelsen thanks for the quick reply!
To my knowledge, I'm not doing anything out of the ordinary. I select Chrome as the device as then click run in Android Studio (not debug, but the regular "play button").

I've updated Flutter and the AS Flutter tools recently and I've had it working since then.

FWIW here is my flutter doctor (the android warning has been there for over a year, I mainly build to iOS so I've ignored it)

[✓] Flutter (Channel master, v1.15.4-pre.48, on Mac OS X 10.14.6 18G84, locale en-US)
 
[!] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
    ! Some Android licenses not accepted.  To resolve this, run: flutter doctor --android-licenses
[✓] Xcode - develop for iOS and macOS (Xcode 11.3)
[✓] Chrome - develop for the web
[✓] Android Studio (version 3.5)
[✓] VS Code (version 1.32.1)
[✓] Connected device (4 available)

! Doctor found issues in 1 category.

[ditman]

@erparker please open a new github issue, we're spamming a bunch of people who may not be willing or able to help :)

I'm going to lock conversation in this PR, it has run its course! Thanks all!

[ditman]

Added WriteBatch support and included examples in example app. Runs on Web and Mobile. However, it always fails if we do the following

// get a document reference `docRef`
batch.update(docRef, {some_data})
batch.delete(docRef)
await batch.commit()
...

Basically if we update something then delete it (which maybe be pointless).. Firebase throws an internal error on all platforms

INTERNAL ASSERTION FAILED: Transform results missing for TransformMutation

@ditman Is that intended behaviour ?

Closing the loop on this @amrfarid140. I asked in our internal "Stack Overflow"-like system, and an engineer from the Cloud Firestore Data Plane team just confirmed that this was, indeed, a bug:

Thanks for finding this-- this was actually a bug! We've sent out a fix, which should finished being pushed out in the next couple days. Let us know if you're still seeing this issue after that.

Nice!