Index arguments are mandatory, causing problems with typings

[stazz]

What version of Effect is running?

2.0.3

What steps can reproduce the bug?

Create this kind of code:

import * as S from "@effect/schema/Schema";
import * as F from "effect/Function";
import * as A from "effect/ReadonlyArray";

const isMyThing = F.pipe(S.string, S.is);

A.takeWhile([false, "this-will-pass"], isMyThing);

What is the expected behavior?

I expect no compilation error.

What do you see instead?

You will get a compilation error No overload matches this call. for 2nd argument of A.takeWhile.

Additional information

This problem stems from the fact that takeWhile and a bunch of other functions have mandatory index parameter added to them in #1850. However this conflicts with optional AST.ParseOptions accepted by function returned by S.is.

On the other hand, this error makes sense, since takeWhile passes number as 2nd parameter, and not AST.ParseOptions. While making sense technically, the actual real-life usage scenarios of combining is and e.g. takeWhile become cumbersome, as then there is a need to add one extra import and extra explicit typing:

import * as S from "@effect/schema/Schema";
import * as F from "effect/Function";
import * as A from "effect/ReadonlyArray";
import * as P from "effect/Predicate"; // Extra import

// Add explicit typing to isMyThing to make compiler error go away
// (still not nice as takeWhile invokes callback with number as 2nd argument)
const isMyThing: P.Refinement<unknown, string> = F.pipe(S.string, S.is);

A.takeWhile([false, "this-will-pass"], isMyThing);

Is it possible to change S.is to NOT have 2nd optional parameter, and then introduce S.isWithOptions which does? Or maybe S.isPlain which does NOT have 2nd optional parameter, and keep S.is as-is? Or maybe make all index number parameter for takeWhile and friends optional?

[mikearnaldi]

Tacit usage is never advised as it can lead to bugs, in the above code while TS is fine you actually have a bug, given that isMyThing is not actually a simple predicate.

The right way to use this is:

import * as S from "@effect/schema/Schema"
import * as F from "effect/Function"
import * as A from "effect/ReadonlyArray"

const isMyThing = F.pipe(S.string, S.is)

A.takeWhile([false, "this-will-pass"], (a) => isMyThing(a))

Note that generally doing whatever.map(f) is considered a bad practice across effect, we should always make the call explicit, like whatever.map(x => f(x)) this will also improve significantly stack traces when things go wrong

[stazz]

I see this issue has been already resolved, but I still just add one thing:

Note that generally doing whatever.map(f) is considered a bad practice across effect,

I see this point, and it is fine if you guys have such rule. However, I am not writing code for effect libraries, but writing code for my own things, and I don't see any problems with passing function directly instead of making custom callback for every simple invocation, considering how much verbosity (and thus cognitive overload) it adds to the code. Also, imposing such "bad/good practice" things on users of the library is perhaps a bit off-putting for many developers.

[fubhy]

I see this issue has been already resolved, but I still just add one thing:

Note that generally doing whatever.map(f) is considered a bad practice across effect,

I see this point, and it is fine if you guys have such rule. However, I am not writing code for effect libraries, but writing code for my own things, and I don't see any problems with passing function directly instead of making custom callback for every simple invocation, considering how much verbosity (and thus cognitive overload) it adds to the code. Also, imposing such "bad/good practice" things on users of the library is perhaps a bit off-putting for many developers.

This is generally not a good idea, not just for effect user code. It can introduce bugs all around due to implicitly passing dangling arguments to your callback function.

So in general, it is advisable to avoid tacit usage of function callbacks across api boundaries.

[stazz]

It can introduce bugs all around due to implicitly passing dangling arguments to your callback function.

Hmm, do you have any reference link to post or something with more information and specifics about this? 🤔

[mikearnaldi]

It can introduce bugs all around due to implicitly passing dangling arguments to your callback function.

Hmm, do you have any reference link to post or something with more information and specifics about this? 🤔

While you can decide to do whatever in your code if you are using effect you have to "follow" the effect recommendation when using effect functionality, ergo in this case don't use tacit passing otherwise you end up with bugs.

The issue with tacit usage is that for TS you can assign a function with an optional parameter to a function without such parameter, this is effectively an unsound decision made by the TS team, which we have to deal with. also it is valid to pass excess parameters, which means if you combine both that you will get the ability to pass params with invalid types (leading to undefined behaviour).

Also TS is weak in checking tacit usage, for example it can't properly discriminate overloads leading to "any" in generics that basically "turn off checking".