angular: keyvalue pipe doesn't preserve a Map's sorted order

🐞 bug report

Affected Package

The issue is caused by package @angular/common

Is this a regression?

No

Description

One of the reasons a developer uses a Map over an Object is to have Object-like features with sorted keys. When passing a Map to *ngFor with the keyvalue pipe, the map's sorted order is lost.

The bug is that the keyvalue pipe uses the defaultComparator for Maps. No sorting should be applied when keyvalue is passed a Map.

🔬 Minimal Reproduction

https://stackblitz.com/edit/angular-keyvalue-pipe-map-order?file=src/app/app.component.ts

🌍 Your Environment

Angular Version:


8.0.2

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 58
  • Comments: 33 (17 by maintainers)

Most upvoted comments

@ravishivt Agree with you.

Although I don’t think it’s a bug if they’ve intended it that way, there should be an easy way to preserve the order. Why not leave it as is, then we can sort however we want right? Why apply the defaultComparator instead?

Say, my Map doesn’t follow any sorting algorithm, and I want to keep the structure. There’s not a way to revert back to that unsorted structure even with a custom comparatorFn

EDIT:

Actually I found a simple way to preserve the order

// *.component.ts
asIsOrder(a, b) {
  return 1;
}
<!-- *.component.html -->
<div
      *ngFor="
        let someVal of someMap | keyvalue: asIsOrder;
        let i = index
      ">
</div>

return 1 because a is the element that gets passed in first, and 1 means it should come first in the array

@radotzki Have you heard about map data structure before? Do not disgrace, close the ticket

The point of this issue is that keyvalue pipe should not do any sorting when passed a Map as they “remember the original insertion order of the keys” unlike objects. Please see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map.

+1 to this - respecting Map entry ordering would be super useful.

note that besides Map type, normal object entries traversal is standardized since es6 (https://2ality.com/2015/10/property-traversal-order-es6.html). There is no need to reinvent the wheel here. when iterating over objects in Angular template I’d expect the same behavior as in javascript.

(this kind of surprises is part of what makes the raw JS usage in react more appealing, please don’t reinvent the core language)

This has been driving me crazy. It seems like a very strange choice by the Angular team. Implementing this sorting would be more work than simply using one of the Map object’s built-in iterators, so it’s not an accident. Would love for this to be addressed.

@Drageaux thanks for chiming in here. I thought I was being brigaded for a second. 😸

Your custom sort is an interesting workaround. I still believe though that the default behavior should be to not sort a Map.

Although I don’t think it’s a bug if they’ve intended it that way, there should be an easy way to preserve the order. Why not leave it as is, then we can sort however we want right? Why apply the defaultComparator instead?

That’s exactly my thought as well and my PR #31422 implements such functionality. When a Map is passed with no custom compareFn, the defaultComparator is not applied. But you still have the option to pass in a custom compareFn if you do want a custom sort applied to a Map. So it’s just adjusting the default behavior to no sort.

The team’s preferred solution here is to allow compareFn to be set to null, which indicates that there should be “no sort”.

Actually if you want a trivial JS aligned solution with zero Angular specific cognitive overload then just use the entries() method directly in the template

Don’t use .entries() in template code.

Most likely you’re going to cause ChangedAfterChecked errors as .entries() creates a new array every time the differ evaluates it.

I’ve created a stackblitz showcasing what happens if you do this: https://stackblitz.com/edit/angular-map-entries-in-template. Check the console.

@petebacondarwin, if I’m missing something here, let me know.

@radotzki yeap! sorry this was addressed to @ravishivt

It would be cool to have this change implemented. Iterating over a Map in the template should feel the same as doing it in the component, and this is not the case by now.

In the meantime I am using this pipe (only for Map) that works the same as keyvalue but without altering the order:

import { Pipe, PipeTransform } from '@angular/core';


@Pipe({name: 'mapEntries'})
export class MapEntriesPipe implements PipeTransform {

  transform<K, V>(input: Map<K, V>): Array<any> {
    return Array.from(input).map(([key, value]) => ({ key, value }));
  }
}

Note that I used Array<any> instead of Array<{ key: K; value: V; }> because the Angular Language Service extension shows compilation error “Expected operands to be of comparable types or any” when comparing item.key === otherValueOfKType in the template. 🤷‍♂️

Inspired by the above sample here is a pipe that caches the result to make change detection happier (assumes the map won’t be changed).

import { Pipe, PipeTransform } from '@angular/core';

// Holds a weak reference to its key (here a map), so if it is no longer referenced its value can be garbage collected.
const cache = new WeakMap<ReadonlyMap<any, any>, Array<{ key: any; value: any }>>();

@Pipe({ name: 'mapkeyvalue', pure: true })
export class MapKeyValuePipe implements PipeTransform {
  transform<K, V>(input: ReadonlyMap<K, V>): Iterable<{ key: K; value: V }> {
    const existing = cache.get(input);
    if (existing !== undefined) {
      return existing;
    }

    const iterable = Array.from(input, ([key, value]) => ({ key, value }));
    cache.set(input, iterable);
    return iterable;
  }
}

@Mdelaf I’ve fixed the return typings with the next code:

import {Pipe, PipeTransform} from '@angular/core';
import {KeyValue} from '@angular/common';

@Pipe({
  name: 'mapEntries'
})
export class MapEntriesPipe implements PipeTransform {
  transform<K, V>(input: Map<K, V>): ReadonlyArray<KeyValue<K, V>> {
    return Array.from(input ?? []).map(([key, value]) => ({key, value}));
  }
}

Actually if you want a trivial JS aligned solution with zero Angular specific cognitive overload then just use the entries() method directly in the template:

<p *ngFor="let food of foods.entries()">
  {{food[0]}} (${{food[1].cost}})
</p>

This could not be any more straightforward for a JS developer and will even be more performant than a pipe since it does less processing.

Pipes can be faster than this sort of expression if they are “pure” but the keyvalue pipe is not pure, since it cannot guarantee that the internals of the object/map have not changed between change detection cycles.

I believe main motivation for the keyvalue pipe is that there were no methods (such as keys(), values() and entries() on plain JS objects.

Also the argument for sorting the entries by key goes back to the times when each JS engine might have a different approach to the order in which properties were iterated, so sorting provided a reliable deterministic order across browsers.

@radotzki Have you heard about map data structure before? Do not disgrace, close the ticket

chad