angular: Pipe "keyvalue" should not sort by default.
Issue / Improvement / Feature Request
Relevant Package
@angular/common
(source)
Description
Currently the keyvalue
pipe sorts the keys. This result in e.g.
1, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 2, 20, 21, ...
Because it’s not natual sorted. …
Describe the solution you’d like
It should not sort by default. Respect the original object state.
It’s annoying to declare a no sort method to each component which uses keyvalue. (function that returns 0
for no sort.)
Describe alternatives you’ve considered
Provide predefined sort functions. The parameter of keyvalue
could be a string or function. The string is the name of the predefined buildin function of Angular. Like:
keyvalue: 'natural'
keyvalue: 'sort' // the current sort behavior
keyvalue: mySort // a custom function
keyvalue // default no parameter, no sort.
Or we could invert the logic so that no parameter means the current behvaior but keyvalue: 'none'
disables the sort. For downwards compatibility. If you care about. But in my opinion we should ignore that. A tiny breaking change. The migration is simple: Just add 'sort'
to the keyvalue pipe. So I still prefer my solution: Don’t sort by default.
About this issue
- Original URL
- State: open
- Created 3 years ago
- Reactions: 77
- Comments: 33 (20 by maintainers)
Commits related to this issue
- fix(common): Add support for disabling keyvalue sort Previously the KeyValuePipe would always sort, by custom compareFn or the defaultComparator. This will make it possible to disable the sorting, in... — committed to Ringish/angular by Ringish 7 months ago
- fix(common): Add support for disabling keyvalue sort Previously the KeyValuePipe would always sort, by custom compareFn or the defaultComparator. This will make it possible to disable the sorting, ins... — committed to Ringish/angular by Ringish 7 months ago
Works well in Firefox and Chrome with this solution,
I think that semantically:
undefined
-> do the default sortnull
-> do no sortingworks for me.
I feel that this is the right first step. Then we can look into making no sort the default and how we would migrate people who want the current default sort.
I’m using Angular 16 and this is still an issue (more than 2 years now). I’ve implemented the
asIsOrder
solution in order to preserve the key order ofMap
. Can anyone from the Angular team look at this?You really want a default behavior like this?
Result:
I doubt anyone will miss that. I think people are more likely to get upset that it won’t finally be changed. It’s not only related to a
Map
. I would really just remove this sort. Keep it simple.@y0nd0
You can use function references for your own sorting types (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/Collator)
Alternative solution, which has not yet been mentioned
The
keyvalue
value pipe does 2 things today:It seems to me that these two operations are completely unrelated to one another. Imho, we would get the cleanest outcome, if we split the
keyvalue
pipe into:entries
pipe, which would (only) convert the Object/Map into an Array of entriessort
pipe, which would be able to sort any ArrayBackwards compatibility
This approach would avoid the backwards compatibility issue, because we could easily:
keyvalue
pipe as deprecated,keyvalue
pipe + eventually thekeyvalue
pipe itself.And this could be spread over a couple Angular versions, if necessary.
This would also mean that negative surprises for the developers would be reduced, because the current behavior of the
keyvalue
pipe would be kept.Usage examples
It also seems to be a good idea to mention in the
sort
pipe’s documentation/JsDoc that it is generally discouraged, and that developers should try to find an alternative solution to using that pipe - because it seems that sorting should be handled together with other major data processing, and not while rendering.And to avoid confusion, I think that this issue better captures what we want to achieve. So I am actually going to reopen this issue and close the other one!
I agree that if we were to implement this pipe from scratch now then we would have no-sort as the default. The problem with changing to that now is that it is a breaking change.
That being said, I was thinking about this a bit more, and I think we could write a migration that would mitigate the breaking change:
compareFn
keyvalue
pipe in templates, where thecompareFn
option is not specifiedcompareFn
to each of these instanceAs a breaking change this would need to land in a major version (e.g. 13.0.0 or 14.0.0). I am not sure that we have time to achieve this goal for 13.0.0. So the compromise of introducing
null
as a configuration option would at least unblock developers from getting access to the original sort order, and can be landing in any minor version (e.g. 13.0.0, 13.1.0, etc).We could land that as an interim step, and the look at switching the default to
null
in a later major version.How does that sound?
I believe that this behaviour goes back to the early days when different JS engines iterated over object keys in a non-deterministic order.
Interesting solution with the natural sorting with “modern” built in stuff. Thanks for that. Seriously nice input.
But note that this issue is about using
keyvalue
without any setup. Means no custom implementation to disable or change the default sort behavior.This pipe should not sort by default. It should just use the object as it is. I wonder, why did the Angular team or collaborators decided to automatically sort the keys? It’s easier not to sort anything. …
The icing on the cake, the
keyvalue
pipe could provide some preset sort like in my initial post described. We could use your solution withIntl
for thekeyvalue: 'natural'
solution. Or just setkeyvalue: 'sort'
and use the natural sorter (if supported, fallback the simple sort). Zero setup. No additional stuff needed for that. Because in the most cases, we don’t want thatkeyvalue
sorts the object. It’s really annoying to add a compare function that returns zero0
or whatever to disable the default behavior of this pipe. … Or is there an advantage that I didn’t see?So the simplest solution is to remove the default sort. And optional work: Accept a string as argument for a built-in compare function.
Example:
I would prefer to just remove the default sort and not replace by a more complex sorter (natrual sort). Because for performance reasons. The developer should be explicit enable the sort. It would be very comfortable if the pipe also accepts a string. The name of the Angular built-in compare function. For the very first step the keyvalue pipe could just evaluate the parameter as boolean and use the defaultComparator. But keep allowing to pass a custom comparator.
I hope I was able to explain my issue in an understandable way. English is not my main language. So please just ask if something is unclear. 🙂
@infacto - using
null
only appears to work because the pipe is actually crashing in the background. See the console log in https://stackblitz.com/edit/angular-ivy-vkawtv?devtoolsheight=33&file=src/app/app.component.htmlSo this “feature” is still to be implemented.
@mlc-mlapis But Map or simple object has no difference in this case. Both will sort like in my prev example post. So keeping the default sort will touch the order. Also for Maps.
I would also rather use
null
than add an unnecessary method to my component. I think it‘s a harmless implementation.Same here
From the sort point of view, the
null
value looks fine in the sense of don’t apply any rule. Theundefined
value is not used directly in code, so it doesn’t have the sense to compare them.It depends upon what
null
refers to… Ifnull
means “no sort”, then we can add that right now, without a breaking change and with no migration.If we want a way to change the default sorting, then we could consider that
null
is the “default sort”. Then the migration would once again be trivial (just adding thenull
everywhere). But this feels awkward sincenull
does not intuitively map to “sort alphabetically”.We could “possibly” have a compromise, which is that
compareFn
could be one ofFunction|undefined|'lexical'
, which introduces a single string based identifier but only to help with the this backward compatibility. But I would definitely not like to add further string based sorts…Ok I can live this:
But I still don’t understand why you stick to the bad default sorting behavior. The votes speak for this change. What benefits has the current sort behavior?
Let’s track this in the older issue: #31420.
The proposal for this is to add a
null
value for thecompareFn
which indicates “no sort”. Changing the default would be a big breaking change, which we don’t want.https://stackoverflow.com/questions/30076219/does-es6-introduce-a-well-defined-order-of-enumeration-for-object-properties
But probably the most important factor is the transforming of Map structures where the order was/is guaranteed from the beginning and where no historic relations exist at all. And by the same time, it’s the most performant way how to solve some cases.
@splincode Yep, it’s elegant but still, it means about one sorting phase more than necessary.