Allow Reader's local to infer the environment type

[thewilkybarkid]

Using local from Reader recently, I found it a bit surprising that I had to redeclare the type even when I didn't change it.

Looking at #892, I've replicated the generics from fp-ts 1: by default, it treats the type as the same. (If you're changing it, you have to declare it anyhow.)

Reversing the order might mean this should wait for fp-ts 3...

[gcanti]

Since the actual goal of local is changing the environment type, deep down, isn't a good thing you had to redeclare?

[thewilkybarkid]

https://github.com/PREreview/prereview.org/commit/f76f85283323886b09a7576291341b4c13bbe6f8 is my use case: I have functions that manipulate the environment, but they have to be typed explicitly rather than relying on extends. It's ok at the moment as it's a short list of known possibilities so I can overload, but allowing inference would solve it.

If you are changing at the type, you do still have to declare it.

[gcanti]

Reversing the order might mean this should wait for fp-ts 3...

What about adding an overloading instead of reversing the order? Should be backward compatible

// Reader.ts

export function local<R>(f: (r: R) => R): <A>(ma: Reader<R, A>) => Reader<R, A>
export function local<R2, R1>(f: (r2: R2) => R1): <A>(ma: Reader<R1, A>) => Reader<R2, A>
export function local<R2, R1>(f: (r2: R2) => R1): <A>(ma: Reader<R1, A>) => Reader<R2, A> {
  return (ma) => (r2) => ma(f(r2))
}

[thewilkybarkid]

@gcanti Took a bit of trial and error to get overloading to work also with my use (I've added more tests to cover it).

[gcanti]

@thewilkybarkid I tried adding your tests locally and they seems to work with

export function local<R>(f: (r: R) => R): <A>(ma: Reader<R, A>) => Reader<R, A>
export function local<R2, R1>(f: (r2: R2) => R1): <A>(ma: Reader<R1, A>) => Reader<R2, A>
export function local<R2, R1>(f: (r2: R2) => R1): <A>(ma: Reader<R1, A>) => Reader<R2, A> {
  return (ma) => (r2) => ma(f(r2))
}

what's the reason of...

// ..this? ----------v-----------v
export function local<R1, R2 = R1>(f: (r2: R2) => R1): <A>(ma: Reader<R1, A>) => Reader<R2, A>
export function local<R2, R1>(f: (r2: R2) => R1): <A>(ma: Reader<R1, A>) => Reader<R2, A>
export function local<R2, R1>(f: (r2: R2) => R1): <A>(ma: Reader<R1, A>) => Reader<R2, A> {
  return (ma) => (r2) => ma(f(r2))
}

[thewilkybarkid]

You're right, they do work with the simpler version. I also checked it against my project, which caused a failure in my 'normal' usage elsewhere (i.e. changing the environment type), https://github.com/PREreview/prereview.org/blob/eebd28e6b220dd7d8464eff58d7111e6b3234b7f/src/app.ts#L81 (R being fp-ts-routing; local from Reader though it's actually a ReaderMiddleware from hyper-ts). I'll see if I can isolate what's happening there.

Edit: Seems to be something to do with the spread operator (I'm trying to add a new property to the environment locally).

[thewilkybarkid]

// $ExpectType Reader<{ b: string; }, string>
pipe(
  _.of<{ a: string; b: string }, string>('a'),
  _.local((env: { b: string }) => ({
    ...env,
    a: env.b
  }))
)

is an example of a test case that fails with the simplified version: it infers Reader<{ a: string; b: string; }, string> even though { b: string } is typed.

[gcanti]

Good catch.

However

export function local<R1, R2 = R1>(f: (r2: R2) => R1): <A>(ma: Reader<R1, A>) => Reader<R2, A>
export function local<R2, R1>(f: (r2: R2) => R1): <A>(ma: Reader<R1, A>) => Reader<R2, A>
export function local<R2, R1>(f: (r2: R2) => R1): <A>(ma: Reader<R1, A>) => Reader<R2, A> {
  return (ma) => (r2) => ma(f(r2))
}

is not backward compatible because AFAIK the second export function local<R2, R1>(f: (r2: R2) => R1): <A>(ma: Reader<R1, A>) => Reader<R2, A> overloading will never be used.

For example the following code

pipe(
  _.of<string, number>(1),
  _.local<boolean, string>((b) => (b ? 'a' : 'b'))
)

compiles fine with [email protected] and also with

export function local<R>(f: (r: R) => R): <A>(ma: Reader<R, A>) => Reader<R, A>
export function local<R2, R1>(f: (r2: R2) => R1): <A>(ma: Reader<R1, A>) => Reader<R2, A>
export function local<R2, R1>(f: (r2: R2) => R1): <A>(ma: Reader<R1, A>) => Reader<R2, A> {
  return (ma) => (r2) => ma(f(r2))
}

but raises and error after the proposed change

[thewilkybarkid]

I suspect the only option left is to target v3.