[ProjectController] fix the sorting of project entities for node v11+

das7pad · das7pad · commit 633c7a625020 · 2019-11-05T14:08:22.000+01:00
For the signature `Array.prototype.sort((a, b) => rt)` `rt` encodes the sorting positions as follows [1]: - `rt < 0`: a should go first - `rt = 0`: same position - `rt > 0`: b should go first Node 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 ``` $ nvm run v10 -e 'console.log( [{ path: "/things/b.txt", type: "doc" }, { path: "/main.tex", type: "doc" }, { path: "/things/a.txt", type: "file" } ].sort((a, b) => { const rt = a.path > b.path console.log("comparing", a.path, b.path, rt) return rt}))' Running node v10.17.0 (npm v6.12.1) comparing /things/b.txt /main.tex true comparing /things/b.txt /things/a.txt true comparing /main.tex /things/a.txt false [ { path: '/main.tex', type: 'doc' }, { path: '/things/a.txt', type: 'file' }, { path: '/things/b.txt', type: 'doc' } ] $ nvm run v11 -e 'console.log( [{ path: "/things/b.txt", type: "doc" }, { path: "/main.tex", type: "doc" }, { path: "/things/a.txt", type: "file" } ].sort((a, b) => { const rt = a.path > b.path console.log("comparing", a.path, b.path, rt) return rt}))' Running node v11.15.0 (npm v6.12.1) comparing /main.tex /things/b.txt false comparing /things/a.txt /main.tex true [ { path: '/things/b.txt', type: 'doc' }, { path: '/main.tex', type: 'doc' }, { path: '/things/a.txt', type: 'file' } ] ``` Using the adjusted return value ``` $ nvm run v10 -e 'console.log( [{ path: "/things/b.txt", type: "doc" }, { path: "/main.tex", type: "doc" }, { path: "/things/a.txt", type: "file" } ].sort((a, b) => { const rt = a.path > b.path ? 1 : -1 console.log("comparing", a.path, b.path, rt) return rt}))' Running node v10.17.0 (npm v6.12.1) comparing /things/b.txt /main.tex 1 comparing /things/b.txt /things/a.txt 1 comparing /main.tex /things/a.txt -1 [ { path: '/main.tex', type: 'doc' }, { path: '/things/a.txt', type: 'file' }, { path: '/things/b.txt', type: 'doc' } ] $ nvm run v11 -e 'console.log( [{ path: "/things/b.txt", type: "doc" }, { path: "/main.tex", type: "doc" }, { path: "/things/a.txt", type: "file" } ].sort((a, b) => { const rt = a.path > b.path ? 1 : -1 console.log("comparing", a.path, b.path, rt) return rt}))' Running node v11.15.0 (npm v6.12.1) comparing /main.tex /things/b.txt -1 comparing /things/a.txt /main.tex 1 comparing /things/a.txt /things/b.txt -1 comparing /things/a.txt /main.tex 1 [ { path: '/main.tex', type: 'doc' }, { path: '/things/a.txt', type: 'file' }, { path: '/things/b.txt', type: 'doc' } ] ``` --- [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 Signed-off-by: Jakob Ackermann <das7pad@outlook.com>
diff --git a/app/src/Features/Project/ProjectController.js b/app/src/Features/Project/ProjectController.js
@@ -349,7 +349,8 @@ const ProjectController = {
           }
           const entities = docs
             .concat(files)
-            .sort((a, b) => a.path > b.path) // Sort by path ascending
+            // Sort by path ascending
+            .sort((a, b) => (a.path > b.path ? 1 : -1))
             .map(e => ({
               path: e.path,
               type: e.doc != null ? 'doc' : 'file'
