leemhenson commented 6 years ago

Hi. I'm trying to use Free to describe a program that exists always in terms of an Either<L, A>. All my instructions are binary in that way, e.g.

export class CountAvailableUnits<L, A> {
  public readonly _tag: "CountAvailableUnits" = "CountAvailableUnits";
  public readonly _A!: A;
  public readonly _URI!: InstructionURI;

  constructor(
    readonly id: string;
    readonly more: (a: Either<L, number>) => Either<L, A>,
  ) {}
}

When I attempt to foldFree(either)(interpreter, program).value I end up with Either<{}, Either<L, A>>. I can't figure out the secret sauce to make foldFree do what I want. It looks like it should, based on the signature of FoldFree2, but this is beyond my ability to debug. Any ideas?

Thanks

gcanti commented 6 years ago

@leemhenson hard to say without a complete example, could you please provide a small repro?

leemhenson commented 6 years ago

So in the process of writing out a repro I realised that I was probably doing something silly. This is what I've got now, and it compiles ok:

import * as free from '../src/Free'
import { Either, either } from '../src/Either'

export class Degree {
  readonly value: number
  constructor(d: number) {
    this.value = (d + 360) % 360
  }
}

declare module '../src/HKT' {
  interface URI2HKT<A> {
    EitherInstruction: InstructionF<A>
  }
}

export const InstructionFURI = 'EitherInstruction'

export type InstructionFURI = typeof InstructionFURI

export class Position {
  constructor(readonly x: number, readonly y: number, readonly heading: Degree) {}
}

export class ForwardError extends Error {
  constructor() {
    super('Forward error')
    this.name = this.constructor.name
  }
}

export class BackwardError extends Error {
  constructor() {
    super('Backward error')
    this.name = this.constructor.name
  }
}

export class Forward<A> {
  readonly _tag: 'Forward' = 'Forward'
  readonly _A!: A
  readonly _URI!: InstructionFURI
  constructor(
    readonly position: Position,
    readonly length: number,
    readonly more: (p: Either<ForwardError, Position>) => Either<ForwardError, A>
  ) {}
}

export class Backward<A> {
  readonly _tag: 'Backward' = 'Backward'
  readonly _A!: A
  readonly _URI!: InstructionFURI
  constructor(
    readonly position: Position,
    readonly length: number,
    readonly more: (p: Either<BackwardError, Position>) => Either<BackwardError, A>
  ) {}
}

export type InstructionF<A> = Forward<A> | Backward<A>

export const forward = (position: Position, length: number) => free.liftF(new Forward(position, length, a => a))
export const backward = (position: Position, length: number) => free.liftF(new Backward(position, length, a => a))

const computation = {
  forward(position: Position, length: number): Position {
    const degree = position.heading.value
    if (degree === 0) {
      return new Position(position.x + length, position.y, position.heading)
    } else if (degree === 90) {
      return new Position(position.x, position.y + length, position.heading)
    } else if (degree === 180) {
      return new Position(position.x - length, position.y, position.heading)
    } else if (degree === 270) {
      return new Position(position.x, position.y - length, position.heading)
    }
    throw new Error(`Unkonwn direction ${degree}`)
  },
  backward(position: Position, length: number): Position {
    return computation.forward(new Position(position.x, position.y, new Degree(position.heading.value + 180)), length)
  }
}

export function interpretEither<A>(fa: InstructionF<A>): Either<ForwardError | BackwardError, A> {
  switch (fa._tag) {
    case 'Forward':
      return fa.more(either.of<ForwardError, Position>(computation.forward(fa.position, fa.length)))
    case 'Backward':
      return fa.more(either.of<BackwardError, Position>(computation.backward(fa.position, fa.length)))
  }
}

const program = (start: Position) => forward(start, 10).chain(p1 => backward(p1, 5))

const result = free.foldFree(either)(interpretEither, program(new Position(0, 0, new Degree(180))))  // result: Either<ForwardError | BackwardError, Position>

I realised that I don't need to make the instructions binary types. What I'm trying to model is that an instruction can raise an error (e.g. it represents a database query that could fail due to network, bad sql syntax etc). But in these situations the set of errors is known, so I can enumerate them in the L of the Either they return. Then I can union them at the top level, giving me Either<AllErrorTypes, A>.

Does that sound like the right approach to you?

leemhenson commented 6 years ago

Ah yes, it compiles but it doesn't do what I want. 😆

This bit:

const program = (start: Position) => 
  forward(start, 10)
    .chain(p1 => backward(p1, 5))    // p1 should be Either<ForwardError, Position>, not Position

I want to surface the Eithers in my program so I define logic around them. But of course the interpreter is hiding them away.

gcanti commented 6 years ago

more is supposed to return A here

export class Forward<A> {
  readonly _tag: 'Forward' = 'Forward'
  readonly _A!: A
  readonly _URI!: InstructionFURI
  constructor(
    readonly position: Position,
    readonly length: number,
    readonly more: (p: Either<ForwardError, Position>) => A
  ) {}
}

export class Backward<A> {
  readonly _tag: 'Backward' = 'Backward'
  readonly _A!: A
  readonly _URI!: InstructionFURI
  constructor(
    readonly position: Position,
    readonly length: number,
    readonly more: (p: Either<BackwardError, Position>) => A
  ) {}
}

What I'm trying to model is that an instruction can raise an error

Actually you already did this since more accepts an Either<ForwardError, Position> as parameter.

Roughly speaking:

an operation which normally would return void is modelled with more: A
an operation which normally would return a type T is modelled with more: (t: T) => A

Then you can just write an interpreter for Identity

export function interpretIdentity<A>(fa: InstructionF<A>): Identity<A> {
  switch (fa._tag) {
    case 'Forward':
      return identity.of(fa.more(either.of(computation.forward(fa.position, fa.length))))
    case 'Backward':
      return identity.of(fa.more(either.of(computation.backward(fa.position, fa.length))))
  }
}

const program = (start: Position) =>
  forward(start, 10).chain(ep1 => ep1.fold(() => backward(start, 5), p1 => backward(p1, 5)))
//                                         ^ dummy error handler

// const result: Identity<Either<BackwardError, Position>>
const result = free.foldFree(identity)(interpretIdentity, program(new Position(0, 0, new Degree(180)))) // result: Either<ForwardError | BackwardError, Position>
console.log(result.value)
/*
right({
  "x": -5,
  "y": 0,
  "heading": {
    "value": 0
  }
})
*/

gcanti commented 6 years ago

Roughly speaking

Let's make the things clearer, this is the general rule:

An operation witch normally would accept the parameters p1: p1, p2: P2, p3: P3, etc. and return a type T is modelled as a data structure:

class OperationFoo<A> {
  readonly _tag: 'OperationFoo' = 'OperationFoo'
  readonly _A!: A
  readonly _URI!: OperationsFURI
  constructor(
    readonly p1: P1  // <= parameters
    readonly p1: P2, // <= parameters
    ...
    readonly more: (t: T) => A
//                     ^---- return type        
  ) {}
}

So an operation which normally would return void (or undefined or whatever we call the unit type) is modelled with

more: (t: undefined) => A

i.e.

more: () => A

or simply

more: A

since the parameter is useless

p.s. Personally I find finally tagless way more flexible and easy to grasp

leemhenson commented 6 years ago

Thanks Giulio, this is instructive.

Personally I find finally tagless way more flexible and easy to grasp.

I've seen people talking about finally tagless but I don't understand what those words refer to. 😅 I was under the impression that it was the same thing as mtl, but perhaps I'm mistaken? I tried writing the equivalent code that I worked on with Free using https://github.com/gcanti/fp-ts/blob/master/examples/mtl.ts as a guide. I found it quite difficult to recommend due to the amount of boilerplate and visual noise from Typescript's inability to infer types as well as Haskell or Purescript can. Do you not have the same experience?

I'm basically trying to work out what is a good, general-purpose approach for making effectful code testable that I can guide my team towards. As per my comments with mtl above, I think I'm solely left with Free or using Reader* to deliver dictionaries of => IO or => Task functions that I can replace at test time. The latter seems to be the easiest sell at the moment, and is going to be more performant than Free too I guess.

I'd love to hear what your "default setting" is for writing testable effectful code.

gcanti commented 6 years ago

AFAIK finally tagless and mtl are (kind of) synonyms

I found it quite difficult to recommend due to the amount of boilerplate and visual noise from Typescript's inability to infer types as well as Haskell or Purescript can

Yes, I feel your pain, while in Haskell/PureScript (or Scala) looks like a no brainer, in TypeScript there's a lot of ceremonies. Perhaps it might worth it in a library but it's a hard sell in the application code

I'm basically trying to work out what is a good, general-purpose approach for making effectful code testable that I can guide my team towards

