typed-typings / npm-ramda

TypeScript's type definitions for Ramda
MIT License
384 stars 64 forks source link

`path` methods typing #269

Open wclr opened 6 years ago

wclr commented 6 years ago

So what about path (assocPath, etc) methods typings?

Maybe type them for most common cases like string props, and up to 3-4 path length?

KiaraGrouwstra commented 6 years ago

I was just about to link to the thread at TS, then realized you were the OP there. :)

wclr commented 6 years ago

But if I'm not mistaken it works well for basic cases [string, string, string], the only problem is just with the size of signature needed, no? As I said just add common case for 3-4 length string tupple.

KiaraGrouwstra commented 6 years ago

Hm, @ikatyang said those were still in... weird.

ikatyang commented 6 years ago

I didn't touch the path typings, they should still remain the same ( source -> dist ).

wclr commented 6 years ago

Hm I thought path could be typed in basic cases, but for now I see I doen't even work for basic things) Is something.

image

ikatyang commented 6 years ago

We do not support the 1-length tuple version since it's a special case that tuples with any length can be all considered 1-length tuple, it's the super type of any tuple.


@tycho01, It seems we did not support path with excess property originally?

R.path(['a', 'c'], { a: { c: 1 } }); //=> number
R.path(['a', 'c'], { a: { c: 1 }, b: 2 }); //=> {}
KiaraGrouwstra commented 6 years ago

We do not support the 1-length tuple version since it's a special case that tuples with any length can be all considered 1-length tuple, it's the super type of any tuple.

Uhh. afaik there are no special cases, it sees length 2 tuples as super types of length 3+ tuples too. That could be addressed by just putting higher length options first I guess.

It seems we did not support path with excess property originally?

Hm... if it messed up on that I imagine it could be addressed by ensuring we'd match using objects containing a string index as well, e.g. { k: V } & { [K: string]: any }.

ikatyang commented 6 years ago

The & { [X: string]: any } thing seems not working:

declare function path2<T1 extends string, T2 extends string, TResult>(path: [T1, T2], obj: {
    [K1 in T1]: {
        [K2 in T2]: TResult;
    };
}): TResult;

declare function path2withAny<T1 extends string, T2 extends string, TResult>(path: [T1, T2], obj: {
    [K1 in T1]: {
        [K2 in T2]: TResult;
    } & {
        [X: string]: any;
    };
} & {
    [X: string]: any;
}): TResult;

path2(["a", "b"], { a: { b: 1 }}); //=> 1
path2(["a", "b"], { a: { b: 1 }, c: 2 });
/*                ^^^^^^^^^^^^^^^^^^^^^
[ts]
Argument of type '{ a: { b: number; }; c: number; }' is not assignable to parameter of type '{ a: { b: ((radix?: number) => string) | (() => number) | ((locales?: string | string[], options?...'.
  Types of property 'a' are incompatible.
    Type '{ b: number; }' is not assignable to type '{ b: ((radix?: number) => string) | (() => number) | ((locales?: string | string[], options?: Num...'.
      Types of property 'b' are incompatible.
        Type 'number' is not assignable to type '((radix?: number) => string) | (() => number) | ((locales?: string | string[], options?: NumberFo...'.
*/
path2withAny(["a", "b"], { a: { b: 1 }}); //=> 1
path2withAny(["a", "b"], { a: { b: 1 }, c: 2 });
/*                       ^^^^^^^^^^^^^^^^^^^^^
[ts]
Argument of type '{ a: { b: number; }; c: number; }' is not assignable to parameter of type '{ a: { b: ((radix?: number) => string) | (() => number) | ((locales?: string | string[], options?...'.
  Type '{ a: { b: number; }; c: number; }' is not assignable to type '{ a: { b: ((radix?: number) => string) | (() => number) | ((locales?: string | string[], options?...'.
    Types of property 'a' are incompatible.
      Type '{ b: number; }' is not assignable to type '{ b: ((radix?: number) => string) | (() => number) | ((locales?: string | string[], options?: Num...'.
        Type '{ b: number; }' is not assignable to type '{ b: ((radix?: number) => string) | (() => number) | ((locales?: string | string[], options?: Num...'.
          Types of property 'b' are incompatible.
            Type 'number' is not assignable to type '((radix?: number) => string) | (() => number) | ((locales?: string | string[], options?: NumberFo...'.
*/
clark0x commented 6 years ago

image

ikatyang commented 6 years ago

@clarkorz It seems you're not using our type:

image

If you're using our type, can you open another issue with steps to reproduce?

goodmind commented 6 years ago

@tycho01 what you think about this?

https://flow.org/try/#0CYUwxgNghgTiAEAzArgOzAFwJYHtX1BAAcBlEARwB4AFAPgAoAoeeAOnaKgwAsAueaowCU-ACQAVZEQggAslCI0ANPEriV1GDiIMiWov03ah8ALy144sxdEBRGQFsQqDOICeREGo37atRsws8IygkLAIAG6w8ERYnmIAwjgORDgAziAASiARIDAZAWB4aRjwGLAA5iAY-ADk5TBVGLVm8PWV1bWFxaUgAB5QKTKtAN6BiDg4-GNB8ABGsNOBs1EQyCBLs7OoICUgwPwNTUrLQQC+pxcsF1dBjEWoJfB7RACMrbGe9OyshKQUTBYtQmOFqJyBCxgYMCtVW6y6QiE9H6g2kICE3UepReACYPnEQN92H8yORAW0dntgAikS9XhjGABueh06bwSkYfZ1I6deBnBkAegAVMwhfA7H1PJhbDAtDBRc8MDAsKgKvAIFhOTAoBB4AADHkYPWsBXibhYNJlDwIC3wFVFFJcLBzYYAd013HZyAcczyooFTJZnKIOP4qG9vpgGKAA https://gist.github.com/zerobias/6e49651079d23f173f95ecc49b6a6ee1

KiaraGrouwstra commented 6 years ago

For posterity,

the 1-length tuple [is] the super type of any tuple

This is fixed since https://github.com/Microsoft/TypeScript/pull/17765.

@goodmind:

So, I haven't paid attention to Flow in a while, notably because the timing I entered front-end engineering got me on the Angular / TS path. But honestly, that looks great.

What I'm getting from this example, and a subsequent glimpse at their docs:

These are the exact game-changers essential for FP I've been clamoring for in TS for ages, and I suppose it only makes sense FB's team behind React and Immutable.js would ensure to get them in.

I guess this goes to show competition is great. I'd seriously consider retrying Flow if I were to try front-end again. Being able to use FP abstractions without having to fight the type system just might make it enjoyable again. :)

KiaraGrouwstra commented 6 years ago

@goodmind: curious to figure out how Flow would fare, I tried a bit to see if I could port my type lib to it in a new branch, WIP. Thoughts:

Guess I'm still pretty new to it, so not quite sure where to look or who to ask yet.

@ikatyang your Jest testing method wasn't taken from a Flow equivalent either, right?

ikatyang commented 6 years ago

I'm not sure what do you mean the testing method, but I didn't use Flow before so it shouldn't be.

goodmind commented 6 years ago

@tycho01

Also I think you can't just convert directly all this things

KiaraGrouwstra commented 6 years ago

replying at https://github.com/tycho01/typical/pull/12 to keep this on-topic. thanks for the input ika.

wclr commented 6 years ago

There probably a solution for 2.8 worth looking https://github.com/Microsoft/TypeScript/issues/12290#issuecomment-381525259