Allow numbers and objects in filters

[bfirsh]

For example: filter={["thing", "==", db.doc("things/" + thing)]}

Fixes #16

[codecov]

Codecov Report

Merging #19 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master    #19   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           6      6           
  Lines          78     78           
  Branches       18     18           
=====================================
  Hits           78     78

Impacted Files	Coverage Δ
src/FirestoreCollection.js	100% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b759d03...3192249. Read the comment docs.

[lrholmes]

Thanks for tackling the issue @bfirsh! @green-arrow any chance of a merge sometime soon?

[green-arrow]

@bfirsh - First off, thanks for working on this! In looking at the docs, it looks like the third item in the array is the only one that is allowed to be a number or object. The first two are always strings, one for the field name and one for the comparison type. I'm wondering if we can write a custom prop type function that would check for an array with exactly 3 items, the first two being strings and the last being a string, number, or object.

It would also then be possible to write some tests for that function that verify an error is thrown if the props don't match the custom shape.

This is something like what I was thinking:

function isValidFirestoreQuery(props, propName) {
  // Validate that props[propName] is a valid query with 3 elements and proper types
}

Then the PropType definition would become:

filter: PropTypes.oneOfType([isValidFirestoreQuery, PropsTypes.arrayOf(isValidFirestoreQuery)])

Reference: https://reactjs.org/docs/typechecking-with-proptypes.html#react.proptypes

Relevant example from reference:

// You can also specify a custom validator. It should return an Error
  // object if the validation fails. Don't `console.warn` or throw, as this
  // won't work inside `oneOfType`.
  customProp: function(props, propName, componentName) {
    if (!/matchme/.test(props[propName])) {
      return new Error(
        'Invalid prop `' + propName + '` supplied to' +
        ' `' + componentName + '`. Validation failed.'
      );
    }
  },

  // You can also supply a custom validator to `arrayOf` and `objectOf`.
  // It should return an Error object if the validation fails. The validator
  // will be called for each key in the array or object. The first two
  // arguments of the validator are the array or object itself, and the
  // current item's key.
  customArrayProp: PropTypes.arrayOf(function(propValue, key, componentName, location, propFullName) {
    if (!/matchme/.test(propValue[key])) {
      return new Error(
        'Invalid prop `' + propFullName + '` supplied to' +
        ' `' + componentName + '`. Validation failed.'
      );
    }
  })

Please let me know if that made sense or if something about it doesn't seem right!

[bfirsh]

That makes sense. It sounds rather overkill though - will this actually catch errors that a user might make?

I haven’t got time to do such an extended fix right now. Feel free to merge this now as a quick fix, if you’d like, and I could tackle it at some point in the future. :)

[bfirsh]

Come to think of it, I'll add documentation first. That'll be much more useful than a really comprehensive propType. There isn't even documentation about filtering by references in the official docs.

[green-arrow]

@bfirsh - sounds like a plan! Let me know when you add documentation and we'll get this merged.

[bfirsh]

Okay done a couple of things:

• Added a simple example in the docs
• Changed the test to use an actual document ref instead of just an object. This will cover the more complex propType that you describe.

The precommit hook seemed to reformat the readme, so I split that out into a separate commit to make my change clear.

[green-arrow]

@bfirsh - thanks again for your work on this!