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

Cloud Firestore Support #274

Open
JGSolutions opened this issue Oct 3, 2017 · 76 comments
Open

Cloud Firestore Support #274

JGSolutions opened this issue Oct 3, 2017 · 76 comments

Comments

@JGSolutions
Copy link

So just found out that firebase has released a new feature called firestore. Lovely (sarcastic way). I read about it and it does seem interesting and looks more powerful than the real time database. Now the question is any plans on creating polymerfirestore? LOL LOL

@mbleigh
Copy link
Contributor

mbleigh commented Oct 3, 2017

I've been prototyping something and hope to work with @tjmonsi to get it in releasable shape soon. Stay tuned!

@JGSolutions
Copy link
Author

WOW nice! Would be another component not part of the polymerfire? For example another web component called polymerfirestore?

@mbleigh
Copy link
Contributor

mbleigh commented Oct 3, 2017

TBD, as I said, stay tuned!

@JGSolutions
Copy link
Author

Thanks I will :)

@mbleigh
Copy link
Contributor

mbleigh commented Oct 3, 2017

Re-opening to leave as a single feature request.

@mbleigh mbleigh reopened this Oct 3, 2017
@mbleigh mbleigh changed the title Firestore Firestore Support Oct 3, 2017
@mbleigh mbleigh changed the title Firestore Support Cloud Firestore Support Oct 3, 2017
@tjmonsi
Copy link
Collaborator

tjmonsi commented Oct 4, 2017

Should we do it as a behavior/mixin or a component?

@stemuk
Copy link

stemuk commented Oct 4, 2017

@tjmonsi A component like firebase-query would be great since it would make the 'getting started' part much easier.

@Scarygami
Copy link
Contributor

I guess a similar approach to how it's done with the real-time database would be best.

Have a firebase-firestore-behavior as a basis, and implement firebase-firestore-document (to allow writing data) and firebase-firestore-query based on it.

With the extended query functionality (like chaining wheres/orders) it will be challenging to implement an all-purpose query component...

@Scarygami
Copy link
Contributor

Btw, I'm open to testing this as soon as you have a branch/repo public somewhere :)

@tjmonsi
Copy link
Collaborator

tjmonsi commented Oct 4, 2017

@Scarygami will ping you once I am done. (Just finishing a devfest site and some experiments)

@mbleigh
Copy link
Contributor

mbleigh commented Oct 4, 2017

FWIW, I don't know that the existing approach is going to work well for Cloud Firestore, especially due to the query complexity that is possible as compared to the Realtime Database. I'm probably going to try to pull my experimental library into a branch so everyone can take a look and see if you like my approach (it's a class mixin, not an element).

@tjmonsi
Copy link
Collaborator

tjmonsi commented Oct 4, 2017

@mbleigh yeah that's my thought as well.

@phidias51
Copy link

I'm curious what this might look like. Suppose you could simply modify the firebase-app tag to point to your firestore database. The firebase-query and firebase-document elements would still work. If you want to add pagination you would simply add a "page-size" attribute to your query, and advance the start-at pointer. This would cut down on the amount of work required to migrate over to firestore.

@mbleigh
Copy link
Contributor

mbleigh commented Oct 4, 2017

In general, if you try to treat things that aren't the same as being the same, you're going to have a bad time. Cloud Firestore is fundamentally different from the RTDB in many ways, so trying to shoehorn it will have negative tradeoffs when the square peg doesn't actually fit the round hole.

Again, I'm trying to get my experimental library in a branch for people to try out as a potential approach.

@phidias51
Copy link

Here's the problem. If anyone has an existing project, they're going to have to touch every query and document element to get this to work. There's nothing that says that there has to be a 1-to-1 correspondence between firebase elements and underlying classes and the JavaScript library.

@tjmonsi
Copy link
Collaborator

tjmonsi commented Oct 5, 2017

@phidias51 As for the firebase-app. I think I just merged a preparation for having a firestore just now at #273. But I don't think it is a good idea to change the firebase-query and firebase-document's functionality to accommodate firestore. Firebase and Firestore are very different from each other. That means we could work with having a firestore-query and firestore-document. But the problem is how to set the path just only using strings. @mbleigh already has a mixin much like PolymerRedux where you can set the path to automatically save it to a particular property, and I am going to experiment on it next week.

@imarian97
Copy link
Contributor

@mbleigh I love your implementation ;) I hope to be well received by the community and be available in polymerfire. Also, I'm writing a blog post about my experience with Polymer and Firestore. Do you have any plans to make the repo public? I want to know how much can I describe and refer your class mixin implementation in my post.

