gcanti / fp-ts

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

ReadonlyArray Intersection doesn't reference values from first array #1504

Open kschwarz1116 opened 3 years ago

kschwarz1116 commented 3 years ago

🐛 Bug report

Current Behavior

These log different things:

import * as Eq from "fp-ts/lib/Eq";
import * as N from "fp-ts/lib/number";
import * as RA from "fp-ts/lib/ReadonlyArray";

console.log(
  RA.intersection(Eq.contramap((a: { id: number; foo: string }) => a.id)(N.Eq))(
    [{ id: 1, foo: "a" }],
    [{ id: 1, foo: "b" }]
  )
); // [{id: 1, foo: "a"}]
console.log(
  RA.intersection(
    Eq.contramap((a: { id: number; foo: string }) => a.id)(N.Eq)
  )([{ id: 1, foo: "a" }])([{ id: 1, foo: "b" }])
); // [{id: 1, foo: "b"}]

Expected behavior

The docs for intersection say:

Creates an array of unique values that are included in all given arrays using a Eq for equality comparisons. The order and references of result values are determined by the first array.

I'd expect the second log to match the first if that were the case.

Reproducible example

Code Sandbox

Suggested solution(s)

If the intent of the additional signature was to match the arg order of the first signature, then the behavior should line up. Otherwise, the docs should change to match what the actual behavior is.

The tests should also address this case.

Your environment

Software Version(s)
fp-ts 2.10.5
TypeScript 4.2.3
gcanti commented 3 years ago

In general curried (or data-last) functions are supposed to be used along with pipe:

import * as Eq from 'fp-ts/Eq'
import { pipe } from 'fp-ts/function'
import * as N from 'fp-ts/number'
import * as RA from 'fp-ts/ReadonlyArray'

const E = pipe(
  N.Eq,
  Eq.contramap((a: { id: number; foo: string }) => a.id)
)

pipe(
  [{ id: 1, foo: 'a' }],
  RA.intersection(E)([{ id: 1, foo: 'b' }]),
  console.log
) // [{id: 1, foo: "a"}]
kschwarz1116 commented 3 years ago

I definitely get that. And if that's what the intended functionality is, then it should continue to work like that. However, the documentation doesn't make it clear that your example will return elements from [{ id: 1, foo: 'a' }] that match elements in [{ id: 1, foo: 'b' }]. I'd still say that the first array being applied to RA.intersection in your example is [{id: 1, foo: 'b'}].

If the only thing I'm reading in the documentation is the function signature:

export declare function intersection<A>(
  E: Eq<A>
): {
  (xs: ReadonlyArray<A>): (ys: ReadonlyArray<A>) => ReadonlyArray<A>
  (xs: ReadonlyArray<A>, ys: ReadonlyArray<A>): ReadonlyArray<A>
}

I'd expect xs and ys to work the same way in each call.

gcanti commented 3 years ago

fair enough, what do you suggest?