Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

Typings for recompose #231

Open
pablobirukov opened this issue Aug 16, 2016 · 30 comments
Open

Typings for recompose #231

pablobirukov opened this issue Aug 16, 2016 · 30 comments

Comments

@pablobirukov
Copy link

pablobirukov commented Aug 16, 2016

Is there any movements regarding typescript annotations?
I've started to annotate methods that I need considering react-redux's HOC factory (connect())

import { ComponentClass, StatelessComponent } from 'react';
interface ComponentDecorator<TOriginalProps, TOwnProps> {
  (component: ComponentClass<TOriginalProps>|StatelessComponent<TOriginalProps>): ComponentClass<TOwnProps>;
}
export interface InferableComponentDecorator {
  <P, TComponentConstruct extends (ComponentClass<P>|StatelessComponent<P>)>(component: TComponentConstruct): TComponentConstruct;
}

E.g. withContext()

export function withContext<ContextProps, ComponentOwnProps>(
  childContextTypes: React.ValidationMap<ContextProps>,
  getChildContext: (props: ComponentOwnProps) => any
): InferableComponentDecorator;

And having such components

interface StatelessCmpProps {
    bar: string;
}
export const StatelessCmp: React.StatelessComponent<StatelessCmpProps> = (props: StatelessCmpProps): JSX.Element => {
    return <div>{props.bar}</div>;
};
interface CmpProps {
    nProp: number;
    sProp: string;
}
export class Cmp extends React.Component<CmpProps, void> {
    public render(): JSX.Element {
        return <div>{this.props.nProp}</div>;
    }
}

We can use it as follows

const withContextFactory = withContext(
    { fooFn: React.PropTypes.object },
    () => ({ fooFn: () => {} })
);

const StatelessCmpContainsContext = withContextFactory(StatelessCmp);
// <StatelessCmpContainsContext /> TSC throws: Property 'bar' is missing in type ...
<StatelessCmpContainsContext bar="baz" /> // Okay

In other methods (like getContext) this solution doesn't work that good so I have to be more verbose on typedefs, for example having annotation

export function getContext<TOriginalProps, TContextProps>(
        contextTypes: ValidationMap<TContextProps>
    ): ComponentDecorator<TOriginalProps, TOriginalProps & TContextProps>;

TSC can't infer types correctly and thus (Cmp is a regular React.ComponentClass<CmpProps>)

const CmpWithContext = getContext({ propFromContext: React.PropTypes.string })(Cmp);
<CmpWithContext />

won't blame you so I have to specify types explicitly

interface PropsFromContext {
    propFromContext: string;
}
// and either
const CmpWithContext
  = getContext({ propFromContext: React.PropTypes.string })(Cmp) as React.ComponentClass<CmpProps & PropsFromContext>;
// or
const CmpWithContext
  = getContext<CmpProps, PropsFromContext>({ propFromContext: React.PropTypes.string })(Cmp);

So I'm wondering if someone have already started (or finished) to do the same. Or maybe someone knows better way to annotate HOCs and HOC factories.

@ianks
Copy link

ianks commented Aug 27, 2016

@R00GER I was just thinking the same thing. In order for this library to be of any use to me I need some high quality typings for it. How much have you done on this? Are you interested in working together to get these finished?

@pablobirukov
Copy link
Author

Hello @ianks. Thanks for the attention
You can take a look at what I've done here https://github.com/evolution-gaming/typed-recompose. I'm on mobile, will answer tomorrow in more details

@ianks
Copy link

ianks commented Aug 29, 2016

@R00GER, the typings looks really good. do you plan on contributign them to definitely typed?

@pablobirukov
Copy link
Author

@ianks, yes, I'm interested to proceed with the typings and contribute them to typings' registry. I wanted to hear some feedback but so far I've got the only response from you.
I've updated readme.md file at repo to make it more newcomers-friendly.
I'll cover few more methods and add more tests for other methods in few days. If you'd like to, then you are welcome to help with other methods and tests

@MrArnoldPalmer
Copy link

@R00GER Thank you so much for these. We have been trying to type HoCs for a couple of days but being new to TS has proved it challenging. This helped us a lot as we are using recompose throughout a large project. I will make sure to provide feedback as it comes.

