Fix inner callback call for non comparable items #23
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -14,7 +14,7 @@ export type ComparisonResult = number | |||
| * Can be used to implement efficient algorithms which profit from sorted data | |||
| * like `mergeSort` or `mergeJoinWith`. | |||
| * | |||
| * Both collections must be sortable. They will be sorted ascendently using | |||
| * Both collections must be sortable. They will be sorted ascendantly using | |||
There was a problem hiding this comment.
why the change? both are good
There was a problem hiding this comment.
When I copied it to my comment in Github, Chrome's autocorrector complained about it 😆
| if (lhsValue < rhsValue) return SORTING_ORDER.LHS_BEFORE_RHS | ||
| if (lhsValue > rhsValue) return SORTING_ORDER.LHS_AFTER_RHS | ||
|
|
||
| throw new Error(`[@geoblink/lodash-mixins#mergeForEach] Found non-comparable values: ${lhsValue} on LHS and ${rhsValue} on RHS`) |
There was a problem hiding this comment.
This might have impact where we use it. We have to be careful about this
There was a problem hiding this comment.
If we find this error then we were relying on the old bug which was actually an undefined/unexpected behaviour, I think it's better to fail even without us expecting it rather than giving false matches (this was happening in My Data wizard before and was a crazy thing to debug)
furstenheim-geoblink
approved these changes
Jul 23, 2019
Sumolari
force-pushed
the
21-Fix-innerCallback-call-for-non-comparable-items
branch
from
d9b2662
to
0e775ab
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.