search

IjzerenHein / firestorter

Use Google Firestore in React with zero effort, using MobX 🤘
http://firestorter.com
MIT License
378 stars 50 forks source link

Collection.hasDocs #68

Closed damonmaria closed 5 years ago

damonmaria commented 5 years ago

Done. Fought a bit with the tests but got it working.

Note: I've turned on experimentalDecorators in tsconfig.json to use @computed on hasDocs. I presume this is OK?

A few tangental things:

  1. Any tests involving the "todos" collection are failing because of insufficient permissions (with the included firebaseConfig.json). For example, Collection.add.test.js are expecting an error to be thrown but the actual error is the permissions error, not what it's expecting. Can the security rules be fixed for that project?
  2. The above could be simpler and the tests speed up (locally, not sure about on TravisCI) if they were switched to using the Firestore emulator.
  3. I originally was adding to the "artists" collection to test mobx didn't 'react' to the docs.length changing (while hasDocs remained true) so made seedSampleData.js cleanup any new docs. Didn't need this in the end but have left it there.
  4. A few minor fixes for hints generated by IDEA.
damonmaria commented 5 years ago

Had to switch from using @computed to mobx's decorate due to jsdoc2md not handling decorators. Meh.

damonmaria commented 5 years ago

@IjzerenHein Hey, just checking in on this. Are there any issues with this PR?

IjzerenHein commented 5 years ago

@damonmaria Hey. No not at all, just been pre-occupied with other things. Will check this out shortly, many thanks for the PR!

IjzerenHein commented 5 years ago

Looks really good @damonmaria 👍 . I've merged and release a new version: https://github.com/IjzerenHein/firestorter/releases/tag/v1.2.2

damonmaria commented 5 years ago

@IjzerenHein If you're happy with the use of decorate to make properties computed then I think it would be best if we went further and applied it to other properties. For example, Document.path would be a good candidate as it does a lot of calculation. I'm sure there are others, and it shouldn't be a breaking change.

IjzerenHein commented 5 years ago

@damonmaria I guess that could be possible. Do you consider path to be a property that is worthwhile of optmization? What is a practical use-case that would benefit from this optimisation?

damonmaria commented 5 years ago

I guess it's not much computation, it would avoid building the path string on repeated uses of accessing the path property. Same would apply to Collection.