@mbleigh
Copy link
Contributor

mbleigh commented Oct 6, 2017

@imarian97 if you wait until next week I think we'll have it in a branch on this repo 😄

I'd love to see your blog post if you have a way to share drafts before you publish.

@imarian97
Copy link
Contributor

Perfect, I'm planning to publish it next week and for sure I'll share it before

@mbleigh mbleigh mentioned this issue Oct 7, 2017
3 tasks
@mbleigh
Copy link
Contributor

mbleigh commented Oct 7, 2017

All right, I've gone ahead and taken a rough pass at porting my experimental work into PolymerFire, and you can check it out at #278. Folks who are interested, feel free to install the firestore branch and give it a spin. Feedback welcome!

@Scarygami
Copy link
Contributor

Scarygami commented Oct 8, 2017

Played a little bit with the mixin and really liking it so far.

Send a PR for an issue I ran into with firebase-app not being ready before the mixin tried to do something with firestore.

Some questions/suggestions:

  • How should the mixin best handle the persistance option? Since the option needs to be enabled before doing anything with firestore it would make sense to include a property on firebase-app which triggers firebase.firestore.enablePersistance and maybe checks for errors/fires error events.

  • The way the observes option is implemented now it requires all observed properties to be truthy before triggering a query. This seems rather strict, and I think this should be changed to checking against !== undefined, or leaving it up to the user entirely to handle the logic when constructing the query. Or there could be two different options for requiredObserves and optionalObserves but I think the first option is cleaner.

  • The element strips away the snapshots, only returning the data. I've wanted to implement pagination, but that requires a documentSnapshot for startsAfter. Including the snapshot similiar to how you include the __id__ would be a possibility. This could also be done by giving another option to the users whether they want to have data or snapshots returned. Additionally having the reference of the document would be helpful in some cases as well, e.g. for easy access to subcollections.

I leave these topics up for discussion and can send PRs for ideas we agree upon.

@tograd
Copy link

tograd commented Oct 9, 2017

I was a little anxious how it would work as a mixin, but the solution seems really neat and tidy. I like not having to add non-rendering dom.

* * <propname>Ready: will be true when all path segments are present and data has been fetched at least once Been a long-standing complaint of mine for firebase-query and firebase-document that you couldn't figure out if request hadn't been done yet or just had no data. So this is really appreciated!

@imarian97
Copy link
Contributor

@mbleigh Here is my blog post on our new blog (built with Firestore and polymer) 😄 https://goldielabsite.firebaseapp.com/post/firestore-new-blog. We are planning to publish it tomorrow (to our domain: goldielab.com) and I hope you like it.
I'm looking forward to receiving feedback

@PFElements
Copy link

So far looks very promising. Kudos.

@tjmonsi
Copy link
Collaborator

tjmonsi commented Feb 21, 2018

I'll merge this to master by tomorrow. Then release a new version.

@Westbrook
Copy link

I'd love to get an official release with Firestore in it. However, with this main branch I continue to get the following error:

"app/no-app"
message: "Firebase: No Firebase App '[DEFAULT]' has been created - call Firebase App.initializeApp() (app/no-app)."
name: "[DEFAULT]"
stack: "[DEFAULT]: Firebase: No Firebase App '[DEFAULT]' has been created - call Firebase App.initializeApp() (app/no-app). ..."
__proto__: Error

When I apply a version of the fixes available in https://github.com/firebase/polymerfire/pull/279/files this goes away.

@JaySunSyn
Copy link

I'm referring to #274 (comment)

As @Scarygami mentioned, the element strips away the snapshots, only returning the data. I want to fetch subcollections and need the path of a document which I could get from the snapshot.

Are there any plans to do so?

Many thanks :)

