Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pipe typings not seeming to work.. #466

Closed
Revilotom opened this issue May 28, 2020 · 16 comments
Closed

Pipe typings not seeming to work.. #466

Revilotom opened this issue May 28, 2020 · 16 comments

Comments

@Revilotom
Copy link

Revilotom commented May 28, 2020

typescript: 3.9.3
rambda: 5.4.3

R.pipe<number[]>( shuffle, R.take(5), R.append(10), shuffle, )([0,1,2,3,4,6]) }

With the above code I get the following error:
Expected 1 arguments, but got 4

Please can someone point out to me what im doing wrong?

@farwayer
Copy link
Contributor

pipe and compose typings are broken 😔

@cinan
Copy link

cinan commented May 29, 2020

For now I have to use pipe types from version 4.5.0 (idk when the pipe got broken)

Why rambda cannot use types from ts-toolbelt as ramda does? Lots of types are broken or semi-broken - for example map when used in pipe with filter, or flatten.

@selfrefactor
Copy link
Owner

selfrefactor commented May 29, 2020

R.pipe and R.compose typings are taken directly from @types/ramda

R.map/R.filter typings issue is mostly due to the fact that the methods supports objects.

Generally I agree with the notion that Typescript definitions of Rambda is far from decent, especially when used with R.pipe/R.compose.

This is not the first time the issue is brought to my attention and every time I point to Proposal: Variadic Kinds -- Give specific types to variadic functions.

If you ask Ramda developers about Typescript, they most likely will decline discussion as they reached consensus that in the current state of Typescript, function library like Ramda is hard to be described. That is the reason why @types/ramda has no affiliation with Ramda.

I also was disappointed, when I realized that I cannot use properly R.compose with Typescript. I can say that forced me to change the way I write my Typescript/Javascript code. The problem is that to properly use R.compose, you have to write something like:

// pseudo code written on the fly
const result = R.compose<number[], string, number, string[]>(
  R.map(String),
  R.map(Number),
  R.map(String)
)([1,2,3,4]) 

Without <number[], string, number, string[]> part, Typescript has hard time to infer the type of the inner function inside R.compose.

I will look into any wrong typings of Ramda, as long as I am provided with example which proves that the problem is missing in @types/ramda

