Firestore

[Westbrook]

trafficstars

Fixes #326 Updates #279 as per the mixin format

• Prevents race conditions on setting up the DB.
• Prevents multi-splicing on live data updating to collections bound with more than one template variable.

[googlebot]

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 all authors are ok 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.

[Westbrook]

Also, I wondered if you might have some thoughts on how to test this. Possibly there are appropriate steps towards getting Firestore permisions on https://polymerfire-test.firebaseio.com like some of these other steps? Seems like mocking the interactions here would be...arduous...unless, that is, there were already a mocking library for the Firebase libraries, 😱⁉️

[merlinnot]

I’ve actually started to work on such library, but it will probably take some serious time and I’ll probably try to reach out to some googler soon to make sure they’re not working on this ATM so my time won’t go to waiste (@laurenzlong ?) 😃

Anyways, I’ll be working on tests for this on February 27th probably, so you might want to wait couple of days and just build on it. Guys from firestore-js-sdk team are actually using the real db to run tests, so... I’ll most probably do the same and just call it “integration tests” so I won’t feel bad with myself 😜

[tjmonsi]

@merlinnot is this ok to be merged on your branch now?

[merlinnot]

Nope, unfortunately. I had some hope that I'll find some time over the weekend to write tests, reproduce this issue and fix the core problem here.

Sorry, but I can't approve it until we have some tests for this (which I'll try to write myself) + one of the changes is most definitely invalid.

[tjmonsi]

@merlinnot any updates on this? is it ok to merge to Firestore before I merge this firestore branch to master?

[merlinnot]

Hey, I went through the changes once again and I’m simply not sure.

I have a few thoughts tho:

1. This PR changes both database and Firestore, we can split it into two separate ones so it’s easier to review.
2. I won’t be ok with any changes for Firestore until we have proper tests. The code is too complicated for this. I’ve tested the current polymerfire branch in production for months, it works fine, but I no longer use it since all of my projects are now upgraded to Lit + Redux + Saga
3. I’m not convinced if we should actually release it if there is no one to take a proper care of it (the whole repo actually)
4. I have a branch with some tests and minor refactoring which probably solves the issue with registration of Firestore, but it’s not in a finished state, lacks many tests and I won’t have time in a near future to work on it, especially when I’m busy working on some stuff in Firestore itself, like this: https://github.com/firebase/firebase-js-sdk/pull/819. It would be great if someone could pick up my work on testing first, before we (possibly) merge this PR.
5. I feel like there needs to be a decision on what to do with Polymerfire. Things are kinda unclear right now, I feel like a community using Firebase with Polymer 1/2 is rather small and now, in the age of Lit and Polymer 3 (which is slowly dying too...), it will get even smaller. Maybe it should be simply deprecated?

We should probably move this discussion elsewhere, maybe at least create an issue. Let me know what you think.

[tjmonsi]

@merlinnot https://github.com/firebase/polymerfire/issues/353 we can discuss it here