gcanti / fp-ts

Functional programming in TypeScript
https://gcanti.github.io/fp-ts/
MIT License
10.73k stars 504 forks source link

Calling Task.traverseArray on a ReadonlyNonEmptyArray returns a ReadonlyArray #1404

Closed thewilkybarkid closed 2 years ago

thewilkybarkid commented 3 years ago

🐛 Bug report

Current Behavior

Calling Task.traverseArray on a ReadonlyNonEmptyArray returns a ReadonlyArray.

Expected behaviour

That it returns a ReadonlyNonEmptyArray.

Reproducible example

import { readonlyNonEmptyArray as RNEA, task as T, function as f } from 'fp-ts';

declare const rats: ReadonlyArray<T.Task<string>>
declare const rneats: RNEA.ReadonlyNonEmptyArray<T.Task<string>>

const ra: T.Task<ReadonlyArray<string>> = f.pipe(rats, T.sequenceArray)
const rnea: T.Task<RNEA.ReadonlyNonEmptyArray<string>> = f.pipe(rneats, T.sequenceArray)

TS Playground; Original source

Suggested solution(s)

Overloading if possible (don't think generics would work); otherwise separate functions?

Additional context

Applies to various other functions where the length of the array doesn't change (eg T.sequenceArray).

I've had a quick go with first generics then overloading, but seems a bit more complicated than hoped.

Your environment

Software Version(s)
fp-ts 2.9.5
TypeScript 4.1.5
gcanti commented 3 years ago

In order to avoid a combinatorial explosion of functions I propose to only add the most powerful traverseReadonlyNonEmptyArrayWithIndex function

traverseReadonlyNonEmptyArray and sequenceReadonlyNonEmptyArray can be easily derived:

traverseReadonlyNonEmptyArray(f) = traverseReadonlyNonEmptyArrayWithIndex((_, a) => f(a))
sequenceReadonlyNonEmptyArray = traverseReadonlyNonEmptyArrayWithIndex((_, a) => a)
thewilkybarkid commented 3 years ago

Would overloading work? I can take another look into it.

gcanti commented 3 years ago

Would overloading work?

Not sure it's possible, last time I tried I got:

This overload signature is not compatible with its implementation signature.ts(2394)

Adding a separate function is ok though (for example I could also add a traverseReadonlyRecordWithKey in the future).

thewilkybarkid commented 3 years ago

Would overloading work?

Not sure it's possible, last time I tried I got:

This overload signature is not compatible with its implementation signature.ts(2394)

Took a quick look and you can get around that by making the array type generic too

export function traverseArrayWithIndex<A, B, C extends ReadonlyNonEmptyArray<A>, D extends ReadonlyNonEmptyArray<B>>(f: (index: number, a: A) => Task<B>): (as: C) => Task<D>;
export function traverseArrayWithIndex<A, B, C extends ReadonlyArray<A>, D extends ReadonlyArray<B>>(f: (index: number, a: A) => Task<B>): (as: C) => Task<D>;
export function traverseArrayWithIndex<A, B>(f: (index: number, a: A) => Task<B>) {
  return (as: ReadonlyArray<A>) => () => Promise.all(as.map((x, i) => f(i, x)()));
}

but the change does need to be propagated.

thewilkybarkid commented 3 years ago

Looking at https://github.com/gcanti/fp-ts/commit/0fb9ad73c04131b03102c1cd500aa42355f3b07b though, it seems like RNEA.traverseWithIndex etc no longer have the performance warning so we could just use that instead...

thewilkybarkid commented 3 years ago
export const traverseArrayWithIndex = <A, B>(f: (index: number, a: A) => Task<B>) => <C extends ReadonlyArray<A>>(
  as: C
): Task<C extends ReadonlyNonEmptyArray<A> ? ReadonlyNonEmptyArray<B> : ReadonlyArray<B>> => () =>
  Promise.all(as.map((x, i) => f(i, x)())) as any

works, but type inference for A and B is lost (e.g. in traverseArray).

thewilkybarkid commented 3 years ago
export const traverseArrayWithIndex = <A, B>(f: (index: number, a: A) => Task<B>) => <C>(
  as: ReadonlyArray<A> & {0?: C}
): Task<C extends A ? ReadonlyNonEmptyArray<B> : ReadonlyArray<B>> => () =>
  Promise.all(as.map((x, i) => f(i, x)())) as any

works, and retains type inference.

Can open a PR with these changes.