Cloud Firestore Support

[mbleigh]

All right, I've done the initial import of my experimental Cloud Firestore work. I also added an analysis.json as per the new rules for making iron-component-page work (not sure if that's the right way to go). Docs should show up properly on webcomponents.org, so that's nice.

@tjmonsi this is probably as far as I'm going to have time to take this in the immediate future. I'd be grateful for your help fleshing it out further.

Todo

• Figure out if this can be ported to a behavior that is backwards compat with Polymer 1
• Flesh out the demo to actually demonstrate some of the cool things about the implementation
• Write tests

Fixes #274

[tjmonsi]

Ok will work on it this coming week :)

[stemuk]

@mbleigh @tjmonsi Any progress on this? Would love to see the final implementation in Polymerfire, the Firestore branch looks good so far!

[tjmonsi]

@stemuk sorry, was just finished doing an event last weekend and was just finishing some backlogs I accumulated. Will work on this once I am free (hopefully this weekend)

[merlinnot]

@tjmonsi there are couple of issues I'd like to work on, especially:

• Eliminating the initial lag caused by the use of get() method when persistence is enabled (it always tries to get the latest data from the server). The best solution would be to make online-only behavior optional.
• Updating only changed documents in collections instead of replacing entire arrays (eventually we could think of deep-diffing documents to notify only changed paths).
• Fixing the bug in the implementation of an initial binding (properties might have their values assigned before connectedCallback is called)

I'd like to avoid wasting my or your's time on fixing the same issues twice, shall we split these ones or do you have sth else in mind to work on over the weekend?

[tjmonsi]

@merlinnot I am still doing some backlogs so please do whatever you can. I think I am going to merge someone else's PR to the firestore branch. If it's alright, can you create an issue about the bugs that you have stated so that it's connected to the PR that you will merge to this.

My plans for this one is review the current version on how it plays with firestore and create a behavior for backwards comp, and elements firestore-query and firestore-document (still deciding). I would be changing the name of this one as well to be firestore-mixin so that it is more appropriate. Hopefully I can merge them all by the end of next week.

[ghost]

@tjmonsi for use of firestore within a dom-repeat I think the only possible way with a mixin is to use a dom-module inside the dom-repeat. With firebase-document it'll be possible to fetch a document directly inside each template.

[ghost]

At the moment the firestore-mixin is a one-way-binding with firestore. Is it for firestore technically also possible to implement two-way-binding (just like firebase-document)?

[merlinnot]

@arcodebonte It is technically possible, but it is generally discouraged to do so. It's a nice feature to show off, but it's most often an anti-pattern.

[tjmonsi]

@merlinnot thanks for setting up the issues :)

[ghost]

@merlinnot thanks for your response! Nice to hear that also the two-way-binding is possible. I agree that this binding must be used thoughful.
In some cases however I think this binding is useful. I use the two-way-binding in firebase-document for example to let the users adjust their settings. For this purpose the two-way-binding works good and it helps to keep the code clean and simple.

[tjmonsi]

@arcodebonte Let us see what's the best way to do it.

[mbleigh]

On two-way binding -- I'm pretty against it, honestly. Firestore charges based on doc reads and doc writes, whereas the Realtime Database charged only based on transferred bandwidth. So updating one field at a time wasn't a big deal in RTDB, but is in Firestore as you're triggering multiple writes. I'd be more interested in seeing demos of, for example, a "debounced save" where changes are tracked and persisted after 500ms of inactivity, for example.

…

On Fri, Oct 20, 2017 at 2:29 AM Toni-Jan Keith Monserrat < ***@***.***> wrote: @arcodebonte <https://github.com/arcodebonte> Let us see what's the best way to do it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#278 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAD_ncQiKPRd_s7TM2z7jeQTOAQ0GtLks5suGf2gaJpZM4PxKeQ> .

[googlebot]

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

[tjmonsi]