I'm trying to find a good balance as well. I'll try to collect my thoughts, I need some time though.

using Reader* to deliver dictionaries of => IO or => Task functions that I can replace at test time

Using Reader seems a good option to avoid passing around the static dictionaries by hand, however I think that is tangential to the heart of the problem which is to abstract over the effect M of a general effectful program (I might be wrong though, I'll think about it)

op: (p1: P1, p2: P2, ...) => M<T>

sledorze commented 6 years ago

I'm also suspecting that Purescript with its RowToList machinery can now do a lot of the structural transformation Typescript is offering. Personally I miss Purescript transparent Type class resolution & inference. on the other hand Typescript has all the (precise) externs. Maybe Purescript would work in the center of an onion architecture with Typescript for adapters (assuming a productive Purescript / Typescript glue solution)..

gcanti commented 6 years ago

Disclaimer: this is pretty long...

Let's say we want to implement the following program

try to load the user from the localStorage
if is not there try to load the user from a remote endpoint
every operation will be logged

Scenario 1: using mtl / finally tagless

First of all let's define the domain model and the capabilities

// mtl.ts
import { Either } from 'fp-ts/lib/Either'
import { HKT } from 'fp-ts/lib/HKT'
import { Monad } from 'fp-ts/lib/Monad'
import { none, Option, some } from 'fp-ts/lib/Option'

export interface User {
  id: string
  name: string
}

// abstract local storage
interface MonadStorage<M> {
  getItem: (key: string) => HKT<M, Option<string>>
}

// abstract remote endpoint
interface MonadUser<M> {
  fetch: (userId: string) => HKT<M, Either<string, Option<User>>>
}

// abstract logging
interface MonadLogger<M> {
  log: (message: string) => HKT<M, void>
}

export interface MonadApp<M> extends MonadStorage<M>, MonadUser<M>, MonadLogger<M>, Monad<M> {}

then the main program

// mtl.ts
export class Main {
  getMain<M>(M: MonadApp<M>): (userId: string) => HKT<M, Option<User>> {
    const withLog = <A>(message: string, fa: HKT<M, A>): HKT<M, A> => M.chain(M.log(message), () => fa)
    const parseUser = (s: string): User => JSON.parse(s)
    return userId => {
      const localUser = M.chain(M.getItem(userId), o =>
        o.fold(withLog('local user not found', M.of(none)), s => M.of(some(parseUser(s))))
      )
      const remoteUser = M.chain(M.fetch(userId), e =>
        e.fold(
          () => withLog('error while retrieving user', M.of(none)),
          o =>
            o.fold(withLog('remote user not found', M.of(none)), user => withLog('remote user found', M.of(some(user))))
        )
      )
      return M.chain(localUser, o => o.fold(remoteUser, user => M.of(some(user))))
    }
  }
}

In order to test the program we must define a test instance of MonadApp, so first of all let's define an overloading of getMain for the State monad

// mtl-test.ts
import { Either, left, right } from 'fp-ts/lib/Either'
import { Type2, URIS2 } from 'fp-ts/lib/HKT'
import { Monad2 } from 'fp-ts/lib/Monad'
import { fromNullable, Option } from 'fp-ts/lib/Option'
import { state, State, URI } from 'fp-ts/lib/State'
import { Main, User } from './mtl'

//
// mtl augmentation
//

interface MonadStorage2<M extends URIS2, L> {
  getItem: (key: string) => Type2<M, L, Option<string>>
}

interface MonadUser2<M extends URIS2, L> {
  fetch: (userId: string) => Type2<M, L, Either<string, Option<User>>>
}

interface MonadLogger2<M extends URIS2, L> {
  log: (message: string) => Type2<M, L, void>
}

interface MonadApp2<M extends URIS2, L> extends MonadStorage2<M, L>, MonadUser2<M, L>, MonadLogger2<M, L>, Monad2<M> {}

declare module './mtl' {
  interface Main {
    getMain<M extends URIS2, L>(M: MonadApp2<M, L>): (userId: string) => Type2<M, L, Option<User>>
  }
}

Now we can define a instance for State

// mtl-test.ts
export interface TestState {
  localStorage: Record<string, string>
  users: Record<string, User>
  error: boolean
  log: Array<string>
}

const testInstance: MonadApp2<URI, TestState> = {
  ...state,
  getItem: key => new State(s => [fromNullable(s.localStorage[key]), s]),
  fetch: userId => new State(s => (s.error ? [left('500'), s] : [right(fromNullable(s.users[userId])), s])),
  log: message => new State(s => [undefined, { ...s, log: s.log.concat(message) }])
}

const main = new Main().getMain(testInstance)
// result: State<TestState, Option<User>>
const result = main('abc')

Finally we can actually test the program with different initial states

// mtl-test.ts
const testState1 = { localStorage: {}, users: { abc: { id: 'abc', name: 'Giulio' } }, error: false, log: [] }
console.log(result.run(testState1))
/*
[ some({
  "id": "abc",
  "name": "Giulio"
}),
  { localStorage: {},
    users: { abc: [Object] },
    error: false,
    log: [ 'local user not found', 'remote user found' ] } ]
*/
const testState2 = { localStorage: {}, users: {}, error: false, log: [] }
console.log(result.run(testState2))
/*
[ none,
  { localStorage: {},
    users: {},
    error: false,
    log: [ 'local user not found', 'remote user not found' ] } ]
*/

Scenario 2: using concrete types

Let's still define the capabilities but with concrete types

// concrete.ts
import { User } from './mtl'
import { IO } from 'fp-ts/lib/IO'
import { Option, some, none, fromNullable } from 'fp-ts/lib/Option'
import { Task, fromIO, task } from 'fp-ts/lib/Task'
import { Either, left, right } from 'fp-ts/lib/Either'

interface MonadStorage {
  getItem: (key: string) => IO<Option<string>>
}

interface MonadUser {
  fetch: (userId: string) => Task<Either<string, Option<User>>> // or TaskEither<string, Option<User>>
}

interface MonadLogger {
  log: (message: string) => IO<void>
}

interface MonadApp extends MonadStorage, MonadUser, MonadLogger {}

and then the main program

// concrete.ts
const getMain = (M: MonadApp): ((userId: string) => Task<Option<User>>) => {
  const withLog = <A>(message: string, fa: Task<A>): Task<A> => fromIO(M.log(message)).chain(() => fa)
  const parseUser = (s: string): User => JSON.parse(s)
  return userId => {
    const localUser = fromIO(M.getItem(userId)).chain(o =>
      o.fold(withLog('local user not found', task.of(none)), s => task.of(some(parseUser(s))))
    )
    const remoteUser = M.fetch(userId).chain(e =>
      e.fold(
        () => withLog('error while retrieving user', task.of(none)),
        o =>
          o.fold(withLog('remote user not found', task.of(none)), user =>
            withLog('remote user found', task.of(some(user)))
          )
      )
    )
    return localUser.chain(o => o.fold(remoteUser, user => task.of(some(user))))
  }
}

How can I test this program? We need a test instance for MonadApp and I'll use IORef for this

// concrete.ts
import { TestState } from './mtl-test'
import { IORef } from './IORef' // <= this is simply a local copy of IORef

const getTestInstance = (ref: IORef<TestState>): MonadApp => {
  return {
    getItem: key => ref.read.map(s => fromNullable(s.localStorage[key])),
    fetch: userId => fromIO(ref.read.map(s => (s.error ? left('500') : right(fromNullable(s.users[userId]))))),
    log: message => ref.modify(s => ({ ...s, log: s.log.concat(message) }))
  }
}

const testState1 = { localStorage: {}, users: { abc: { id: 'abc', name: 'Giulio' } }, error: false, log: [] }
const ref1 = new IORef(testState1)
const testInstance1 = getTestInstance(ref1)
getMain(testInstance1)('abc')
  .chain(result => fromIO(ref1.read).map(state => [result, state]))
  .run()
  .then(console.log)
/*
[ some({
  "id": "abc",
  "name": "Giulio"
}),
  { localStorage: {},
    users: { abc: [Object] },
    error: false,
    log: [ 'local user not found', 'remote user found' ] } ]
*/
const testState2 = { localStorage: {}, users: {}, error: false, log: [] }
const ref2 = new IORef(testState2)
const testInstance2 = getTestInstance(ref2)
getMain(testInstance2)('abc')
  .chain(result => fromIO(ref2.read).map(state => [result, state]))
  .run()
  .then(console.log)
/*
[ none,
  { localStorage: {},
    users: {},
    error: false,
    log: [ 'local user not found', 'remote user not found' ] } ]
*/

using Reader* to deliver dictionaries of => IO or => Task functions that I can replace at test time

@leemhenson In both scenarios, using Reader instead of passing MonadApp (or the other static dictionaries) manually seems a good option

gcanti commented 6 years ago

Scenario 2 is my preferred general-purpose approach at the moment since is more beginner friendly

Pros

explicit capabilities (MonadStorage, MonadUser, etc...)
concrete types are easier to manipulate / reason about
faked do notation: you can use map, chain etc.. as methods
the work required to test programs with IORef looks more or less equivalent to mtl + State

Cons

you lose some generality

For example what if logging becomes async (let's say you want to log to a file)?

In many cases such refactoring looks straightforward though: let's change MonadLogger to cope with async loggers

interface MonadLogger {
-  log: (message: string) => IO<void>
+  log: (message: string) => Task<void>
}

TypeScript raises 2 errors which are easy to fix

const getMain = (M: MonadApp): ((userId: string) => Task<Option<User>>) => {
-  const withLog = <A>(message: string, fa: Task<A>): Task<A> => fromIO(M.log(message)).chain(() => fa)
+  const withLog = <A>(message: string, fa: Task<A>): Task<A> => M.log(message).chain(() => fa)
  ...
}
...
const getTestInstance = (ref: IORef<TestState>): MonadApp => {
  return {
    getItem: key => ref.read.map(s => fromNullable(s.localStorage[key])),
    fetch: userId => fromIO(ref.read.map(s => (s.error ? left('500') : right(fromNullable(s.users[userId]))))),
-    log: message => ref.modify(s => ({ ...s, log: s.log.concat(message) }))
+    log: message => fromIO(ref.modify(s => ({ ...s, log: s.log.concat(message) })))
  }
}

In general, both in browser and in node, you likely end up with a program running in the TaskEither monad since pretty much every program needs

async effect -> Task
error handling -> Either

The resulting monad which combines those 2 effects is then TaskEither.

I used scenario 2 in order to test fp-typed-install which is a simple demo of porting an impure program to functional style. This can be slightly simplified using IORef.

raveclassic commented 6 years ago

@gcanti Wonderful examples! Could you add them to the docs?

sledorze commented 6 years ago

Aren't any of you using TaskEither with rich error types? I'm more and more using fully typed union errors types.

In the form:

FutureEither<UserNotFound | Unauthorized | ..., Token>

IOEither<UserNotFound | Unauthorized | ..., Token>

Basic idea is that each Capability exhibit business level errors in the left part and it is to the consumer of the capability to deal with business error in meaningful ways.

(I use Future/IO from funfix instead of IO/Task from fp-ts)

Also there can still be technical errors in the underlying Future / IO to deal with.

To make that possible, I use a special version of chain to augment the Left part with new possible errors.

  chain<L2, B>(f: (a: A) => FutureEither<L2, B>): FutureEither<L | L2, B> {
    return new FutureEither<L | L2, B>(eitherTFuture.chain<L | L2, A, B>((a: A) => f(a).value, this.value))
  }

and also a focus helper function (some other may come) to be able to put in focus a possible type (either from the Right or Left side).

  focus<A2 extends L | A, L2 = Exclude<L | A, A2>>(predicate: (x: L | A) => x is A2): FutureEither<L2, A2> {
    return new FutureEither<L2, A2>(
      this.value.map(e => {
        const v = e.value
        return predicate(v) ? eitherRight<L2, A2>(v) : eitherLeft<L2, A2>(v as any)
      })
    )
  }

We're trying it on our code base, I would like to get some feedback (thoughts on it). I know this is not traditional way of doing things in languages with (closed) ADTs however with Typescript inference I think we may benefit from more precise contrôle on the error side.

I'm open to feedback / criticisms of the approach.

leemhenson commented 6 years ago

@gcanti Grazie, these are terrific walkthroughs. I do like the clarity of the mtl approach, and the implementation using concrete types + ioref looks like it might make the boilerplate signal/noise ratio acceptable. I'll try it out next time I get a chance!

@sledorze :

Aren't any of you using TaskEither with rich error types?

Yeah, we used to use TaskEither<Error, A> but now we're doing TaskEither<ExplicitErrorA | ExplicitErrorB | etc, A>. The documentary information when looking at a fn signature if great, except that the latest vscode insiders builds seem to have started minifying long signatures by substituting in an ellipsis: TaskEither<...> which is no help to anyone! 😭

sledorze commented 6 years ago

@leemhenson if you're using also Unions of Errors, aren't you experiencing the limitations induced by the chain signature (among others)? In its current incarnation; one has to know the set of all errors beforehand and not having global inference means a lot of type maintenance.

I would love to have your feedback on this @leemhenson @gcanti. It's not because we can that we should but I stress this idea as I'm not sure the laws it breaks.

leemhenson commented 6 years ago

Yes, initially we did:

either.of<ErrorA, number>(1)
  .mapLeft<ErrorA | ErrorB>(identity)
  .chain(() => left<ErrorB, number>(errB)

but then around the same time I opened this issue a while ago which @raveclassic commented on:

https://github.com/gcanti/fp-ts/issues/483#issuecomment-397244757

So I have added a few extras where I found them useful. For example natural transformations like TaskEither.toReaderTaskEither<E>() but also (I wish I could come up with a better name than this):

TaskEither<L, A>.widenLeft<M>() => TaskEither<L | M, A>;

which does the mapLeft dance behind the scenes

leemhenson commented 6 years ago

You could add a chainL or something that combined widenLeft and chain.

gcanti commented 6 years ago

Aren't any of you using TaskEither with rich error types?

IMO rich error types is the way to go and fp-ts should promote such a good practice.

Let's say we must implement the following program

fetch some items from the database
dump each item to the filesystem

So we have 2 subsystems each with specific errors

interface Item {
  id: string
  name: string
}

import { TaskEither } from 'fp-ts/lib/TaskEither'

type DatabaseError = 'ConnectionError' | 'NoSuchTable'

interface MonadDatabase {
  fetchItems: (limit: number) => TaskEither<DatabaseError, Item[]>
}

type FilesystemError = 'FileNotFound' | 'DiskIsFull'

interface MonadFilesystem {
  saveItem: (item: Item) => TaskEither<FilesystemError, void>
}

So the main program might raise the union of those errors

type ProgramError = DatabaseError | FilesystemError

const program = (M: MonadDatabase & MonadFilesystem): TaskEither<ProgramError, void> => {
  return M.fetchItems(10)
    .mapLeft<ProgramError>(identity)
    .chain(items => array.traverse(taskEither)(items, M.saveItem))
    .map(() => undefined)
}

If I comment the line containing .mapLeft<ProgramError>(identity) I get the following error

Type 'FilesystemError' is not assignable to type 'DatabaseError'

This is because M.fetchItems(10) has type TaskEither<DatabaseError, Item[]> while array.traverse(taskEither)(items, M.saveItem) has type TaskEither<FilesystemError, void[]>.

There are 2 consequences

a) if I'm mixing 2 subsystems accidentally this is nice (the type-checker helps me prevent such errors)
b) if I'm mixing 2 subsystems deliberately this is not so nice (I have to add some boilerplate using mapLeft)

What do we want to value the most? a) or b)?

If a) the the current implementation of chain is fine. If b) then we should do something.

leemhenson commented 6 years ago

If we made chain automatically do the mapLeft(identity) dance, then we end up with a richer L. And presumably somewhere higher up, you would be pattern matching on that L with assertNever or similar to prove totality. I don't think the worry about losing the knowledge of accidentally mixing subsystems is a big deal because your compiler error just happens somewhere else. You would still have one and be able to track that down.

raveclassic commented 6 years ago

@leemhenson This is not a general behavior for Chain. What about Type3<F, U, L, A> and higher? What exactly should be merged, U or L? On the other hand if it's possible (not sure) to update only concrete chain (Either, TaskEither etc) I think it would be ok.

leemhenson commented 6 years ago

Yeah I see your point. It seems natural to me that chain would always target L and A, as that's what happens in Either and TaskEither but maybe to other people it wouldn't be so natural. Maybe you could chain Type3s like

ReaderTaskEither<E, L, A>.chain<B, M = L, F extends E = E>(fb: (a: A) => ReaderTaskEither<F, M, B>) => ReaderTaskEither<F, L | M, B>

raveclassic commented 6 years ago

Well, changing to the following somehow doesn't break Chain2<URI> interface:

class Left<L, A> {
  chain<M, B>(f: (a: A) => Either<M, B>): Either<L | M, B> {
    return this as any
  }
}
class Right<L, A> {
  chain<M, B>(f: (a: A) => Either<M, B>): Either<L | M, B> {
    return f(this.value)
  }
}

// Chain2<URI> instance
export interface Chain2<F extends URIS2> extends Apply2<F> {
  readonly chain: <L, A, B>(fa: Type2<F, L, A>, f: (a: A) => Type2<F, L, B>) => Type2<F, L, B>
}
const chain = <L, M, A, B>(fa: Either<L, A>, f: (a: A) => Either<M, B>): Either<L | M, B> => {
  return fa.chain(f)
}
const either: Chain2<URI> = { chain }

const result = left(123).chain(() => left('foo')) // Either<string | number, {}> - handy
const result2 = either.chain(left(123), () => left('foo')) // error - as expected

But why?

sledorze commented 6 years ago

When we call chain on a Either<L, A> we're using the Either<L, ?> Monad so what is mixed would be the Ls and the Type param of the monad is replaced. So if you have three params, I think it could follow the same principle but this is gut feeling and I'm not sure of it, we need a Mathematicien! :)

On the using of richer types and possible subsystem accident; I think that you may protect yourself from diverging subsystems at the outer level.

(a) I do not see (yet) the real world impact of the downsides but there must be some. (b) Open to drastically more precise business errors handling and a whole new dimension into type safety.

I tend to think that we're all(?) trying to bend the existing library to fit that need and I'm positive we should try (but I'm a bit of a early adopter).

Do we have prior art evidence of the bad and ugly of it?

P.S.: (Related) I am also using this https://github.com/gcanti/fp-ts/issues/532 to lift a specific union member on the right (exclusively in that version) via a predicate.

sledorze commented 6 years ago

@raveclassic maybe it resolves L in the Chain2 definition to L | M in the chain one? (or something weakly typed is happening). Can't test ATM..

gcanti commented 6 years ago

I don't think the worry about losing the knowledge of accidentally mixing subsystems is a big deal because your compiler error just happens somewhere else

There's no guarantee though. Let me explain and also answer to

And presumably somewhere higher up, you would be pattern matching on that L with assertNever or similar to prove totality

This can be avoided: if E1, E2 are tagged unions then

u1) { type: 'E1', e1: E1 } | { type: 'E2', e2: E2 } is a tagged union (<= and in this case we certainly need mapLeft)
u2) if tagOf(E1) = tagOf(E2) then E1 | E2 might be a tagged union