@farism
Copy link
Contributor

farism commented Sep 15, 2016

Thanks for providing a starting point! I just got into TypeScript a couple days ago and recompose was one of the first libraries that I ran into trouble with due to no typings. I decided to write a full definition out with specs for DefinitelyTyped, as a way to get more familiar with TS.

Would appreciate your guys feedback/whatever contributions you may have before I submit the PR.

@R00GER @ianks @MrArnoldPalmer @acdlite

DefinitelyTyped/DefinitelyTyped@master...farism:master

@ianks
Copy link

ianks commented Sep 15, 2016

@farism beforeI comment on the code, I think we should def try to attempt to unify the code bases for both you and @R00GER, so we can combine our efforts and excel :)

@farism
Copy link
Contributor

farism commented Sep 15, 2016

@ianks That would be ideal! I had started for fun before finding that @R00GER had already wrote this, but some ideas were taken from him (like InferableComponentDecorator from react-redux).

I am also not sure if everything is implemented 100% correct. I am very very new to TS, but I tried to avoid any hack wherever possible. I am also not sure how detailed the specs need to be. DefinitelyTyped is stating in the contribution guide that if you use something like code from the library specs, and it compiles, it should be fine.

However, @R00GER, I noticed your repository has much more detailed specs, testing prop intersection/union types etc. Do you think this is necessary for iteration 1? And some of the definitions were crossed out in the readme, but I did not see them in the remote repository.

@farism
Copy link
Contributor

farism commented Sep 15, 2016

I also just saw #241 opened two days ago, things are moving fast! Maybe we can get @gcanti in on this discussion?

@pablobirukov
Copy link
Author

pablobirukov commented Sep 15, 2016

@farism

Do you think this is necessary for iteration 1?

I would say that process of making types stricter is not acceptable. Once you will release next version with stricter typings/better inferring you will see type mismatches. I think everyone can admit that it's terrible feeling when your app is broken because of vendors. That's why I stopped at certain point and asked community for feedback. I think that typings are to be good (tested, infers as much as possible) from the very fist version

@iskandersierra
Copy link

Hi
First of all I’m loving this package a lot, and using it on almost every react component I develop. So a couple of months ago I stumbled upon the @R00GER ‘s version of typings and decided to give it a go. I had some problems with missing functions that @acdlite had incorporated afterwards.

So I decided to develop an almost from scratch version of typings, based initially on @R00GER ‘s. After trying with a smaller package (change-emitter from @acdlite as well) I finally got recompose to DefinitellyTyped. It would be nice that all of you take a look at it and help me reach an excelent typings of this great package for the community to use it.

Recompose typings
Change-emitter typings

@morlay
Copy link

morlay commented Dec 15, 2016

getContext may be this, like withProps.

function getContext<TContext, TProps>(contextTypes: ValidationMap<TContext>): ComponentEnhancer<TContext & TProps, TProps>

https://github.com/acdlite/recompose/blob/master/src/packages/recompose/getContext.js#L6

@arolson101
Copy link

@iskandersierra thanks for creating them!
I see a bug with branch- the falseEnhancer should be optional

@threehams
Copy link

threehams commented Jun 10, 2017

RxJS is deciding its next API - likely based around a compose/pipe structure - which has triggered a lot of movement on Typescript to finally support this style without explicit types on every argument. This should be extremely helpful in typing recompose.

Latest issue:
microsoft/TypeScript#9366

The last month of design notes was almost completely dedicated to this issue:
https://github.com/Microsoft/TypeScript/issues?utf8=%E2%9C%93&q=label%3A%22Design%20Notes%22%20
(5/19 - 6/9)

Some people have gotten extremely close to getting literal type subtraction without any syntax changes:
microsoft/TypeScript#12215 (comment)

This feels like it's getting really close.

@istarkov
Copy link
Contributor

istarkov commented Jul 18, 2017

May be https://github.com/acdlite/recompose/tree/master/types can help to create typings for typescript

@jzimmek
Copy link

jzimmek commented Sep 28, 2017

I used recompose in a couple of project - in the current one we wanna migrate to typescript.

We came up with the following:

