Add 'include' option to memorySource.find and memorySource.findLinked

[opsb]

Just opening this to allow discussion.

Adds the following option to find:

source.find('planet', 'planet1', {include: ["moons", "sun"]});

[opsb]

I'm wondering if we should be adding the 'include' option to findLinked. I notice that findLinked isn't listed in the methods for requestable in the orbit.js readme so I'm not sure if it's considered part of the public API or not(possibly the readme just hasn't been updated). If it is I think it would be more consistent to allow the 'include' option with findLinked. What's the general feeling?

[opsb]

Another issue, wondering about the various ways to call find when options are included, you've got:

find('type')
find('type', options) << collision
find('type', id)
find('type', id, options)
find('type', ids)
find('type', ids, options)
find('type', queryObject) << collision
find('type', queryObject, options)

It's difficult to distinguish between the queryObject and options (they're all objects). The simplest option from an implementation point of view is that options always has to be passed as the 3rd argument. That's pretty ugly though for find('type', null, options).

[dgeb]

The simplest option from an implementation point of view is that options always has to be passed as the 3rd argument. That's pretty ugly though for find('type', null, options).

One more argument for #110, in which options would always be the second argument.

For now though, I think options has to be the third argument.

[dgeb]

I'm wondering if we should be adding the 'include' option to findLinked.

Yes, definitely.

I'm not sure if it's considered part of the public API or not(possibly the readme just hasn't been updated).

Sorry - the readme does need updating.

[opsb]

One more argument for #110

Yup, that does make sense, so we'd have:

find('type')
find('type', options)
find(id)
find(id, options)
find(ids)
find(ids, options)
find(queryObject)
find(queryObject, options)

[opsb]

Actually, seeing as type and id are both strings, how would we distinguish between the two?

[dgeb]

Actually, seeing as type and id are both strings, how would we distinguish between the two?

find(id) // string
find(ids) // array
find({type: 'type'}) //object

[opsb]

Ha, that does seem pretty reasonable.

[opsb]

@dgeb This PR now adds support for the include option to both memorySource.find and memorySource.findLinked.

I've replaced the source.findLinked implementation, it seems to cover all the existing functionality, at least the tests pass anyway.

The options parsing is currently done in memory-source, in orbit-firebase I introduced an Options object which took care of this. It's a bit heavier though so perhaps you're happy with the way it is? (see https://github.com/orbitjs/orbit.js/pull/123/files#diff-3afbad5d9865c146aff683c3f945b68fR111)

[opsb]

Another thought, I'd expect findLinked to return an empty array rather than null.

[dgeb]

Merged via a3ca9aa and 55f111e

Wanted to get this in before beginning the major rebase refactor discussed in #110.

[opsb]

Thanks @dgeb! We' be been running an internal beta against orbit.ja and orbit-firebase for a couple of weeks now without bugs. Looking forward to the refactor in #110 as we've already get a case where we need polymorphism.

[dgeb]

That's great news, @opsb!