Fix startAfter/endBefore for orderByKeys queries #4363
Conversation
🦋 Changeset detectedLatest commit: ca75e92 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -101,21 +101,15 @@ export class Query { | |||
| 'or equalTo() must be a string.'; | |||
| if (params.hasStart()) { | |||
| const startName = params.getIndexStartName(); | |||
| if ( | |||
There was a problem hiding this comment.
These just revert a previous change.
| childKey = MAX_NAME; | ||
| let params: QueryParams; | ||
| if (this.index_ === KEY_INDEX) { | ||
| if (typeof indexValue === 'string') { |
There was a problem hiding this comment.
This check makes it so that startAfter still throws exceptions for incorrectly typed keys.
There was a problem hiding this comment.
Where does it throw? Should we throw at the callsite to preserve the stacktrace of the original mistake?
There was a problem hiding this comment.
It throws in validateQueryEndpoints_: https://github.com/firebase/firebase-js-sdk/blob/master/packages/database/src/api/Query.ts#L110 (this behavior is tested), which is the same place that startAt() and endAt() throw for this case.
I think that the way it's done now the original mistake, passing a non-string key to startAfter() should still be clear. I'll also update the error message there.
Size Analysis Report |
We should not be passing a fallback key to startAfter if we're querying on a key index. The indexValue is the key in this case.