1

Ok, so we had some code that was working fine and passing tests fine on node 10, now following upgrading to node 11 the code now fails unit tests. Code maps over array of objects altering properties then sorts based on a string name value i.e. array.sort(a, b) => a.toLowerCase() > b.toLowerCase().

It now maps correctly but the sort does not work and returns the mapped array only without sorting, when i've tried splitting the two functions out into individual map then sort the sort returns undefined.

Have researched and tried to find some examples to see what needs to be changed but not found a great deal other than suggestions of the sort algorithm changing to timsort in v8.

simple code

export default places => places
   .map(place => ({
    value: place.properties.code, label: place.properties.name
   }))
   .sort((placeA, placeB) => placeA.label.toLowerCase() > 
   placeB.label.toLowerCase())

test array:

      type: 'Place',
      properties: {
        code: 'CA076757',
        name: 'Brockway'
      }
    }, {
      type: 'Place',
      properties: {
        code: 'MN486464',
        name: 'Ogdenville'
      }
    }, {
      type: 'Place',
      properties: {
        code: 'S4889785',
        name: 'North Haverbrook'
      }
    }]

expected result

      {value: 'CA076757', label: 'Brockway'},
      {value: 'S4889785', label: 'North Haverbrook'},
      {value: 'MN486464', label: 'Ogdenville'}
    ]

actual result

      {value: 'CA076757', label: 'Brockway'},
      {value: 'MN486464', label: 'Ogdenville'}.
      {value: 'S4889785', label: 'North Haverbrook'}
    ]
SBWork
  • 23
  • 5
  • using `>` as the comparator imples the values are equal when `a < b`, so the implementation is allowed the freedom to swap them, or not, as the specification does not guarantee a stable sort algorithm. – Patrick Roberts Jun 27 '19 at 18:13
  • Thanks to all for your help and explanations as to the behaviour and why things are happening, all incredibly useful :) many many thanks – SBWork Jun 28 '19 at 13:43

4 Answers4

4

we had some code that was working fine and passing tests fine on node 10, now following upgrading to node 11 the code now fails unit tests

To be blunt, that means that your tests are not providing sufficient coverage ;-)

In JavaScript, a comparator function cmp(a, b) for Array.sort should return:

  • a value less than zero if a is less than b
  • zero if a is equal to b
  • a value greater than zero if a is greater than b

If you use a comparator function that returns a boolean value, then false will silently map to 0, and true will silently map to 1. There is no way to signal the a < b case. If your test cases get (or used to get) sorted correctly anyway, then they didn't cover that case.

A suitable comparator function for your example, regardless of which Node version or which browser you're using, would be:

(placeA, placeB) => {
  let a = placeA.label.toLowerCase();
  let b = placeB.label.toLowerCase();
  if (a < b) return -1;
  if (a > b) return 1;
  return 0;
}
jmrk
  • 34,271
  • 7
  • 59
  • 74
0

You can use localeCompare for that:

export default places => places
    .map(place => ({
        value: place.properties.code, label: place.properties.name
    }))
    .sort((placeA, placeB) => placeA.label.toLowerCase().localeCompare(placeB.label.toLowerCase()));

Output:

[ { value: 'CA076757', label: 'Brockway' },
  { value: 'S4889785', label: 'North Haverbrook' },
  { value: 'MN486464', label: 'Ogdenville' } ]
Aritra Chakraborty
  • 12,123
  • 3
  • 26
  • 35
  • 1
    This is sound advice, but keep in mind that `localeCompare` has pros and cons: on the pro side, it takes the current locale into account, which can be very much desirable (or, when running in browsers, can lead to different behavior for different users, which might be weird). On the con side, it tends to be slower than more primitive, non-locale-aware alternatives, which may or may not matter for a given use case. – jmrk Jun 27 '19 at 18:24
0

Based on my answer to Sort array of objects by string property value, a sufficient way to sort strings when string locales are not important, is the following approach:

const sortBy = fn => (a, b) => {
  const fa = fn(a)
  const fb = fn(b)
  return -(fa < fb) || +(fa > fb)
}

const sortByLabelCaseInsensitive = sortBy(
  place => place.label.toLowerCase()
)

const fn = places => places.map(place => ({
  value: place.properties.code,
  label: place.properties.name
})).sort(sortByLabelCaseInsensitive)

const array = [{
  type: 'Place',
  properties: {
    code: 'CA076757',
    name: 'Brockway'
  }
}, {
  type: 'Place',
  properties: {
    code: 'MN486464',
    name: 'Ogdenville'
  }
}, {
  type: 'Place',
  properties: {
    code: 'S4889785',
    name: 'North Haverbrook'
  }
}]

console.log(fn(array))
Patrick Roberts
  • 49,224
  • 10
  • 102
  • 153
0

try this one

.sort((placeA, placeB) => { 
   if(placeA.label.toLowerCase() < placeB.label.toLowerCase()) return -1
   if(placeA.label.toLowerCase() > placeB.label.toLowerCase()) return 1
   return 0;
});

You need to compare each element with the next one and return that it is equal, greater or less