@mbleigh I see your point as well. The auto 2-way binding will have bad consequences on every minuscule update. I also like your idea about a debounce of 500ms but I don't know 500ms enough. That would mean every short type burst of here and there would still create a significant number of writes instead of one good write.

It might take like a really complicated solution (like caching all edits first and then until a threshold has been reached (either by time that is longer or a change in location, then it auto-saves... but that's also too general)

[Westbrook]

Any chance we'll see a merge/release on these features sometime soon?

[IchordeDionysos]

Jep Bug will be fixed if code will be changed to this: https://cl.ly/220W2O0r1U0A

[IchordeDionysos]

Isn't here the query missing.
Shouldn't it, so it's available in this line 194.
I looked through the code because I was wondering why queries are never called and realised that query is never assigned to the config.

Is this a correct discovery?
Would love to see this fixed if so.
:)

[merlinnot]

#303

[IchordeDionysos]

@merlinnot Is there a version which fixes this issue and can be used for internal tools?

[merlinnot]

@IchordeDionysos Not really. I've made quite a few PRs to fix various issues in a Firestore mixin, but repo owners are really unresponsive 😢 Take a look at #274.

[derhuebiii]

It might take like a really complicated solution (like caching all edits first and then until a threshold has been reached (either by time that is longer or a change in location, then it auto-saves... but that's also too general)

@tjmonsi Did you progress on this thoughts?
When I think those options through, they seem to be very app specific (e.g. when editing a document, save when document looses focus + after a certain amount of time + window looses focus)...
I think for me it will end up in a mix of Firebase and Firestore, using Firebase only for storing documents (which kind of is weird, but luckily I don't need them to be indexed or queried).
Unfortunately it looks like all the great features of Firestore are on heavy cost of the realtime benefits of Firebase :(

@merlinnot
Another question:
Does the FirestoreMixin already support some way to add documents or collections? I checked but I couldn't figure out.
The array that we get for the collection looses the add functionality from the Firestore.js object, right?

db.collection("cities").add({
    name: "Tokyo",
    country: "Japan"
})

https://firebase.google.com/docs/firestore/manage-data/add-data#add_a_document

[merlinnot]

Hi @derhuebiii, welcome onboard, nice to see someone interested in this PR :)

There is actually some support for this. Given

static get properties() {
  return {
    someProp: {
      type: Array,
      collection: 'someFirestoreCollection',
    },
  };
}

a property named somePropRef is created which is a reference to the collection you've defined (literally the same one that is being used in the mixin). Tiny example:

async addSomeStuffToTheCollection() {
  return this.somePropRef.add({someStuff: true});
}

Both docs and collections have these properties, the Ref (note the capital letter) suffix is appended to the name of a property.

[derhuebiii]

Thanks @merlinnot ! I will play around with this. Also thanks for creating this branch!

Might make sense to add this to the documentation. I am not so familiar with participating on github but after playing around, I might try to suggest some edit.

[merlinnot]

Actually it's not me who created this branch, it's @mbleigh.

Documentatio is a part of this PR, see https://github.com/firebase/polymerfire/pull/278/files#diff-ed26b76bc80e40b84633288e109db714R90

[Westbrook]

Question here around whether the mixin should defensively prevent the binding of the DB when propArgs[n] === '' as well as when propArgs[n] === undefined? Theoretically it can be programmed around, and it publishes a clear error up the line, but it seems like here's a pretty solid place to prevent complaints, as well as make possible some potentially useful patterns.

[halloweenman]

The mixin doesn't seem to work with IE11 - just get a blank page when using mixin with latest version of Polymer Starter Kit and using polymer build --js-compile option. See #341

[JesseCorbett]

Has there been any progress on this front? I'm looking into adding firestore support to an existing Polymer app and would greatly prefer to use a polymerfire solution rather than roll my own or use a less well supported resource.

[tjmonsi]

I'm finishing it up soon. You can see the progress here: #346