polymerfire icon indicating copy to clipboard operation
polymerfire copied to clipboard

Cloud Firestore Support

Open mbleigh opened this issue 7 years ago • 22 comments

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

mbleigh avatar Oct 07 '17 00:10 mbleigh

Ok will work on it this coming week :)

tjmonsi avatar Oct 07 '17 07:10 tjmonsi

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

stemuk avatar Oct 16 '17 11:10 stemuk

@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)

tjmonsi avatar Oct 18 '17 16:10 tjmonsi

@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?

merlinnot avatar Oct 19 '17 21:10 merlinnot

@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.

tjmonsi avatar Oct 20 '17 03:10 tjmonsi

@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 avatar Oct 20 '17 08:10 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)?

ghost avatar Oct 20 '17 08:10 ghost

@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.

merlinnot avatar Oct 20 '17 09:10 merlinnot

@merlinnot thanks for setting up the issues :)

tjmonsi avatar Oct 20 '17 09:10 tjmonsi

@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.

ghost avatar Oct 20 '17 09:10 ghost

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

tjmonsi avatar Oct 20 '17 09:10 tjmonsi

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 < [email protected]> 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 https://github.com/firebase/polymerfire/pull/278#issuecomment-338155786, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAD_ncQiKPRd_s7TM2z7jeQTOAQ0GtLks5suGf2gaJpZM4PxKeQ .

mbleigh avatar Oct 20 '17 16:10 mbleigh

So there's good news and bad news.

:thumbsup: 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.

:confused: 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.

googlebot avatar Oct 21 '17 09:10 googlebot

@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)

tjmonsi avatar Oct 21 '17 10:10 tjmonsi

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

Westbrook avatar Dec 16 '17 14:12 Westbrook

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

derhuebiii avatar Feb 20 '18 14:02 derhuebiii

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.

merlinnot avatar Feb 20 '18 18:02 merlinnot

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.

derhuebiii avatar Feb 21 '18 09:02 derhuebiii

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

merlinnot avatar Feb 21 '18 10:02 merlinnot

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

halloweenman avatar Apr 30 '18 19:04 halloweenman

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.

JesseCorbett avatar May 19 '18 09:05 JesseCorbett

I'm finishing it up soon. You can see the progress here: https://github.com/firebase/polymerfire/issues/346

tjmonsi avatar May 19 '18 16:05 tjmonsi