To do that, you need to edit any source/*-spec.ts file and do something like:

import * as R from 'ramda'
import {pipe, take, append} from 'rambda'

describe('bug', () => {
  it('R.pipe', () => {
   const result = pipe(take(5), append(10))([0,1,2,3,4,6])
   result // $ExpectType number[]
   
   const ramdaResult = R.pipe(R.take(5), R.append(10))([0,1,2,3,4,6])
   ramdaResult // $ExpectType number[]
  })
})

Now you need to run yarn typings to run dtslint test. If you are using VSCode(as I do), if there is something wrong, you will see it even before you run the test.

If indeed the issue is solely in Rambda typings, then result evaluation will be wrong and ramdaResult evaluation will be correct.

As for ts-toolbelt - Rambda use it, but it doesn't declare it as dependency(there was separate issue regarding that). If ts-toolbelt could solve R.compose/R.pipe issues, then we should have the solution in @types/ramda as I am sure that the developers supporting those typings has deeper knowledge in Typescript.

Actually I started using ts-toolbelt after they start using it in @types/ramda. I also follow each change in these typings as they are directly related to Rambda definitions. That from me for now and I will welcome further discussing what could be done to solve at least some of the problems.

@selfrefactor
Copy link
Owner

I just changed the typings of R.flip to match that of @types/ramda and the end result is not great:

import * as R from 'ramda'

describe('Ramda.flip', () => {
  it('function with arity of 2', () => {
    const subtractFlipped = R.flip(R.subtract)
    const result = subtractFlipped(1, 7)
    const curriedResult = subtractFlipped(1)(7)
    curriedResult // $ExpectType number
     
    // This is wrong
    // ============================================
    result // $ExpectType (y: number) => number
  })
})

As you can see, like it or not, there is definitely a price to pay when using Typescript with Ramda/Rambda.

@farwayer
Copy link
Contributor

When I wrote about broken pipe and compose typings I meant number of functions not about argument types. See https://github.com/DefinitelyTyped/DefinitelyTyped/blob/7df04df6a1a83df5c73bd4e35f824e09099dfeeb/types/ramda/index.d.ts#L1411

@selfrefactor
Copy link
Owner

selfrefactor commented May 30, 2020

@farwayer I understand it the other way, that's why I was a bit excessive in my explanation. Thank you for the clarification.

@falkenhawk
Copy link

falkenhawk commented Jun 15, 2020

@selfrefactor can at least rambda's types for compose & pipe be unified with those of ramda?
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/7df04df6a1a83df5c73bd4e35f824e09099dfeeb/types/ramda/index.d.ts#L292-L320

Currently rambda can't even handle more than 1 argument - there are only typings for fn0 variant:
https://github.com/selfrefactor/rambda/blob/master/index.d.ts#L239-L242

which makes these functions unusable with typescript...
variadic arguments is one thing, but let's at least have it statically typed to some reasonable number of function arguments, please

@selfrefactor
Copy link
Owner

selfrefactor commented Jun 15, 2020

@falkenhawk Thank you for bringing this up - it is error of the build step as Rambda.compose has all @types/rambda definitions in files/index.d.ts

@selfrefactor
Copy link
Owner

Should be fixed with 5.6.3

@falkenhawk
Copy link

falkenhawk commented Jun 16, 2020

@selfrefactor I still could not use compose/pipe without specifying generics manually...

compose(fn1, fn2)(v => v);

was causing

error TS2554: Expected 0 arguments, but got 1.

so I figured this workaroud...

import * as rambda from 'rambda';

// workaround for error TS2554: Expected 0 arguments, but got X.
// https://github.com/microsoft/TypeScript/issues/29904
// https://github.com/selfrefactor/rambda/issues/466
declare module 'rambda' {
  function compose<V extends any[], T1>(fn0: () => T1): (...args: V) => T1;
  function compose<V extends any[], T1, T2>(fn1: (x: T1) => T2, fn0: () => T1): (...args: V) => T2;
  function compose<V extends any[], T1, T2, T3>(fn2: (x: T2) => T3, fn1: (x: T1) => T2, fn0: () => T1): (...args: V) => T3;
  function compose<V extends any[], T1, T2, T3, T4>(fn3: (x: T3) => T4, fn2: (x: T2) => T3, fn1: (x: T1) => T2, fn0: () => T1): (...args: V) => T4;
  function compose<V extends any[], T1, T2, T3, T4, T5>(fn4: (x: T4) => T5, fn3: (x: T3) => T4, fn2: (x: T2) => T3, fn1: (x: T1) => T2, fn0: () => T1): (...args: V) => T5;
  function compose<V extends any[], T1, T2, T3, T4, T5, T6>(fn5: (x: T5) => T6, fn4: (x: T4) => T5, fn3: (x: T3) => T4, fn2: (x: T2) => T3, fn1: (x: T1) => T2, fn0: () => T1): (...args: V) => T6;

  function pipe<V extends any[], T1>(fn0: () => T1): (...args: V) => T1;
  function pipe<V extends any[], T1, T2>(fn0: () => T1, fn1: (x: T1) => T2): (...args: V) => T2;
  function pipe<V extends any[], T1, T2, T3>(fn0: () => T1, fn1: (x: T1) => T2, fn2: (x: T2) => T3): (...args: V) => T3;
  function pipe<V extends any[], T1, T2, T3, T4>(fn0: () => T1, fn1: (x: T1) => T2, fn2: (x: T2) => T3, fn3: (x: T3) => T4): (...args: V) => T4;
  function pipe<V extends any[], T1, T2, T3, T4, T5>(fn0: () => T1, fn1: (x: T1) => T2, fn2: (x: T2) => T3, fn3: (x: T3) => T4, fn4: (x: T4) => T5): (...args: V) => T5;
  function pipe<V extends any[], T1, T2, T3, T4, T5, T6>(fn0: () => T1, fn1: (x: T1) => T2, fn2: (x: T2) => T3, fn3: (x: T3) => T4, fn4: (x: T4) => T5, fn5: (x: T5) => T6): (...args: V) => T6;
  function pipe<V extends any[], T1, T2, T3, T4, T5, T6, T7>(fn0: () => T1, fn1: (x: T1) => T2, fn2: (x: T2) => T3, fn3: (x: T3) => T4, fn4: (x: T4) => T5, fn5: (x: T5) => T6, fn6: (x: T6) => T7): (...args: V) => T7;
  function pipe<V extends any[], T1, T2, T3, T4, T5, T6, T7, T8>(fn0: () => T1, fn1: (x: T1) => T2, fn2: (x: T2) => T3, fn3: (x: T3) => T4, fn4: (x: T4) => T5, fn5: (x: T5) => T6, fn6: (x: T6) => T7, fn7: (x: T7) => T8): (...args: V) => T8;
  function pipe<V extends any[], T1, T2, T3, T4, T5, T6, T7, T8, T9>(fn0: () => T1, fn1: (x: T1) => T2, fn2: (x: T2) => T3, fn3: (x: T3) => T4, fn4: (x: T4) => T5, fn5: (x: T5) => T6, fn6: (x: T6) => T7, fn7: (x: T7) => T8, fn8: (x: T8) => T9): (...args: V) => T9;
  function pipe<V extends any[], T1, T2, T3, T4, T5, T6, T7, T8, T9, T10>(fn0: () => T1, fn1: (x: T1) => T2, fn2: (x: T2) => T3, fn3: (x: T3) => T4, fn4: (x: T4) => T5, fn5: (x: T5) => T6, fn6: (x: T6) => T7, fn7: (x: T7) => T8, fn8: (x: T8) => T9, fn9: (x: T9) => T10): (...args: V) => T10;
}

@selfrefactor
Copy link
Owner

@falkenhawk Can you give me more detailed example than compose(fn1, fn2)(v => v); so I can use the power of dtslint to verify your suggestion?

@selfrefactor selfrefactor reopened this Jun 16, 2020
@falkenhawk
Copy link

@selfrefactor would this reproduction be enough? 🤔 https://codesandbox.io/s/relaxed-moore-9wf1n?file=/src/index.ts

@selfrefactor
Copy link
Owner

It is enough, but I don't see a problem that Typescript complains, as I believe that it should:

const result = compose(
  () => {},
  () => {} // here you declare that you don't expect anything coming from the pipe
)(() => {}); // but hen you pass something in it

this code works:

const result = compose(
      () => {},
      () => {}
 )()

I am open to accept your suggestion, but I will need a stronger use case than you initially suggested. Also, in the case of R.pipe/compose I am simply following @types/ramda definitions and I really need a good argumentation to brake this connection.

@falkenhawk
Copy link

falkenhawk commented Jun 16, 2020

Actually, it might be only our case where we use combine/pipe to wrap "enhancers" for graphql resolvers (a series of functions which wrap the original resolver e.g. checking auth, privileges before calling it, or modify a knex/objection.js querybuilder instance returned by that resolver before calling it)...

I may try to set up a reproduction, but it's quite convoluted in our real code with the different arguments, overloads and return types of those "resolver enhancers" of ours... and a simple example I came up quickly with works just fine: https://codesandbox.io/s/relaxed-moore-9wf1n?file=/src/index2.ts

It may also turn out that there is an issue on our side with typings or another typescript issue with e.g. handling union types, maybe something along the lines of microsoft/TypeScript#29904

@falkenhawk
Copy link

falkenhawk commented Jun 16, 2020

And thanks for the tip about mismatching "inputs", you are right I was passing something - what the individual functions did not expect - to the combined function and that's why the typescript rightfully complained in that simple example.

There is just a little something that triggers typescript to yell at me in my real code 🙈 and the "workaround" posted by me may have only accidentally "fixed" it by disabling some ts checks.

@selfrefactor
Copy link
Owner

selfrefactor commented Jun 16, 2020

Glad to be able to help. index2.ts file in codesandbox is empty(at least this is what I see).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants