gcanti / monocle-ts

Functional optics: a (partial) porting of Scala monocle
https://gcanti.github.io/monocle-ts/
MIT License
1.05k stars 52 forks source link

Setting an optional value doesn't do anything #146

Closed josh-degraw closed 3 years ago

josh-degraw commented 3 years ago

πŸ› Bug report

Current Behavior

Using set to assign an optional, doesn't seem to work like I would expect. I apologize if I'm just not understanding properly. Basically if I have an Optional<S,A> that currently doesn't have a value, and I call set(someValue), it results in a noop, and the value is not set.

It currently only seems to set the value when the getOption(S) returns Some, which is quite frustrating as I otherwise need to so something else like using a Lens<S, Option<A>> and mapping the value of that.

Expected behavior

I would expect that after calling set, regardless of whether or not a value existed previously, the value passed to set should be present on the resulting object.

Reproducible example

interface State {
  nullableItem: string | null
}
const state: State = {
  nullableItem: null
}

// Pipe-based
const itemLens = pipe(lens.id<State>(), lens.prop('nullableItem'), lens.fromNullable)
expect(itemLens.set('non-null value')(state).nullableItem).toBe('value') // Fails because `nullableItem` is null

// Class-based
const itemOption = Optional.fromPath<State>()(['nullableItem'])
expect(itemOption.set('value')(state).nullableItem).toBe('value') // Fails because `nullableItem` is null

Your environment

Which versions of monocle-ts are affected by this issue? Did this work in previous versions of monocle-ts?

Software Version(s)
monocle-ts ^2.3.3
fp-ts ^2.8.2
TypeScript ^4.1.2
gcanti commented 3 years ago

@josh-degraw the current behavior is correct according to the Optional laws

  1. pipe(getOption(s), fold(() => s, a => set(a)(s))) = s
  2. getOption(set(a)(s)) = pipe(getOption(s), map(_ => a))
  3. set(a)(set(a)(s)) = set(a)(s)

I otherwise need to so something else like using a Lens<S, Option> and mapping the value of that

That's right, you need a Lens.

josh-degraw commented 3 years ago

Okay, fair enough if I misunderstood the intended behavior, my bad. It may be worth making some clarifications in the docs on the difference between when you would want to use an Optional<S,A> vs using a Lens<S, Option<A>>, since as a newcomer to using lenses it was not clear when you would use one over the other (even looking at the laws, and already having a decent amount of experience with fp, just not with lenses).

I spent a few hours trying to figure out why my setter was not being respected and some sort of documentation to clarify these points would be incredibly useful. Maybe even just a disclaimer on the differences between Option and Optional would be helpful, if not a few examples or utilities demonstrating how to work with the different use-cases.

Are there any existing utilities to assist in working with Lens<S, Option<A>>? There are many things that are available via Optional<S,A> such as optional.prop(string) et. al. that AFAIK will require a lot more manual tweaking when using Lens<S, Option<A>>. Any help or pointing in the direction of existing documentation would be greatly appreciated.

Carsten-Leue commented 1 year ago

Hi @josh-degraw I am struggling with the same usecase. How have you solved it on your end, in particular when it comes to nested optional properties such as in

interface Inner {
  someprop?: string
}

interface Outer {
  inner?: Inner
}

how would a les for someprop.inner be constructed and used?