fp-ts

fp-ts copied to clipboard

🐛 Bug report

Current Behavior

Expected behavior

Apply.sequenceS shouldn't crash when it encounters an empty struct.

Reproducible example

import * as T from 'fp-ts/Task'
import * as Apply from 'fp-ts/Apply'

const data: Record<string, T.Task<string>> = {};

Apply.sequenceS(T.Monad)(data)().then(console.log)

Suggested solution(s)

I'm unsure what is leading to this bug, so I am unsure how it could be fixed.

Additional context

No additional context.

Your environment

Software	Version(s)
fp-ts	2.10.5, 2.11.1
TypeScript	4.3.2

The problem is that the code compiles with a record as input. sequenceS is meant to be used with structures and the code would not compile with an empty structure.

@DenisFrezzato That's a fair point, I was stretching the use case a bit. However I still see this as a bug, as the code compiles but leads to a runtime error.

I agree, but I'm not sure there is a trick to prevent this.

How about in the sequenceS code it check Object.keys(record).length and if it's 0, then return Applicative.of({}) but otherwise do what it does now? That should fix the runtime error

It seems the internal check for non empty records does not work as expected

type EnforceNonEmptyRecord<R> = keyof R extends never ? never : R

If you put Record<string, any> in this type you get R back which is not intended. I have doubts that there is any reliable check for that at all. I don't think that removing that constraint and adding an empty record handling does break anything intended?

IMO we should deprecate sequenceS in favour of apS / apSW

@gcanti I tried to replace all sequenceS calls in a bigger codebase and there are two points I would like to address when deprecating sequenceS.

1. ~~We would need DoSeq instances otherwise bigger constructs would always be execute in parallel which could be a problem.~~ We can use the general apS function from Apply and provide a ApplicativeSeq instance by ourself as we did with sequenceS.
2. sequenceS is especially useful when you simple want to reuse existing names without redefining them in apS and apSW calls e. g.

with sequenceS

sequenceS(O.Applicative)({
  draftHeader,
  draftSections,
  draftTasks,
  me,
  projectName,
  template,
  genderTranslations,
  taskStatusTranslations
}),

with apS

O.Do,
O.apS('draftHeader', draftHeader),
O.apS('draftSections', draftSections),
O.apS('draftTasks', draftTasks),
O.apS('me', me),
O.apS('projectName', projectName),
O.apS('template', template),
O.apS('genderTranslations', genderTranslations),
O.apS('taskStatusTranslations', taskStatusTranslations),

But beside of that it seems we should fix the underlying problem beside from deprecating sequenceS.

This can actually not be fixed without moving/duplicating sequenceS to Applicative else we are not able to return an empty instance via F.of({}). With the deprecating in mind I don't know if this is a worth pursuing solution.

Just ran into this today trying to do sequenceS on a record created from a reduced array. Something like this:

type Thing = {
  id: string;
  name: string;
};

const log = <T>(thing: T): R.Reader<{ logger: Logger }, T> => ({logger}) => {
  logger.debug(thing);
  return thing
};

const whatever = (things: Thing[]): R.Reader<{ logger: Logger }, Record<Thing['id'], Thing['name']>> =>
  pipe(
    things.reduce<Record<Thing['id'], R.Reader<{ logger: Logger }, Thing['name']>>>((acc, thing) => ({
      ...acc,
      [thing.id]: pipe(
        thing,
        log,
        R.map((thing) => thing.name)
      )
    }), {}),
    sequenceS(R.Apply)
  )

If sequenceS is not meant to be used on empty records, does it also mean it's not meant to be used on dynamically-sized records? If so, what does one do for a dynamically-sized records where each value is a monad?

as a workaround, I just check that the record is not empty before applying sequenceS

does it also mean it's not meant to be used on dynamically-sized records?

Correct.

what does one do for a dynamically-sized records where each value is a monad?

Take a look at traverse and traverseWithIndex from Record module.