@MarcMouallem
Copy link

MarcMouallem commented Feb 27, 2018

@merlinnot I'm a little confused on why you went with a Mixin to bind Polymer properties to database value(s) rather than use Polymer elements as was done with Firebase Realtime Database?

@tjmonsi Is this how it's going to be done moving forward?

@merlinnot
Copy link
Contributor

@JaySunSyn
At the first glance it looks like a very legitimate use case, I think that's a thing we'd like to support. Please make an issue for this with an expected behaviour etc., I'll try to dive into this in the coming weeks (tests are my #1 priority ATM).

@MarcMouallem
It was originally @mbleigh idea, but I fully support it. There are couple things to consider here (I'll name just a few):

  1. Mixins are much more efficient than elements.
  2. It enables us to provide an API for Firestore features which would be rather hard to expose and maintain in any reasonable way. Take a look at query and observe... We're passing in the element as an argument so you can read all of the props you need. How would you reproduce such behaviour if it was an element?
  3. It's much easier to build your own stuff on top of this (as an example, I use it in one app to manage the "loading" state, I've just extended this mixin, hooked up to one or two methods and it works like a charm)
  4. We can add some things to the scope of your element by default, so you can use it straight away (see *Ready, *Ref). We wouldn't be able to do so using an element (I mean we technically could..., but that would be sooooo dirty...).

@FluorescentHallucinogen
Copy link
Contributor

IMO, “mixin vs. elements” is the wrong question. Why not both? IIUC, e.g. Firebase Realtime Database support in PolymerFire is implemented as Polymer.FirebaseDatabaseBehavior mixin and <firebase-document> and <firebase-query> web components on top of it, Cloud Storage for Firebase support is implemented as Polymer.FirebaseStorageBehavior mixin and <firebase-storage-ref>, <firebase-storage-upload-task> and <firebase-storage-multiupload> web components on top of it.

On the one hand, there is an extra performance cost for registering web component as an HTML element, DOM declaration and data binding. Every property does come at a cost (observers, listeners, etc.) that are not necessary for working with Cloud Firestore.

But on the other hand, web components are extremely easy to use. Avoiding web components breaks the original Polymer's “There's an element for that” idea.

@FluorescentHallucinogen
Copy link
Contributor

