yobacca / natural-orderby Goto Github PK

I had an issue where "2019-12-30" and "12/30/2019" were not comparing as equal. While I recognize this is really an issue with JavaScript, it might make sense for a sort of work-around to be available in the library.

From Firefox:

When I try to install this package using yarn via yarn add, on a system running Node v6.x, the installation fails.

In this module's package.json file, there is an entry for the node engine:

  "engines": {
    "node": "^8.10.0 || >=9.10.0"
  },

Most npm packages that don't have direct dependencies on any node features and they usually don't specify a node version requirement, and use this:

  "engines": {
    "node": "*"
  },

We are using yarn for our node package manager, and it appears that yarn pays attention to this engine property, and compares it to the active node version. So, this means that if I'm running Node v6.x, I can't install natural-orderby, because it specifies Node v >8.10 -- even though it doesn't require node.

This should be changed in the npm package's package.json, to:

  "engines": {
    "node": "*"
  },

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Datasource	Name	Replacement PR?
npm	babel-eslint
npm	eslint-plugin-node

Rate-Limited

These updates are currently rate-limited. Click on a checkbox below to force their creation now.

• chore(deps): replace dependency eslint-plugin-node with eslint-plugin-n ^14.0.0
• chore(deps): update dependency @rollup/plugin-node-resolve to v15.2.3
• chore(deps): update dependency eslint to v8.57.0
• chore(deps): update dependency eslint-config-prettier to v8.10.0
• chore(deps): update dependency eslint-plugin-import to v2.29.1
• chore(deps): update dependency eslint-plugin-jest to v27.9.0
• chore(deps): update dependency lint-staged to v13.3.0
• chore(deps): update dependency prettier to v2.8.8
• chore(deps): update dependency rollup-plugin-dts to v5.3.1
• chore(deps): update dependency typescript to v4.9.5
• chore(deps): update typescript-eslint monorepo to v5.62.0 (@typescript-eslint/eslint-plugin, @typescript-eslint/parser)
• chore(deps): update actions/checkout action to v4
• chore(deps): update actions/setup-node action to v4
• chore(deps): update commitlint monorepo to v19 (major) (@commitlint/cli, @commitlint/config-conventional, @commitlint/types)
• chore(deps): update dependency eslint to v9
• chore(deps): update dependency eslint-config-prettier to v9
• chore(deps): update dependency eslint-plugin-jest to v28
• chore(deps): update dependency eslint-plugin-prettier to v5
• chore(deps): update dependency husky to v9
• chore(deps): update dependency lint-staged to v15
• chore(deps): update dependency prettier to v3
• chore(deps): update dependency rollup to v4
• chore(deps): update dependency rollup-plugin-dts to v6
• chore(deps): update dependency typescript to v5
• chore(deps): update typescript-eslint monorepo to v8 (major) (@typescript-eslint/eslint-plugin, @typescript-eslint/parser)
• 🔐 Create all rate-limited PRs at once 🔐

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

• chore(deps): replace dependency babel-eslint with @babel/eslint-parser ^7.11.0
• chore(deps): update dependency @babel/preset-modules to v0.1.6
• chore(deps): update dependency @rollup/plugin-babel to v6.0.4
• chore(deps): update dependency husky to v8.0.3
• chore(deps): update babel monorepo (@babel/core, @babel/preset-env, @babel/preset-typescript)
• chore(deps): update commitlint monorepo to v17.8.1 (@commitlint/cli, @commitlint/config-conventional, @commitlint/types)
• chore(deps): update dependency @changesets/cli to v2.27.7
• chore(deps): update dependency @rollup/plugin-terser to ^0.4.0
• chore(deps): update dependency ts-jest to v29.2.4
• chore(deps): update jest monorepo (@types/jest, babel-jest, jest)
• Click on this checkbox to rebase all open PRs at once

Detected dependencies

• Check this box to trigger a request for Renovate to run again on this repository

I haven't dove into it too much, but in a specific case I noticed it doesn't appear to be sorting as intended.

var naturalOrderby = require("natural-orderby")

var items = ['A110', 'A100', 'A090', 'A200', 'A50'];
naturalOrderby.orderBy(items);

Result

["A50", "A100", "A110", "A200", "A090"]

https://runkit.com/embed/fxldnunwae3g

It's putting 090 at the end. If you remove the A it works as expected.

Library returns the following sort: ["00A", "00aa", "000A"]. However, a similar library for server side (Python) returns ["00A", "000A", "00aa"]. Which one is right? What is used to decide the difference?

I was using natural-orderby to try compare "2-020-00" and "3-000-00", and found that Firefox produced the expected ordering, while Chrome did not.

compare()("2-020-00", "3-000-00"); // 1 in Chrome, -1 in Firefox and Safari

I believe this is because Date.parse('2-020-00'); yields 951026400000 in Chrome, but NaN in Firefox and Safari.

I realize this is a browser difference that would be rather hard to fix. MDN even recommends against Date.parse for presumably that reason. But in case you had any ideas, I thought I'd post it here. :-D

