Add some and every?

[Brodzko]

Doing a bit of refactoring, going through the usages of lodash in our codebase and replacing them with remeda alternatives, I noticed there is no some and every alternatives for arrays. Is this intentional? I guess you could accomplish the same with

const every =
  <T>(fn: (val: T) => boolean) =>
  (arr: T[]) =>
    R.pipe(
      arr,
      R.map(x => () => fn(x)),
      R.allPass
    )(1); // this feels _very_ weird, but the value doesn't matter

but that seems pretty clunky. Is the omitting of some and every intentional? If you're interested I'm happy to open a PR.

Edit: mistake in implementation

[cjquines]

i think so. for example, i believe the only reason we have map and filter is because they're useful in pipes and have better than default types. but some and every aren't as useful in the middle of pipes, because typically you want to R.pipe(...).some(...) and R.pipe(...).every(...). the other case i would think of where every would be useful is something like

const data: Array<string | number> = ["a", "b"];

if (data.every(R.isString)) {
  data; // Array<string>
}

but Typescript already does this.

[Brodzko]

Right! I guess I can kind of see them being useful in control flow, for example your latter case, instead of having to write if statements, I could have a functional form like

const data: Array<string | number> = ["a", "b"];

const result = R.pipe(
  data
  R.conditional(
    [R.every(R.isString), doStringStuff],
    R.conditional.defaultCase(doLooseStuff)
  )
);

[cjquines]

hm... i'd defer to the judgment of @eranhirsch here, because i don't have a clear vision myself of what the library should be. it's certainly changed over time; compare #54 to the set of functions we have now. i do think some and every are nice, composable "building blocks", and it does make the code easier to read in the example you gave, compared to the alternative...

[eranhirsch]

I support adding these functions as it would allow us to offer a dataLast implementation, which could become lazy in the future(!) (after we fix some stuff with our lazy implementation). We can also provide some (?) type narrowing to the input:

Widening example:

const data = [] as number[];
if (some(data, isString)) {
  console.log(data);
  //          ^? (number | string)[]
}

or narrowing

const data = [] as (number | string)[]
if (!some(data, isString)) {
  console.log(data);
  //          ^? number[];
}

(but we probably can't have both).

Back to you @cjquines, wdyt?

[Brodzko]

@eranhirsch hmm would this not be achievable (both) if we also had logic functions? Think not, and, or...

[eranhirsch]

The question is do we limit the narrowed type to extends T or not. When we do then our types protect users from non-sensical checks where the types don't overlap (e.g. checking isString on an array of numbers). When we don't then types could be expanded, but you lose type-safety when your code changes and an upstream dependency changes the type of your array.

Checkout recent PRs trying to make omitBy, pickBy, and isIncludedIn narrow correctly without breaking typing. In general typescript isn't good at conditional narrowing.

[cjquines]

(but we probably can't have both)

!some(condition) is the same as every(!condition), and i think in this case we can get the type we want. in the same way that !pickBy(condition) doesn't narrow well, but omitBy(!condition) does

i'm supportive of this—i think we should fix our lazy implementations and then make this lazy as well, so i'll add the v2 tag to this.

thanks for all the suggestions @Brodzko !

[Brodzko]

Can I ask for some pointers towards what the problems are with lazy implementation? 🙏

[eranhirsch]

...so i'll add the v2 tag to this.

There's no reason not to add it to v1 and improve it after v2 is released. As long as we get the types right, the changes to the laziness won't be breaking API changes so could be released as a minor or even a patch version.

[cjquines]

Can I ask for some pointers towards what the problems are with lazy implementation? 🙏

see the discussion in #74

in any case, i think it's possible to implement this with the current lazy infrastructure

[eranhirsch]

in any case, i think it's possible to implement this with the current lazy infrastructure

We can't because you'd need to push false (in the case of some) or true (in the case of every) after the pipe is complete; otherwise, you'd return undefined in those cases.

[Brodzko]

So I've had a go at every (see #631) and while doing that I have encountered a somewhat interesting issue. I believe getting the type for every right essentially boils down to being able to create a data-last "derived" type guard, i.e. given a type guard on T, we want to output a new type guard on Array<T>.

For simplicity though, let's just assume an "identity" transform on a type guard:

const wrapGuard = <T, S extends T>(predicate: (input: T) => input is S) => (input: T): input is S => predicate(input);

What's interesting to me, that when using this for example in conditional, this does not work correctly:

const data2 = R.pipe(
    test,
    R.conditional(
        [wrapGuard(R.isNumber), x => {
            expectTypeOf(x).toEqualTypeOf<number>() // `x` actually not narrowed
        }]
    )
)

while simply saving the value of the resulting type guard in a separate variable does:

const wrappedIsNumber = wrapGuard(R.isNumber);

const data3 = R.pipe(
    test,
    R.conditional(
        [wrappedIsNumber, x => {
            expectTypeOf(x).toEqualTypeOf<number>() // `x` correctly narrowed
        }]
    )
)

Here is a TS playground demonstrating this.

Note that I have intentionally used a trivial version of isNumber to not get confused by the resulting extra generics. Yet, I don't necessarily see the reason why TS seems to think the two functions are different.

I am suspecting that in the former case, type inference "fails" inside Case["when"] and it assumes it returns a simple boolean instead of a type assertion?

Am I missing something important about how data-last is/should be implemented or is this coming from somewhere deeper in Typescript itself? 🤔

Cc. @eranhirsch @cjquines

[cjquines]

this is likely a deeper issue with typescript generic inference; i would say don't worry about the R.conditional case for now

[eranhirsch]

This is related to what I wrote about in the long comment in the other PR (#635 (comment)). Typescript would infer "forward" when it encounters a generic used as an operand, meaning that when inside conditional it would first type wrapGuards generics regardless of the typing of the operand used inside wrapGuard, and if that doesn't cause a type-error, it would continue "forward" by narrowing the operand of wrapGuard.

You can see this by hovering over the conditional invocation and seeing how the generics are filled in. I've shortened it for clarity but you can see the difference here:

// when wrapGuard is used directly
unknown, void, never[...], string | number, unknown[...]
// when wrapGuard is pulled out
unknown, void, never[...], number, unknown[...]

You can see that typescript tried to type wrapGuard's T with the typing information it had available: string | number, and because string | number extends string | number it used it for S too. Then, when it checked that isNumber still matches the expected type it was OK because (x: unknown) => x is number extends (x: unknown) => x is string | number.

When you pulled out wrapGuard it didn't have any "forward" information so it used the operand itself to type "backwards". Once that type is set, you are removing a level of "generalization" from the type, so typescript has to chose narrower typing in conditional

I don't know typescript internals that much so I might be wrong about this, but this is the gist of it, you can see similar conclusions when we tried to type tap in the PR that added it (#409).

[Brodzko]

Thank you for the clarification, I suspected as much. Do you think this is something potentially addressable by the Typescript team or is this the way it has to work? I would intuitively kind of expect that in these cases, the types get inferred "bottom-up" 🤔

[eranhirsch]

I think this is the reason typescript added the const T generic constraint and the NoInfer utility; but both might not be a good fit for building utility libraries which have a broader set of concerns then a regular day-to-day project.