groupBy return either a Map or an OrderedMap: make the type more prec…

…ise (#1924)

CHANGELOG.md

@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 Dates are formatted as YYYY-MM-DD.
 
+## Unreleased
+
+- TypeScript: `groupBy` return either a `Map` or an `OrderedMap`: make the type more precise than base `Collection` [#1924](https://github.com/immutable-js/immutable-js/pull/1924)
+
 ## [4.2.2] - 2023-01-02
 
 - [Flow] Add type for `partition` method [#1920](https://github.com/immutable-js/immutable-js/pull/1920) by [Dagur](https://github.com/Dagur)

__tests__/groupBy.ts

@@ -1,6 +1,53 @@
-import { Collection, Map, Seq } from 'immutable';
+import {
+  Collection,
+  Map,
+  Seq,
+  isOrdered,
+  OrderedMap,
+  List,
+  OrderedSet,
+  Set,
+  Stack,
+  Record,
+} from 'immutable';
 
 describe('groupBy', () => {
+  it.each`
+    constructor   | constructorIsOrdered | isObject
+    ${Collection} | ${true}              | ${false}
+    ${List}       | ${true}              | ${false}
+    ${Seq}        | ${true}              | ${false}
+    ${Set}        | ${false}             | ${false}
+    ${Stack}      | ${true}              | ${false}
+    ${OrderedSet} | ${true}              | ${false}
+    ${Map}        | ${false}             | ${true}
+    ${OrderedMap} | ${true}              | ${true}
+  `(
+    'groupBy returns ordered or unordered of the base type is ordered or not: $constructor.name',
+    ({ constructor, constructorIsOrdered, isObject }) => {
+      const iterableConstructor = ['a', 'b', 'a', 'c'];
+      const objectConstructor = { a: 1, b: 2, c: 3, d: 1 };
+
+      const col = constructor(
+        isObject ? objectConstructor : iterableConstructor
+      );
+
+      const grouped = col.groupBy(v => v);
+
+      // all groupBy should be instance of Map
+      expect(grouped).toBeInstanceOf(Map);
+
+      // ordered objects should be instance of OrderedMap
+      expect(isOrdered(col)).toBe(constructorIsOrdered);
+      expect(isOrdered(grouped)).toBe(constructorIsOrdered);
+      if (constructorIsOrdered) {
+        expect(grouped).toBeInstanceOf(OrderedMap);
+      } else {
+        expect(grouped).not.toBeInstanceOf(OrderedMap);
+      }
+    }
+  );
+
   it('groups keyed sequence', () => {
     const grouped = Seq({ a: 1, b: 2, c: 3, d: 4 }).groupBy(x => x % 2);
     expect(grouped.toJS()).toEqual({ 1: { a: 1, c: 3 }, 0: { b: 2, d: 4 } });
@@ -14,53 +61,38 @@ describe('groupBy', () => {
   });
 
   it('groups indexed sequence', () => {
-    expect(
-      Seq([1, 2, 3, 4, 5, 6])
-        .groupBy(x => x % 2)
-        .toJS()
-    ).toEqual({ 1: [1, 3, 5], 0: [2, 4, 6] });
+    const group = Seq([1, 2, 3, 4, 5, 6]).groupBy(x => x % 2);
+
+    expect(group.toJS()).toEqual({ 1: [1, 3, 5], 0: [2, 4, 6] });
   });
 
   it('groups to keys', () => {
-    expect(
-      Seq([1, 2, 3, 4, 5, 6])
-        .groupBy(x => (x % 2 ? 'odd' : 'even'))
-        .toJS()
-    ).toEqual({ odd: [1, 3, 5], even: [2, 4, 6] });
+    const group = Seq([1, 2, 3, 4, 5, 6]).groupBy(x =>
+      x % 2 ? 'odd' : 'even'
+    );
+    expect(group.toJS()).toEqual({ odd: [1, 3, 5], even: [2, 4, 6] });
   });
 
   it('groups indexed sequences, maintaining indicies when keyed sequences', () => {
-    expect(
-      Seq([1, 2, 3, 4, 5, 6])
-        .groupBy(x => x % 2)
-        .toJS()
-    ).toEqual({ 1: [1, 3, 5], 0: [2, 4, 6] });
-    expect(
-      Seq([1, 2, 3, 4, 5, 6])
-        .toKeyedSeq()
-        .groupBy(x => x % 2)
-        .toJS()
-    ).toEqual({ 1: { 0: 1, 2: 3, 4: 5 }, 0: { 1: 2, 3: 4, 5: 6 } });
-  });
+    const group = Seq([1, 2, 3, 4, 5, 6]).groupBy(x => x % 2);
 
-  it('has groups that can be mapped', () => {
-    expect(
-      Seq([1, 2, 3, 4, 5, 6])
-        .groupBy(x => x % 2)
-        .map(group => group.map(value => value * 10))
-        .toJS()
-    ).toEqual({ 1: [10, 30, 50], 0: [20, 40, 60] });
+    expect(group.toJS()).toEqual({ 1: [1, 3, 5], 0: [2, 4, 6] });
+
+    const keyedGroup = Seq([1, 2, 3, 4, 5, 6])
+      .toKeyedSeq()
+      .groupBy(x => x % 2);
+
+    expect(keyedGroup.toJS()).toEqual({
+      1: { 0: 1, 2: 3, 4: 5 },
+      0: { 1: 2, 3: 4, 5: 6 },
+    });
   });
 
-  it('returns an ordered map from an ordered collection', () => {
-    const seq = Seq(['Z', 'Y', 'X', 'Z', 'Y', 'X']);
-    expect(Collection.isOrdered(seq)).toBe(true);
-    const seqGroups = seq.groupBy(x => x);
-    expect(Collection.isOrdered(seqGroups)).toBe(true);
+  it('has groups that can be mapped', () => {
+    const mappedGroup = Seq([1, 2, 3, 4, 5, 6])
+      .groupBy(x => x % 2)
+      .map(group => group.map(value => value * 10));
 
-    const map = Map({ x: 1, y: 2 });
-    expect(Collection.isOrdered(map)).toBe(false);
-    const mapGroups = map.groupBy(x => x);
-    expect(Collection.isOrdered(mapGroups)).toBe(false);
+    expect(mappedGroup.toJS()).toEqual({ 1: [10, 30, 50], 0: [20, 40, 60] });
   });
 });

type-definitions/immutable.d.ts

@@ -4574,7 +4574,7 @@ declare namespace Immutable {
     ): this;
 
     /**
-     * Returns a `Collection.Keyed` of `Collection.Keyeds`, grouped by the return
+     * Returns a `Map` of `Collection`, grouped by the return
      * value of the `grouper` function.
      *
      * Note: This is always an eager operation.
@@ -4600,7 +4600,7 @@ declare namespace Immutable {
     groupBy<G>(
       grouper: (value: V, key: K, iter: this) => G,
       context?: unknown
-    ): /*Map*/ Seq.Keyed<G, /*this*/ Collection<K, V>>;
+    ): Map<G, this>;
 
     // Side effects
 

type-definitions/ts-tests/groupBy.ts

@@ -0,0 +1,33 @@
+import { List, Map, OrderedMap, Record, Set, Seq, Stack, OrderedSet, DeepCopy, Collection } from 'immutable';
+
+{
+    // $ExpectType Map<string, Indexed<string>>
+    Collection(['a', 'b', 'c', 'a']).groupBy(v => v);
+
+    // $ExpectType Map<string, Keyed<string, number>>
+    Collection({ a: 1, b: 2, c: 3, d: 1 }).groupBy(v => `key-${v}`);
+
+    // $ExpectType Map<string, List<string>>
+    List(['a', 'b', 'c', 'a']).groupBy(v => v);
+
+    // $ExpectType Map<string, Indexed<string>>
+    Seq(['a', 'b', 'c', 'a']).groupBy(v => v);
+
+    // $ExpectType Map<string, Keyed<string, number>>
+    Seq({ a: 1, b: 2, c: 3, d: 1 }).groupBy(v => `key-${v}`);
+
+    // $ExpectType Map<string, Set<string>>
+    Set(['a', 'b', 'c', 'a']).groupBy(v => v);
+
+    // $ExpectType Map<string, Stack<string>>
+    Stack(['a', 'b', 'c', 'a']).groupBy(v => v);
+
+    // $ExpectType Map<string, OrderedSet<string>>
+    OrderedSet(['a', 'b', 'c', 'a']).groupBy(v => v);
+
+    // $ExpectType Map<string, Map<string, number>>
+    Map({ a: 1, b: 2, c: 3, d: 1 }).groupBy(v => `key-${v}`);
+
+    // $ExpectType Map<string, OrderedMap<string, number>>
+    OrderedMap({ a: 1, b: 2, c: 3, d: 1 }).groupBy(v => `key-${v}`);
+}