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

FirebaseObjectFactory query option not used #706

Closed
cartant opened this issue Dec 1, 2016 · 6 comments · Fixed by #772
Closed

FirebaseObjectFactory query option not used #706

cartant opened this issue Dec 1, 2016 · 6 comments · Fixed by #772

Comments

@cartant
Copy link
Contributor

cartant commented Dec 1, 2016

Version info

Angular: N/A

Firebase: N/A

AngularFire: 2.0.0-beta.6

Expected behavior

The documentation for FirebaseObjectObservable suggests that the query option can be used.

Actual behavior

The query-related imports and the query option are not used in the FirebaseObjectFactory implementation, so any specified query option is ineffectual.

It's not clear whether it's the source or the documentation that's out of date. Either the query option should be implemented (or should log a not-implemented warning, perhaps) or the documentation and source should be updated to reflect that the query option cannot be used with the FirebaseObjectObservable.

@katowulf
Copy link
Contributor

katowulf commented Dec 1, 2016

The "Querying" section of that doc explains why queries would not be useful for object observables. Perhaps summed up best by: Hence, for operations that require sorting, you are probably looking for a list.

@cartant
Copy link
Contributor Author

cartant commented Dec 1, 2016

@katowulf So, should the option be removed?

@cartant
Copy link
Contributor Author

cartant commented Dec 1, 2016

@katowulf And what, exactly, does this mean?

For filtering response data see and repeat 'Object' to yourself everytime you read the word 'List'.

@katowulf
Copy link
Contributor

katowulf commented Dec 1, 2016

Sounds like a word puzzle. Maybe there's an easter egg? : )

@cartant
Copy link
Contributor Author

cartant commented Dec 1, 2016

@katowulf Would you welcome a PR that removes the unused imports and option or logs a not-implemented/supported warning? If so, which one?

BTW, using the query option with an FirebaseObjectObservable is not something I'd ever needed nor attempted. I saw an SO question related to it and took a peak at the source.

@katowulf
Copy link
Contributor

katowulf commented Dec 1, 2016

An initial guess is that it's probably best to remove the unused imports and clean up or maybe remove the docs section as well. But I haven't looked at the code to verify they are superfluous or that they aren't intended to be added later (unlikely).

cartant added a commit to cartant/angularfire that referenced this issue Dec 12, 2016
The docs mention that lists should be used for querying/filtering and
the query option is ignored by the object factory. The query option
should be removed so that it is no longer suggested by TypeScript-based
tools, etc.

Closes angular#706
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

Successfully merging a pull request may close this issue.

2 participants