fireorm
fireorm copied to clipboard
wovalle
QuerySnapshot Events – Proof of Concept
Hi @wovalle, I built a quick proof-of-concept to implement listening on snapshot events: https://github.com/garviand/fireorm/pull/1
It is very rough but I was wondering if you think it is worth pursuing this direction as a contribution. Would love to hear your suggestions on how to proceed if you think it is worthwhile. Thanks!
It looks good and simple!
There are some things that I want to point out:
- Unit tests
- Some typescript nuances (
anys in the integration tests) What kind of information can be extracted from the QuerySnapshot? I'm asking because I'd prefer abstract the firestore object (good work here!)
I'd like to do some tests on my own to see if the api seems coherent with the rest of the project but I do think this feature is a step in the right direction.
Awesome! Yes, I cheated with the types so I didn't spend too much time 😂
I'll try to implement your suggestions and send it back over.
Hey @wovalle, I removed all of the anys and added types just to get a better sense of what can be abstracted. I am a little lost in terms of how I can do these abstractions in a way that fits with the rest of the project. Do you have any suggestions? It seems that the two types which are not in line with the project so far and need abstraction are:
QuerySnapshot
() => void (which I added to several spots, it seems messy)
I'll go ahead and start adding unit tests, but if you want to see the updates I made: https://github.com/garviand/fireorm/pull/1/files
I'll check this out on the weekend! I'm having a busy week.
About the QuerySnapshot, given than the list of documents must be parsed with extractTFromColSnap, I don't think is a good idea to return the QuerySnapshot to the user but a custom object with the parsed entities and any other important info that can be extracted from the snapshot.
Overall it looks good! I'll try to dedicate it some time on the weekend (maybe writing something to test it, I don't know).
Keep it going! Great work! Next release will be exciting!
Ok, thank you! No need to rush on this, I am busy as well. I'll do my best to keep improving it and I'll let you know what I end up doing so we don't work on the same things. This is such a fantastic library!
@wovalle I changed all the QuerySnapshot types to generics (T) which I think is more in line with the rest of the project. When you have a minute let me know what you think. If it is good, I'm happy to start writing unit tests. Thanks!
Hey @wovalle I couldn't tag you in my PR for some reason but I updated some things here: https://github.com/garviand/fireorm/pull/2/files
Let me know your thoughts and any next steps you can think of. Thanks!
This looks awesome! Can you create a PR to my repo and let's continue the discussion there?
I'm quite busy these days because I'm starting my holidays vacations next week, after that I'll way more time to dedicate to this project. My prio is to finally merge #172, then pick up this one and finally dedicate time to #90.
Thanks so much for all your contributions! Next week I'll have more time to finally check/merge this 🙏