Note that u1 is safe while u2 is unsafe: what if

interface ErrorSubsystem1 {
  tag: 'A'
  a: string
}

interface ErrorSubsystem2 {
  tag: 'A'
  a: string
}

type ErrorProgram = ErrorSubsystem1 | ErrorSubsystem2 // ops...

// the type-checker doesn't help
const f = (x: ErrorProgram): string => {
  switch (x.tag) {
    case 'A':
      return x.a
  }
}

Unless we find a way to statically prevent such situations, the best practice is always using ~u2~ EDIT: u1 (and mapLeft).

p.s. I know that this "mapLeft dance" is pretty annoying, and unofficially I feel your pain: doing type ErrorProgram = ErrorSubsystem1 | ErrorSubsystem2 is so handy... but fp-ts is grounded on type-safety first so officially I must resist and say no to shortcuts unless they are safe. (you are still free to add an overloading to chain if you really want to)

sledorze commented 6 years ago

Would that be safe with symbol tagged errors? (Understanding that would be too verbose to implement and that we would loose generality) I m thinking about maybe an alternative type.

peterhorne commented 6 years ago

In the case of u2 they are "the same" type due to the fact that Typescript is structurally typed. That doesn't mean that it's unsafe, does it?

sledorze commented 6 years ago

Actually the problem with u2 already exist with today s chain implementation.