const App = (props: { name: string; age: number }) => {
  return (
    <div>
      name: {props.name}, age: {props.age}
    </div>
  );
};

const MyApp = compose<{ name: string; age: number }, {}>(
  withProps<{ name: string }, {}>(() => ({ name: "joe" })),
  withProps<{ age: number }, {}>(props => ({ age: 99 }))
)(App);

const x = renderToString(<MyApp />);

console.info("===>", x);

This works as expected and outputs:

===> <div data-reactroot="">name: <!-- -->joe<!-- -->, age: <!-- -->99</div>

But the following surprise me by not raising any error:

...
const MyApp = compose<{ name: string; age: number }, {}>(
  withProps<{ name: string }, {}>(() => ({ name: "joe" })),
//  withProps<{ age: number }, {}>(props => ({ age: 99 }))
)(App);

const x = renderToString(<MyApp />);

I would expect the typescript compiler to fail because the "age" property is not provided anymore to the App component. But it compiles just fine.

Is this supposed to work or am I doing something completly wrong?

Versions:
@ types/react: "^16.0.7",
@ types/react-dom: "^15.5.5",
@ types/recompose: "^0.24.2",
"react": "^16.0.0",
"react-dom": "^16.0.0",
"recompose": "^0.25.1"

@arolson101
Copy link

arolson101 commented Sep 29, 2017 via email

@jzimmek
Copy link

jzimmek commented Sep 29, 2017

@arolson101 thanks for your answer. unfortunately it does not work either, nor any other TInner/TOutter variation.

Switching TInner/TOutter fails with:

bildschirmfoto 2017-09-29 um 12 25 04

Setting TInner/TOutter also fails:

bildschirmfoto 2017-09-29 um 12 26 28

Setting TInner/TOutter compiles if I explicitly set name/age on but is obviously not what I want to do.

bildschirmfoto 2017-09-29 um 12 28 27

Almost everything else in typescript/recompose works as expected immediately.

Being able to see "missing props of a react component" was one of the reason we even considered to migrate to typescript - loosing this feature would be a shame. But I have to admit that I am relatively new to typescript, so I still hope that I am just doing something wrong here.

Any help on this is really appreciated.

Please let me know if I should create separate issue.

@arolson101
Copy link

Change your App definition to

const App = (props: { name: string; age: number } & React.Props<any>) => {
  return (
    <div>
      name: {props.name}, age: {props.age}
    </div>
  )
}

@mgutz
Copy link

mgutz commented Oct 3, 2017

@jzimmek

withProps provides the props so make them optional with Partial

export interface WithProps {
  name: string
  age: number
}

const App = (props: Partial<WithProps>) => {
  return <div>{props.name}</div>
}

withProps(() => ({name: 'joe'}))(App)

@jasonkuhrt
Copy link

I think this is relevant to the conversation as it surveys how far we can get with HOCs in TS + React: https://medium.com/@jrwebdev/react-higher-order-component-patterns-in-typescript-42278f7590fb

@squidfunk
Copy link

squidfunk commented Aug 1, 2018

I'am a big fan of recompose but I think the compose function is very weakly typed. It just expects functions as a variadic argument, there's no type safety between the HOCs that compose is composing, e.g.:

export const withImagesData = (loading?: React.SFC) =>
  compose<WithImagesData, {}>(
    graphql<{}, ImagesQuery, ImagesQueryArgs>(GetImages),
    ...(loading
      ? [
          branch<ImageQueryProps>(
            props => props.data.loading,
            renderComponent(loading)
          )
        ]
      : []
    ),
    withProps<WithImagesData, ImageQueryProps>(props => ({
      images: props.data.images || []
    }))
  )

All HOCs need to be typed explicitly - this is a huge burden and margin for error because I have to make sure that types match. Would it be possible to infer input/output typings from HOCs so that typings for HOCs can be omitted (as much as possible) and we just set the expected inner and outer type on compose?

@threehams
Copy link

compose has been a problem for a long time, and was only possible at all late last year. Neither Flow nor Typescript support variadic types yet, so you need a series of overloads. Flow supports compose... mostly. Sometimes its inference fails and you get unreadable errors.

