Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected behavior using orderBy #361

Open
HZ-labs opened this issue Aug 8, 2024 · 6 comments · Fixed by #365
Open

Unexpected behavior using orderBy #361

HZ-labs opened this issue Aug 8, 2024 · 6 comments · Fixed by #365

Comments

@HZ-labs
Copy link

HZ-labs commented Aug 8, 2024

Something wrong if the last element has another type

Using orderBy(data, ["value"], ["asc"])

const data = [
  { id: 1, value: "apple" },
  { id: 2, value: "banana" },
  { id: 3, value: "cherry" },
  { id: 4, value: 10 },
  { id: 5, value: 5 },
  { id: 7, value: 20 },
  { id: 6, value: "banana" },
];

// Result:
[
  { id: 1, value: "apple" },
  { id: 2, value: "banana" },
  { id: 3, value: "cherry" },
  { id: 5, value: 5 },
  { id: 4, value: 10 },
  { id: 7, value: 20 },
  { id: 6, value: "banana" },
]

But if we change a bit data, its OK:

const data = [
  { id: 1, value: "apple" },
  { id: 2, value: "banana" },
  { id: 3, value: "cherry" },
  { id: 4, value: 10 },
  { id: 5, value: 5 },
  { id: 6, value: "banana" },
  { id: 7, value: 20 },
];

// Result:
[
  { id: 1, value: "apple" },
  { id: 2, value: "banana" },
  { id: 6, value: "banana" },
  { id: 3, value: "cherry" },
  { id: 5, value: 5 },
  { id: 4, value: 10 },
  { id: 7, value: 20 },
]

Example https://codesandbox.io/p/sandbox/es-toolkit-orderby-22zkzm

@raon0211
Copy link
Collaborator

raon0211 commented Aug 9, 2024

Thanks! We will check this.

@dayongkr
Copy link
Contributor

dayongkr commented Aug 9, 2024

Actually, lodash also have same this issue. But we have to fix this issue for expected behavior.

This problem can be fixed by handling type coercion.

I'll fix this issue today.

@raon0211
Copy link
Collaborator

This actually needs a little bit of more discussion, reopening the issue.

@raon0211
Copy link
Collaborator

I guess in this case, the ideal way of using orderBy would be:

orderBy(data, x => x.value.toString(), ["asc"])

This would ensure that all numbers and strings are converted into string before comparison.

@dayongkr
Copy link
Contributor

dayongkr commented Aug 10, 2024

I guess in this case, the ideal way of using orderBy would be:

orderBy(data, x => x.value.toString(), ["asc"])

This would ensure that all numbers and strings are converted into string before comparison.

We have additional ways that we convert into string just using map before using orderBy and use custom order function.

But I think custom key function is simpler than others.

@raon0211
Copy link
Collaborator

As you mentioned, I guess we might try with custom key generating functions. It would be much simple.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants