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

path methods typing #269

Open
wclr opened this issue Oct 5, 2017 · 17 comments
Open

path methods typing #269

wclr opened this issue Oct 5, 2017 · 17 comments
Labels

Comments

@wclr
Copy link
Contributor

wclr commented Oct 5, 2017

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
Copy link
Member

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

@wclr
Copy link
Contributor Author

wclr commented Oct 5, 2017

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
Copy link
Member

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

@ikatyang
Copy link
Member

ikatyang commented Oct 5, 2017

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

@wclr
Copy link
Contributor Author

wclr commented Oct 5, 2017

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
Copy link
Member

ikatyang commented Oct 5, 2017

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
Copy link
Member

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
Copy link
Member

ikatyang commented Oct 7, 2017

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...'.
*/

@ikatyang ikatyang added the bug label Oct 7, 2017
@clark0x
Copy link

clark0x commented Dec 26, 2017

image

@ikatyang
Copy link
Member

@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?

@KiaraGrouwstra
Copy link
Member

KiaraGrouwstra commented Dec 28, 2017

For posterity,

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

This is fixed since microsoft/TypeScript#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:

  • no erasure (edit: ?)
  • composition ($ComposeReverse, $Compose)
  • heterogeneous map ($TupleMap, $ObjMap)
  • type calls ($Call)

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
Copy link
Member

@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:

  • I've yet to figure out an equivalent for TS's in, which I used there to make object types from generic keys ({ [P in K]: 1 }, the type equivalent of dynamic JS keys like { [k]: 1 }).
  • Not sure how to properly test this yet. Try Flow 404s on a few JS assets, IDE plugins often just seem to hang for me (like for TS). I'm having trouble figuring out the result types -- there doesn't seem to be an equivalent to TS's .d.ts output files, nor to the TS Compiler API we used for type tests at typical.

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
Copy link
Member

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

@goodmind
Copy link

goodmind commented Feb 11, 2018

@tycho01

  • maybe $ObjMapi ?
  • equivalent to TS's .d.ts is .js.flow
  • there's indeed no TS Compiler API, babel can manipulate nodes but no typecheck (would be cool if Flow had OCaml compiler plugins)
  • I think you can use what https://github.com/facebook/flow using for tests (either tests or newtests)

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

@KiaraGrouwstra
Copy link
Member

replying at KiaraGrouwstra/typical#12 to keep this on-topic. thanks for the input ika.

@wclr
Copy link
Contributor Author

wclr commented Apr 16, 2018

There probably a solution for 2.8 worth looking microsoft/TypeScript#12290 (comment)

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

No branches or pull requests

5 participants