Fix fixer for jsx-sort-comp in case of more than 10 props #2012
Conversation
Currently tests contains reduced cases. Do you propose create test cases with exatly same code from issues? |
|
Yes, auto fixing is pretty subtle, and it allows for complete confidence that it avoids the regression. “Too many tests” is a good problem to have :-) |
|
Fix commit message |
| @@ -107,7 +107,7 @@ const generateFixerFunction = (node, context, reservedList) => { | |||
| }); | |||
| }); | |||
|
|
|||
| fixers.sort((a, b) => a.range[0] < b.range[0]); | |||
| fixers.sort((a, b) => b.range[0] - a.range[0]); | |||
There was a problem hiding this comment.
Woah great fix here. @tihonove could you explain how this fix works?
There was a problem hiding this comment.
Comparators are supposed to only return a negative, positive, or zero number - previously it returned a boolean here.
There was a problem hiding this comment.
Ah thanks, I wasn't really aware of that.
But then I just couldn't piece together what this had to do with the original issue of 10 or more props. Why 10? Is it exactly 10, or somewhere around that number?
For posterity - I tested and noticed that booleans do work on arrays of 10 items or less, and immediately break on longer arrays. Something clicked and I recalled that V8 uses a different algorithm on small arrays; turned out that according to the source 10 is the threshold for that.
Which I guess makes sense because their quicksort implementation uses all 3 possible comparison results, whereas insertion sort only requires 2, so a boolean value is enough.
ghost
mentioned this pull request
Just a little bug. Fixes #1922. Fixes #1993