Matching queries with doesNotExist constraint #1250
Conversation
Current coverage is
|
| @@ -206,7 +206,9 @@ function matchesKeyConstraints(object, key, constraints) { | |||
| } | |||
| break; | |||
| case '$exists': | |||
| if (typeof object[key] === 'undefined') { | |||
| let propertyExists = typeof object[key] !== 'undefined'; | |||
| let existenceIsRequired = constraints['$exists']; | |||
There was a problem hiding this comment.
is that true / false or a value or can be undefined/null??
There was a problem hiding this comment.
From my tests and understanding it will always be a boolean, because this query representation is done by the SDKs. It could be something different when the query is done via REST but this is not the case for LiveQuery right?
Anyway the logic would still work if constraints['$exists'] is null/undefined.
There was a problem hiding this comment.
The question is more, do we want it to work when it's null / undefined?
There was a problem hiding this comment.
Hum.. I see. Should I enclose it under an if (typeof constraints['$exists'] === "boolean")?
There was a problem hiding this comment.
i'm not sure :) that's the problem :P cc: @wangmengyan95
There was a problem hiding this comment.
I think we want true/false only.
There was a problem hiding this comment.
@andrecardoso if you can address that concern we'll be able to merge that :)
|
@andrecardoso just a little question but otherwise looks good |
|
@andrecardoso updated the pull request. |
facebook-github-bot
added
GH Review: review-needed
and removed
GH Review: needs-revision
labels
|
I'm going to merge this and submit a separate PR that adds the |
|
Thanks guys! |
I was trying LiveQuery using a query with a doesNotExist constraint and after some time debugging I've found out that QueryTools wasn't matching it correctly.
This pull request resolves this issue.
Feedback is appreciated.
Thanks.