acdlite / recompose

A React utility belt for function components and higher-order components.
MIT License
14.76k stars 1.26k forks source link

Typings for recompose #231

Open pablobirukov opened 8 years ago

pablobirukov commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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

pablobirukov commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

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

https://github.com/DefinitelyTyped/DefinitelyTyped/compare/master...farism:master

ianks commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

I also just saw https://github.com/acdlite/recompose/pull/241 opened two days ago, things are moving fast! Maybe we can get @gcanti in on this discussion?

pablobirukov commented 8 years ago

@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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

threehams commented 7 years ago

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: https://github.com/Microsoft/TypeScript/issues/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: https://github.com/Microsoft/TypeScript/issues/12215#issuecomment-306022583

This feels like it's getting really close.

istarkov commented 7 years ago

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

jzimmek commented 6 years ago

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 commented 6 years ago

I think it’s working properly. To get the behavior you’re looking for, use the second template parameter of compose- e.g. compose<{}, {name:...}>(...)

-ARO

On Sep 28, 2017, at 1:32 PM, Jan Zimmek notifications@github.com wrote:

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 (

name: {props.name}, age: {props.age}

); };

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

const x = renderToString();

console.info("===>", x); This works as expected and outputs:

===>

name: joe, age: 99
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(); 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"

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

jzimmek commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

FWIW the compose in redux has pretty good typing

threehams commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

monapasan commented 5 years ago

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

threehams commented 5 years ago

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

monapasan commented 5 years ago

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