I mean someone using the current chain api with a Left of ErrorSubsystem1 and chaining on a function returning a ErrorSubsystem2 will detect nothing due to the structural nature of TS like @peterhorne said.

So switching to that new Error consolidation scheme would not be a regression.

Is it correct @gcanti ?

leemhenson commented 6 years ago

I'll throw in my tuppence too. @gcanti I've probably missed some subtlety in your comment, but to me it reads like "what if the programmer writes a bug - the type system won't catch it". Which is all well and good, but I could break existing fp-ts code by writing a bug:

taskEither.of(1).chain(n => null as any)

The type system isn't going to save me here either. I dunno, i guess I feel like "if you write code that circumvents the type system then you're gonna have a bad day anyway".

sledorze commented 6 years ago

Is there any no other reasons why it would not be acceptable based on our shared goal of unconditional type safety preservation?

gcanti commented 6 years ago

@leemhenson in your example you are circumventing the type system deliberately, I'm more concerned by unwanted behaviours and I wonder what we can do (or not do) to prevent possible bugs. But...

"what if the programmer writes a bug - the type system won't catch it"

...you, @peterhorne and @sledorze are right, in a structural type system what I wrote above shouldn't be considered a bug or unsafe code but instead something that I actually wanted. Otherwise I should arrange the code towards something more nominally-flavoured, for example using symbols as suggested by @sledorze, or some type-level trick (<= an interesting topic by itself).

