Skip to content

Commit

Permalink
[ProjectController] fix the sorting of project entities for node v11+
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
das7pad committed Nov 5, 2019
1 parent 33dc94d commit 85dc1bf
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion app/src/Features/Project/ProjectController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down

0 comments on commit 85dc1bf

Please sign in to comment.