mobily/ts-belt

Wrong types/behavior for A.groupBy or D.mapWithKey

Closed this issue · 4 comments

	const someValues = [ { id: 1, value: "hello" }, { id: 2, value: "world" } ]
    pipe(
        someValues,
        A.groupBy(value => value.id),
        D.mapWithKey((id, values) => {
			console.log(typeof id) // prints out string
			const numId: number = id; // this should throw, but it doesn't because type is number instead of string

			return values;
        }),
    );

This is kind of interesting problem it could be either type issue or implementation issue. First solution could be that it will be same type as value.id (A.groupBy(value => value.id)) and only types that could become object keys should be allowed (probably only number/string ?). Then implementation needs to be fixed.

Or if current implementation is correct it should be types as always string.

@Nodonisko this is fixed in v4.0.0-rc.0 (the latest is v4.0.0-rc.1)

CleanShot 2022-12-04 at 18 37 56@2x

Thanks, sadly I can't use rc version for our app, but soon as it will be released I will try it.

I also have problems with A.groupBy due to the way it's typed:

import {A, D, pipe} from "@mobily/ts-belt"

function groupErrors(errors: Dict<string>): Dict<string[]> {
  return pipe(
    errors,
    D.toPairs,
    A.groupBy(pair => pair[1]), 
    D.map((pairs: any): string[] => // <-- remove this `any` suppressor to see an issue
      A.map(pairs, (pair: [string, string]) => pair[0])
    )
  )
}

console.log(groupErrors({
  login1: "NotFoundError",
  login2: "NotFoundError",
  login3: "TimeoutError",
  login4: "TimeoutError",
}))

// ->
const result = {
  NotFoundError: ["login1", "login2"],
  TimeoutError: ["login3", "login4"]
}
void result

Screen Shot 2023-01-10 at 13 46 49

Original typing:

export declare function groupBy<A>(xs: Array<A>, groupFn: (item: A) => PropertyKey): Record<PropertyKey, readonly [A, ...A[]]>;
export declare function groupBy<A>(groupFn: (item: A) => PropertyKey): (xs: Array<A>) => Record<PropertyKey, readonly [A, ...A[]]>;

The example code above works fine with:

export declare function groupBy<A>(xs: Array<A>, groupFn: (item: A) => PropertyKey): Record<PropertyKey, Array<A>>;
export declare function groupBy<A>(groupFn: (item: A) => PropertyKey): (xs: Array<A>) => Record<PropertyKey, Array<A>>;

Change: readonly [A, ...A[]]> --> Array<A>

In my experience readonly modifiers bring more harm than good 😞

@ivan-kleshnin You can probably wrap pairs in F.toMutable(pairs) to solve issue, but I agree with you that readonly are very problematic. I already created this issue about them #65