The order is incorrect when the string contains diacritics

> orderBy(['b', 'd', 'f', 'A', 'C', 'E'])
[ 'A', 'C', 'E', 'b', 'd', 'f' ]
// correct

> orderBy(['b', 'd', 'f', 'A', 'Cé', 'E'])
[ 'A', 'b', 'Cé', 'E', 'd', 'f' ]
// incorrect

Tested environment:

• Chromium 92/Firefox 90 on Ubuntu, Chrome on Mac (unsure of the versions)

Description

• When sorting an array of objects (of any length), execution times when calling orderBy increase dramatically as the length of the string value being sorted increases.(Times will obviously vary from machine to machine, but I imagine the curve will look mostly the same anywhere)
  • Given a length of 20 characters, I see times around 0.2ms
  • 100 chars gives ~13ms
  • 200 chars gives ~210ms
  • 300 chars gives ~1000ms

Repro

• I ran the code below in Chromium/Firefox, as well as RunJS to narrow down the issue, and the results were pretty consistent across environments
• The value of label in this case is 200 characters in length, and can be changed to see different execution times

import { orderBy } from 'natural-orderby';

const inputArray = [
  {
        "label": "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuv",
        "value": "foo"
    }
];

const key = 'label';

const time0 = performance.now();

orderBy(inputArray, key);

const time1 = performance.now();

console.log(`call to orderBy took ${time1 - time0}ms`);

Output: 'call to orderBy took 204.47000000058324ms'

Other notes

• Breaking up the string with hyphens or white space offsets the increase in execution time
• The number of elements in the array has no significant impact on execution times, unless those items also have similar long unbroken string values

I'm not sure that this is a design goal, but I see that dollar values (and presumably other currencies) are not parsed to be identical to plain numbers.

const {compare} = require("natural-orderby")

console.log("strings and numbers", compare()("1.00", 1)); // 0
console.log("dollar-strings and numbers", compare()("$1.00", 1)); // 1

(See https://runkit.com/dahjelle/5dd552e003c3750014962150 for a runnable version of the above.)

I haven't dived in deep to see how the algorithm in this library works, but does it seem like this is a possible and desired feature?

Given:

• "2024-01-01"
• "2024-05-01"
• "2024-01-05"
• "2024-01-05 Foo"

Expected order:

• "2024-01-01"
• "2024-01-05"
• "2024-01-05 Foo"
• "2024-05-01"

Output order of the package:

• "2024-01-05 Foo"
• "2024-01-01"
• "2024-01-05"
• "2024-05-01"

This happens because they are parsed as dates, except for the one with the 'Foo' suffix.

var naturalOrderby = require("natural-orderby")

naturalOrderby.orderBy(["05", "06", "07*", "08A", "09 B", "10", "11", "12A", "13"])

Expected result: no change. Actual result:

["05", "06", "07*", "10", "11", "12A", "13", "08A", "09 B"]

See https://runkit.com/embed/b88ooo12p3ap

I think I have the specifics pinned down in the title, but I'm not sure if that's quite it. I know repeating the test with numbers ranging from 15 to 23 behaves as expected, and I can't find any non-alphabetic characters that prompt the same error.

Would it be possible to implement a parameter to pick the first N elements of the sort results, without the need to sort the entire array? Similar to a heap sort.

Do you have any plans to also support Typescript by adding a declaration file?

For now I'm using this, to have it work in Typescript as well:

export function compare(options: any): any;
export function orderBy(collection: any, identifiers: any, orders: any): any;

export namespace compare {
    const prototype: {
    };
}

export namespace orderBy {
    const prototype: {
    };
}

But it is an own inferred declaration, so 'any' types all over.

I noticed that in the baseOrderBy (hence affecting the top-level orderBy) that the standard Array.prototype.sort() is being used. On chrome (before version 70), this is not a stable sort algorithm.

Recommend that standard array sort is replaced with https://www.npmjs.com/package/stable or ensure an algorithm such as mergesort is used.

With these strings:

const paths = [ 
  'IMG_0001.JPG',
  'IMG_0002.JPG',
  'IMG_0010.JPG',
  'IMG_0011.JPG',
  'IMG_0020.JPG',
  'IMG_0021.JPG' 
]

With node-natural-sort (expected/correct behavior):

import naturalSort from "node-natural-sort";
paths.sort(naturalSort());

//  'IMG_0001.JPG',
//  'IMG_0002.JPG',
//  'IMG_0010.JPG',
//  'IMG_0011.JPG',
//  'IMG_0020.JPG',
//  'IMG_0021.JPG'

With natural-orderby (unexpected/incorrect behavior):

import { orderBy } from "natural-orderby";

orderBy(paths)

//  'IMG_0010.JPG',
//  'IMG_0020.JPG',
//  'IMG_0001.JPG',
//  'IMG_0002.JPG',
//  'IMG_0011.JPG',
//  'IMG_0021.JPG' 

Thanks for the great library!