gcanti / fp-ts

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

Should Record monoid include props that aren't shared between records? #1595

Open kylegoetz opened 3 years ago

kylegoetz commented 3 years ago

🐛 Bug report

Consider the code

const getMonoidSame: <A>() => Monoid<A|undefined> = () => ({
  empty: undefined,
  concat: (a, b) => a === b ? a : undefined,
})

const keepOnlyCommonValues = Monoid.concatAll(Record.getMonoid(getMonoidSame()))

I would expect

const entries = [
  { foo: '1', bar: '2' },
  { foo: '1', },
]

keepOnlyCommonValues(entries) // { foo: '1' }

but instead I get { foo: '1', bar: '2' } because bar is only in the first entry, so getMonoidSame().concat is not called for bar.

It seems like concatenating two records should run concat on all props rather than skipping the unshared ones and then including them in the result.

Edit The use case here is if you're loading up a "bulk edit" dialog for an array of data of the same type, you'd want the inputs in your page corresponding to foo to be initialized with the shared value of foo but since bar does not have a common value, you'd want that input to be initialized as blank (so as not to be overwriting bar values by default).

gcanti commented 3 years ago

Note that getMonoidSame doesn't return a Monoid

const M = getMonoidSame<number>()

console.log(M.concat(1, M.empty)) // => undefined, should be 1
kylegoetz commented 3 years ago

Ah, I got confused about the monoid law. I thought e was the multiplicative zero not the multiplicative identity.