RxJS seems to have come up with a good solution, at least for pipe... but it relies on all operators taking in one argument. This might need to be exponentially longer to support every signature for every HOC in recompose.

interface UnaryFunction<T, R> {
    (source: T): R;
}

export declare function pipe<T>(): UnaryFunction<T, T>;
export declare function pipe<T, A>(op1: UnaryFunction<T, A>): UnaryFunction<T, A>;
export declare function pipe<T, A, B>(op1: UnaryFunction<T, A>, op2: UnaryFunction<A, B>): UnaryFunction<T, B>;
export declare function pipe<T, A, B, C>(op1: UnaryFunction<T, A>, op2: UnaryFunction<A, B>, op3: UnaryFunction<B, C>): UnaryFunction<T, C>;
export declare function pipe<T, A, B, C, D>(op1: UnaryFunction<T, A>, op2: UnaryFunction<A, B>, op3: UnaryFunction<B, C>, op4: UnaryFunction<C, D>): UnaryFunction<T, D>;
export declare function pipe<T, A, B, C, D, E>(op1: UnaryFunction<T, A>, op2: UnaryFunction<A, B>, op3: UnaryFunction<B, C>, op4: UnaryFunction<C, D>, op5: UnaryFunction<D, E>): UnaryFunction<T, E>;
export declare function pipe<T, A, B, C, D, E, F>(op1: UnaryFunction<T, A>, op2: UnaryFunction<A, B>, op3: UnaryFunction<B, C>, op4: UnaryFunction<C, D>, op5: UnaryFunction<D, E>, op6: UnaryFunction<E, F>): UnaryFunction<T, F>;
export declare function pipe<T, A, B, C, D, E, F, G>(op1: UnaryFunction<T, A>, op2: UnaryFunction<A, B>, op3: UnaryFunction<B, C>, op4: UnaryFunction<C, D>, op5: UnaryFunction<D, E>, op6: UnaryFunction<E, F>, op7: UnaryFunction<F, G>): UnaryFunction<T, G>;
export declare function pipe<T, A, B, C, D, E, F, G, H>(op1: UnaryFunction<T, A>, op2: UnaryFunction<A, B>, op3: UnaryFunction<B, C>, op4: UnaryFunction<C, D>, op5: UnaryFunction<D, E>, op6: UnaryFunction<E, F>, op7: UnaryFunction<F, G>, op8: UnaryFunction<G, H>): UnaryFunction<T, H>;
export declare function pipe<T, A, B, C, D, E, F, G, H, I>(op1: UnaryFunction<T, A>, op2: UnaryFunction<A, B>, op3: UnaryFunction<B, C>, op4: UnaryFunction<C, D>, op5: UnaryFunction<D, E>, op6: UnaryFunction<E, F>, op7: UnaryFunction<F, G>, op8: UnaryFunction<G, H>, op9: UnaryFunction<H, I>): UnaryFunction<T, I>;
export declare function pipeFromArray<T, R>(fns: Array<UnaryFunction<T, R>>): UnaryFunction<T, R>;

There are open tickets in Typescript for variadic types (to avoid the overloads) and better generic inference. It's a hard thing to solve.

@arolson101
Copy link

FWIW the compose in redux has pretty good typing

@threehams
Copy link

Looks like they're doing about the same thing:
https://github.com/reduxjs/redux/blob/master/index.d.ts#L416

I think I'm wrong about the complexity here. Most HOCs are props => props, and all the extra arguments are in the outer functions. Later today I'll try out recompose with redux compose and see what happens.

@jasonkuhrt
Copy link

jasonkuhrt commented Aug 1, 2018

TypeScript 3.0 should help with some of its new features. For example variadic parameter typing (as opposed to just having static overloaded typing before).

@squidfunk
Copy link

Thanks for the explanation! I will subscribe to the issue and hope that it get's fixed ;-)

@monapasan
Copy link

@threehams Any update on this? Have you tried it with redux compose?

@threehams
Copy link

Tried and failed, sorry. The good news is that React hooks are dead simple to type.

@monapasan
Copy link

@threehams You are right. I guess once the api of hooks will be stable, recompose will become obsolete.

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

No branches or pull requests