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

feat(query): implement more query methods #173

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

FrederikBanke
Copy link

@FrederikBanke FrederikBanke commented Aug 4, 2023

Description

I have implemented 4 methods related to the Query class.

  • limit()
  • orderBy()
  • startAt()
  • startAfter()

They should behave more like the actual Firestore methods.

Do note, that I have not taken simulateQueryFilters into consideration, but it should be a quick fix, if the new functions should only run when simulateQueryFilters is set to true.

Limitations

In functions where values needs to be compared, only numbers, strings and timestamps are compared.

How to test

I have added tests in query.test.js, that test Query.limit(), Query.orderBy(), Query.startAt() and Query.startAfter().

@FrederikBanke FrederikBanke changed the title Implementing query methods feat(query): implement more query methods Aug 7, 2023
@sbatson5 sbatson5 self-requested a review August 8, 2023 19:05
@sbatson5
Copy link
Owner

sbatson5 commented Aug 8, 2023

I'm gonna set some time aside to review this soon. But as a note, I am planning on releasing a breaking version (1.0.0) where I think I'm going to make simulateQueryFilters true by default. So the param would be disabling. My only worry is that if we don't have perfect feature parody, people may end up thinking their tests are wrong even though it's just our mock that isn't querying exactly right (which isn't important for writing tests imo).

@FrederikBanke
Copy link
Author

I understand not merging, if it does not align with the purpose of the project. The reason I made the changes myself, was because I did need the queries to simulate for my own tests.

I just created the pull request in case it was of use to others.

I also understand that it can create my problems, if it doesn't work 1:1 with the real queries.

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 this pull request may close these issues.

2 participants