This repository has been archived by the owner on Aug 6, 2021. It is now read-only.
[ProjectController] fix the sorting of project entities for node v11+ #697
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.
Description
TL;DR: The array sorting algorithm has changed in node v11, which is breaking a single unit test.
The patch is backwards compatible, see below.
Background and Demo
For the signature
Array.prototype.sort((a, b) => rt)
rt
encodes the sorting positions as follows [1]:rt < 0
: a should go firstrt = 0
: same positionrt > 0
: b should go firstNode version 11 bundles an updated v8 engine [2]. One of the changes is
a different sorting algorithm for Array.prototype.sort.
Quicksort was replaced by Timsort[3], a stable sorting algorithm.
Node v12 (new LTS) has the same updated behaviour.
For an arbitrary sorted array every comparision would yield 0 or 1.
Our comparision function using
rt = a > b
, is not sufficient anymore,as it would yield the signature of a sorted array and no changes are
made to the sequence accordingly.
The fix is simple: adjust the return value of 0 to -1.
There should be only one entity per unique path, so we can omit the rt=0
case.
Here is a minimal demo of the unit test
test/unit/src/Project/ProjectControllerTests.js
"ProjectController"
-> "projectEntitiesJson"
-> "should produce a list of entities"
For future reference I kept the nvm output referencing the used node
version (Running node ...) in place.
Using the current sorting function
Using the adjusted return value
[1] https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#Description
[2] nodejs/node#22754
[3] https://chromium-review.googlesource.com/c/v8/v8/+/1186801
Screenshots
Related Issues / PRs
Review
Potential Impact
Likely none, no acceptance tests are impacted.
Manual Testing Performed
Accessibility
Deployment
Deployment Checklist
Metrics and Monitoring
Who Needs to Know?