Adds support for pointer/string pointers comparison in LiveQuery #4231
Conversation
spec/QueryTools.spec.js
Outdated
| @@ -495,4 +495,79 @@ describe('matchesQuery', function() { | |||
| expect(matchesQuery(message, q)).toBe(false); | |||
|
|
|||
| }); | |||
|
|
|||
| function pointer(className, objectId) { | |||
| return {__type: 'Pointer', className, objectId }; | |||
There was a problem hiding this comment.
add whitespace between { __
src/LiveQuery/QueryTools.js
Outdated
| @@ -90,6 +90,28 @@ function queryHash(query) { | |||
| } | |||
|
|
|||
| /** | |||
| * contains -- Determines wether a constaint in for form of a list | |||
There was a problem hiding this comment.
"whether" and "constraint" are misspelled. I haven't read the code yet, but I really can't understand this comment.
src/LiveQuery/QueryTools.js
Outdated
| @@ -90,6 +90,28 @@ function queryHash(query) { | |||
| } | |||
|
|
|||
| /** | |||
| * contains -- Determines wether a constaint in for form of a list | |||
| * contains a particular object. | |||
| * This also supports, pointer comparion and strings as lightweight pointers | |||
There was a problem hiding this comment.
"comparison" is misspelled.
src/LiveQuery/QueryTools.js
Outdated
| if (object.__type && object.__type === 'Pointer') { | ||
| for (const i in compareTo) { | ||
| const ptr = compareTo[i]; | ||
| if (typeof ptr == 'string' && ptr === object.objectId) { |
src/LiveQuery/QueryTools.js
Outdated
| * contains a particular object. | ||
| * This also supports, pointer comparion and strings as lightweight pointers | ||
| */ | ||
| function contains(compareTo: Array, object: any): boolean { |
There was a problem hiding this comment.
The names of the parameters are confusing to me. particularly compareTo.
haystack, needle?
Codecov Report
@@ Coverage Diff @@
## master #4231 +/- ##
==========================================
- Coverage 92.24% 92.22% -0.02%
==========================================
Files 116 116
Lines 8098 8107 +9
==========================================
+ Hits 7470 7477 +7
- Misses 628 630 +2
Continue to review full report at Codecov.
|
|
Thanks for the quick review @acinader ! |
src/LiveQuery/QueryTools.js
Outdated
| @@ -90,6 +90,30 @@ function queryHash(query) { | |||
| } | |||
|
|
|||
| /** | |||
| * contains -- Determines whether a constraint in the form of a list | |||
There was a problem hiding this comment.
how about:
Determines if an object is contained in a list with special handling for Parse pointers.
|
seems that eslint don't catch issue when doing |
src/LiveQuery/QueryTools.js
Outdated
| @@ -99,8 +99,7 @@ function contains(haystack: Array, needle: any): boolean { | |||
| if (typeof ptr === 'string' && ptr === needle.objectId) { | |||
| return true; | |||
| } | |||
| if (typeof object !== 'undefined' && | |||
| ptr.className === needle.className && | |||
| if (ptr.className === needle.className && | |||
There was a problem hiding this comment.
that seems good, you've already validated that needle is not undefined in the conatining if.
There was a problem hiding this comment.
not really sure if we should be forcing
typeof needle !== 'undefined'
over
if(needle)
I use the second form in my code. Obviously could be an issue in the case of boolean but i just ignore that...
There was a problem hiding this comment.
Obviously could be an issue in the case of boolean but i just ignore that...
if it's a boolean, we'll go to the 'indexOf' case which is correct :)
There was a problem hiding this comment.
yup, i was musing out loud more generally about if we should enforce a more formal test than just if(foo) without advocating that we switch to it.... :). so safe to ignore me.
There was a problem hiding this comment.
all those bottom values in JS makes it difficult to make it consistent :/
No description provided.