sledorze commented 6 years ago

Is it a Go for richer Error types then? There's also the consideration of the amount of code to change / maintain; how may we approach that / where to start? (I think trying on Either alone would get a first feeling..)

gcanti commented 6 years ago

@sledorze for me this was a general discussion (Free -> mtl -> error handling / union types), for what concerns specific changes (for example chain's signature) we need a proposal (in a new issue). It's not clear to me

is it a change "in the small" (i.e. just change the method signature)?
or in the "large" (i.e. change the corresponding type class)?
if it is in the large, how it interacts with other type classes and with higher kinded types like Type3?
is there prior art?
what about ap?
and why @raveclassic 's code is type checking here?

sledorze commented 6 years ago

@gcanti I think it is not actionnable yet, we need to discuss more. may the discussion stay here?

On the disjoint unions side:

How to differentiate between a 'colliding' union member and a same type in a union member? I think that's not the goal of what you're proposing (and it is really useful) but if one want to apply such disjoint union logic in a 'chain' then we face an issue with a notion of type 'equality' (which is again structural).

gcanti commented 6 years ago

FYI while porting "FP to the max" to TypeScript (https://twitter.com/GiulioCanti/status/1027435403329110016) I think I got a way to make more handy to write a program in a mtl / tagless-final style using the following declarations

interface Syntax<M extends URIS, A> {
  map: <B>(f: (a: A) => B) => HK<M, B>
  chain: <B>(f: (a: A) => HK<M, B>) => HK<M, B>
}

type HK<F extends URIS, A> = Type<F, A> & Syntax<F, A>

interface MonadSyntax<M extends URIS> {
  of: <A>(a: A) => HK<M, A>
}

Here's the "Scenario 1: using mtl / finally tagless" above rewritten with HK and MonadSyntax.

Note that I can use map and chain as methods

import { Either } from 'fp-ts/lib/Either'
import { Type, URIS } from 'fp-ts/lib/HKT'
import { none, Option, some } from 'fp-ts/lib/Option'

interface Syntax<M extends URIS, A> {
  map: <B>(f: (a: A) => B) => HK<M, B>
  chain: <B>(f: (a: A) => HK<M, B>) => HK<M, B>
}

type HK<F extends URIS, A> = Type<F, A> & Syntax<F, A>

interface MonadSyntax<M extends URIS> {
  of: <A>(a: A) => HK<M, A>
}

export interface User {
  id: string
  name: string
}

interface MonadStorage<M extends URIS> {
  getItem: (key: string) => HK<M, Option<string>>
}

interface MonadUser<M extends URIS> {
  fetch: (userId: string) => HK<M, Either<string, Option<User>>>
}

interface MonadLogger<M extends URIS> {
  log: (message: string) => HK<M, void>
}

export interface MonadApp<M extends URIS> extends MonadStorage<M>, MonadUser<M>, MonadLogger<M>, MonadSyntax<M> {}

export function main<M extends URIS>(M: MonadApp<M>): (userId: string) => HK<M, Option<User>> {
  const withLog = <A>(message: string, fa: HK<M, A>): HK<M, A> => M.log(message).chain(() => fa)
  const parseUser = (s: string): User => JSON.parse(s)
  return userId => {
    const noUser: HK<M, Option<User>> = M.of(none)
    const localUser = M.getItem(userId).chain(o =>
      o.fold(withLog('local user not found', noUser), s => M.of(some(parseUser(s))))
    )
    const remoteUser = M.fetch(userId).chain(e =>
      e.fold(
        () => withLog('error while retrieving user', noUser),
        o => o.fold(withLog('remote user not found', noUser), user => withLog('remote user found', M.of(some(user))))
      )
    )
    return localUser.chain(o => o.fold(remoteUser, user => M.of(some(user))))
  }
}

gcanti / fp-ts

On free monad, tagless-final and error handling #528

Scenario 1: using mtl / finally tagless

Scenario 2: using concrete types