Please remember that Polymer was initially conceived as a library that helps create declarative, encapsulated, reusable elements, and use them to break apps up into right-sized components, making code cleaner and less expensive to maintain (https://developers.google.com/web/updates/2014/01/Chrome-Dev-Summit-Polymer-declarative-encapsulated-reusable-components).

One of the key words here is “declarative”.

Declarative code has powerful advantages, which all boil down to hiding complexity and exposing it simply. With declarative code, we tell the computer what to do, not how to do it. That makes declarative code easier to understand and therefore to create. Anyone can tell someone what to do. Telling someone how to do it can be much harder. What's more, describing programs in chunks of what is not only easy, it takes less code. Declarative code opens up the world of programming to more people, and helps those people do more while expending less time and effort. Web components help lower the barrier to entry for building complex, well-structured apps.

See the following amazing article for more info: https://scotch.io/bar-talk/universal-web-components.

If you think that web components are not suitable for non-visual elements (there is an extra performance cost for registering web component as an HTML element, DOM declaration and data binding; every property does come at a cost (observers, listeners, etc.) that are not necessary for working non-visual elements), you are wrong. Yes, many of the web components available now are visual, but not all. Take a look e.g. at <iron-ajax>, <app-route>, etc. These aren't visual, but they are extremely useful.

As the layer we're using for composability is the DOM using attributes for our APIs, our elements will be significantly more interoperable than they've been in the past. Imagine a future where elements or components written with different libraries can be used together with a level of ease. Elements will (ideally) be created with a minimal amount of script and will try to reuse the functionality of other sub-elements where possible.

The original idea of web components was: “web apps in the near future will be composed almost entirely from elements (tags)” (see https://addyosmani.com/blog/the-webs-declarative-composable-future/). You can think of web components as Lego bricks. People can build apps with no code (see https://medium.com/@ladyleet/building-apps-with-no-code-a-web-component-future-4cf29a27ccdd).

Creating mixins instead of elements killing the Polymer's philosophy behind “Everything is an element” mantra.

I can't help feeling that Polymer is moving in the wrong direction.

See e.g. the deprecation of declarative <neon-animation> elements in favor of Web Animations API and CSS Animations (https://www.polymer-project.org/blog/2017-07-12-an-update-on-neon-animation.html), and the deprecation of declarative <platinum-sw> elements in favor of Workbox, because their functionality allegedly fundamentally incompatible with web component model.

Polymer slowly killing the original idea behind “declarative” and moves in the direction of increasing complexity.

Where there used to be HTML files with a bit of JS code (all kinds of people who would not be considered programmers can and do work with HTML code), there are JavaScript files with a bit of HTML (HTML templates are now defined by providing a template getter that returns a string — not the <dom-module> and <template> elements). The migration from Polymer 1.0's declarative syntax to the ES6 class syntax in Polymer 2.0 means that people must know how to write ES6 classes in JavaScript and extend Polymer.Element base class. This is not only increases the barrier to entry for people who want to just break their HTML apps into well-abstracted components, it forces to write more code.

See https://github.com/ebidel/declarative-web-components and https://github.com/Polymer/lit-html to see the differences in the approaches.

The latest Polymer changes only confirms this idea. Importing web components by package name, rather than the path to the package (as required by the HTML specification) means that people can't anymore just drop some tags (HTML elements) to the page and run the code without requiring any special tools or build steps. This doesn't fit with Polymer's “Use the Platform” motto. But that's a whole different story…

P.S. Why GraphQL is one of the top mainstream tech to learn in 2018 (trend, buzzword)? Some people claim GraphQL is a REST killer. But why the fuss? Because it's declarative!

It gives you the power to ask (show) for what you need and get exactly that, nothing more and nothing less. One call to one endpoint is enough. You simply send a string that describes the data you want and how you want it structured and you get it back in JSON (you tell what to do, not how to do it). GraphQL queries always return predictable results. You can think about GraphQL as declarative data fetching for modern web apps.

I'm super excited to see what I can build using the declarative GraphQL data query language and declarative Polymer web components together. 😉

@ebidel @addyosmani @justinfagnani @notwaldorf @robdodson How do you feel about all this?

@tjmonsi
Copy link
Collaborator

tjmonsi commented Mar 6, 2018

@FluorescentHallucinogen

I agree with you on your point about the mixin and element but I will have to stop you there when you said about Workbox, lit-html, Web Animations API and CSS Animations.

You see not all SHOULD be in the HTML tag. I agree, it is easier to be declarative, but not all declarative parts should be in the DOM.

You see <neon-animation> uses JS which is a performance killer. Web Animation API is the standard and allows the browser to do the animation rather than JS handling it. Especially CSS, CSS SHOULD handle animation, not JS, and not HTML, ever wonder why they created styles???

And Workbox or service workers are specialized Web workers, which cannot and will not handle DOM. That means a DOM element to handle code for service worker will not work.

I am sorry if I am triggered by this but understanding what should be in the DOM and what should not be in the DOM should be clear. It doesn't mean that if it is NOT in DOM, then it is NOT declarative.

As for <link rel="import"> as oppose to import, haven't you heard about <script type="module">?? That's another way of just loading your web component. You don't need a builder to have that.

And for "This [ES6] is not only increases the barrier to entry for people who want to just break their HTML apps into well-abstracted components, it forces to write more code." I only have this to say - ES6 is THE standard. It makes it you write LESS CODE which in turn makes your app RUNS FASTER.

lit-html allows you to write it in template code in JS without much overhead from writing it like jsx. The thing with lit-html is it opens up to new avenues for HTML standard where we can use <template> in the future to also insert data in the template. https://github.com/w3c/webcomponents/blob/gh-pages/proposals/Template-Instantiation.md

The thing is, Polymer 3 still allows you to write it in HTML tag, just use <template id="name-of-component"> put it in the HTML (or load it using <link rel="import"> and then reference it from the static get template() part of your component. It still allows you to separate the declarative content structure part of your component from the imperative part of JS.

And don't think that JS is imperative only. It has declarative parts! Think functional programming and you'll get the drift.

The thing is, your post above has a lot of misleading assumptions on Polymer going away from using the platform when in fact it is using the platform

  • deprecating performance bottlenecks so that we don't use them or else we pay the consequences of having slow apps (deprecating neon-animation in favor on platform ready CSS animations and Web Animations API)
  • deprecating unusable components because they are in fact improbable when using with Service workers
  • moving to ES6 because we can now write more elegant, more declarative, and less code for browsers that are ES6 enabled which will be in the next 5 years
  • moving to using import so that we can just drop the script that activates a web component while waiting for a consensus on how to implement <link rel="import">
  • The use of <template> and adding template instantiation which is using the platform
  • and using more functional programming and class extensions which is lesser code.

The point is, if your "declarative" path is taking you AWAY from being performant on the platform, then it is should be taken out. Period.

To end, all declarative objects has its imperative internals. Which goes into my thought process on the Firestore Mixin - it is an imperative internal of the Firestore Element i'm coding (which is i am not yet done because of work)

If you looked at the code pattern here, they used firebase-database-behavior which is actually a mixin for both firebase-query and firebase-document. The idea of DRY code is done here. And I am hoping having the firestore-mixin will allow me to just reuse the code on another element that will just extend it.

So the <firestore-element> is in the works for you guys to easily use without the need of looking under the hood on how to imperatively work it with your components.

@merlinnot
Copy link
Contributor

You're definitely a little triggered, but no too much ;)

Just as an update, I'm still going to write some tests for the mixin, but I'm not entirely sure if I'll manage to do it this weekend. I have some paid work to do too ;)

@tjmonsi
Copy link
Collaborator

tjmonsi commented Mar 7, 2018

Yeah me too, and they keep piling up. I have to really release this and have to work on the firestore-element soon. And then create little scenarios to make it sure it works.

@dandv
Copy link

dandv commented Mar 8, 2018

The point is, if your "declarative" path is taking you AWAY from being performant on the platform, then it is should be taken out. Period.

By your logic, we should still program in assembler.

Optimizing for performance is short-sighted, because the platform improves all the time, and because hardware improves. We still think manipulating the DOM is slow, but @Swizec has recently pointed out that's technically no longer true.

Optimizing for developer experience will serve everyone better in the long term, and transpilers can take care of optimizing for performance under the hood.

@tjmonsi
Copy link
Collaborator

tjmonsi commented Mar 8, 2018

@dandv

By your logic, we should still program in assembler.

Nitpicking on that particular line alone would really mean what you have said, but your missing the context of it.

Optimizing for developer experience will serve everyone better in the long term

I do agree, but I do hope you put it in the context of what I said to what @FluorescentHallucinogen was suggesting: Deprecating some custom-elements over proper usage of the platform on the context of service workers, web animation api, css animations, and usage of ES6 code. The latter are far more superior in terms of both performance and user dev experience than the former custom-elements created for it and coding in ES5.

But if we really want it to be declarative, by all means, create a custom-element that will use those API to abstract the imperative part of it and make it declarative.

And I didn't say anything about manipulating DOM being slow, or not better used. All I am saying that there are new APIs involved and they are the improvements in the platform and we should use it, not fall trap on old declarative code that doesn't use the improvements in the platform. These new APIs are here to serve everyone in long term. If they think they are still imperative, create a declarative abstraction of it to help others.

@mbleigh
Copy link
Contributor

mbleigh commented Mar 8, 2018

FWIW the reason I went with a mixin instead of declarative elements is because I felt that Firestore's query model was going to end up making a declarative element fairly unpalatable. With the complex interactions of where, orderBy, etc. trying to create an attribute-based declarative model was going to take it far away from the JS API calls and likely feel more like a burden than a good developer experience.

I don't have any objection in theory from declarative Firestore elements, but feel the mixin is overall a better developer experience for this case. If someone designs a rad declarative API that feels great, I'd certainly be willing to consider the pull request (not that I have had much time to consider pull requests on this repo 😢).

@merlinnot
Copy link
Contributor

Tests are not finished yet, but I've already started and I have some progress: https://github.com/merlinnot/polymerfire/tree/fix-issue-333

I'll try to make a PR tomorrow 🛌

@maria-le
Copy link

@merlinnot:

It's much easier to build your own stuff on top of this (as an example, I use it in one app to manage the "loading" state, I've just extended this mixin, hooked up to one or two methods and it works like a charm)

Could you please show your sample code as an example of how you are using it to manage the "loading" state? I want to do the same thing and I'm finding it difficult.

@kr05
Copy link

kr05 commented May 7, 2018

@merlinnot

Tests are not finished yet, but I've already started and I have some progress: https://github.com/merlinnot/polymerfire/tree/fix-issue-333

is this still the most up to date branch? Planning on integrating this in an app and hopefully help in the development process!

@merlinnot
Copy link
Contributor

Yes, this is the most up-to-date branch. I could use some help, I'm really busy lately.

@kr05
Copy link

kr05 commented May 7, 2018

I would be happy to. Quick question though...are there any issues with collections specifically? I have the following:

users: { type: Array, collection: 'users' }

which is the correct length, but the values are undefined. I'm about to do a deep dive on the source code, but hopefully you can point me in the right direction before!

image

@kr05
Copy link

kr05 commented May 7, 2018

After some more testing, it seems the problem is only on initial load. I added a document to the collection, and surprisingly it correctly displayed. After reloading the app, all values were once again undefined.

@Westbrook
Copy link

@kr05 I've had a number of issues working with this branch and have made a number of fixes into PRs and https://github.com/westbrook/polymerfire/tree/firebase-no-empty-strings if you want to see if your issue persists therein.

@merlinnot I've been working with the above branch in production for a couple of weeks with no issues. If there is some way I could support getting those changes or others mainlined so an official release can be cut, please let me know.

@merlinnot
Copy link
Contributor

I’m using the current polymerfire#firestore in production with no issues (variety of documents and collections). The branch with tests has some changes which I made during the testing, but it’s not finished and there might be arbitrary large number of issues ;) Feel free to make a PR against my branch (the one with tests), I’ll review it ASAP (I really want to make it production-ready). We can combine it into one, well tested and really nicely written solution and make one PR upstream so it’s easier for Firebase guys to review and merge.

You can reach out to me on Slack, email, here, however you want. We can even do some pair programming if you feel like it, Google Meet or a visit in Amsterdam will do ;)

@merlinnot
Copy link
Contributor

@kr05 that would be incredibly helpful if you could add a failing test to my branch with tests. I haven’t seen such behavior before.

@kr05
Copy link

kr05 commented May 7, 2018

I have a feeling that the culprit is "assignedDocument". The argument "instance" is null. I can't say for certain if it's the cause of the problem, but I have a feeling it shouldn't be null. I'll keep trying to find a solution. As for the main firestore branch, I just installed it and my collection loads sucessfully!

@stemuk
Copy link

stemuk commented May 27, 2018

@mbleigh @kr05 @merlinnot Could someone tell me how to initialize the firebase-app component for usage with the firestore mixin? As far as I can tell the firebase-app component is only meant to work with firebase realtime database, since the 'database-url' property only references the Firebase Realtime Database.

@merlinnot
Copy link
Contributor

The only thing you need is project ID.

@stemuk
Copy link

stemuk commented Jun 25, 2018

Is there any leftover problem that is keeping this issue from getting closed and the branch from getting pushed to master? It has been 9 months now since Firestore has been first introduced and there is still no official Polymer solution from the Firebase team.

@merlinnot has made some great contributions and I am currently using them without any issues. It would be great to see this issue finally getting closed 👍

@merlinnot
Copy link
Contributor

No tests = doesn’t work ;)

@MarcosGalaviz
Copy link

Guys very nice work, im new, but I like Polymer and I realy want to use it with Firestore, but I cant understand very well how to use it with this mixin :( ¿could you help me with a practical example? Please.

Thanks a lot, you are doing an excelent job.

@merlinnot
Copy link
Contributor

If you’re just starting, don’t start with Polymer 2 and this library. Go with lit-html and RxJS or Redux + Redux Saga, it will make you a happier person :)

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

No branches or pull requests