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

Component loses typings with HOC #379

Closed
brettjurgens opened this Issue Dec 21, 2016 · 4 comments

Comments

Projects
None yet
3 participants
@brettjurgens
Copy link
Contributor

brettjurgens commented Dec 21, 2016

Steps to Reproduce

Describe how to reproduce this issue.
component1.tsx:

import Query from './query.graphql';

interface IProps {
  foo: string;
  bar: number;
}
const Component: (props: IProps) => JSX.Element = ({ foo, bar }) => {
  // return JSX
}

export default @graphql(Query)(Component)

component2.tsx:

import Component from './component1';

const SimpleWrapper: (props: {}) => JSX.Element = () => (
  <Component foo={false} />
)

Buggy Behavior

The above will compile, because react-apollo has typings:

export default function graphql(document: Document, operationOptions?: OperationOption): (WrappedComponent: any) => any;

Expected Behavior

This shouldn't compile, because prop bar is missing and foo is of the wrong type.

I'm thinking that the graphql HOC can take generics like:

interface InjectedProps<T> = {
  data?: T;
  loading: boolean;
  // etc.
}

interface ComponentDecorator<OriginalProps, AllProps> {
  (component: React.ComponentClass<AllProps> | React.StatelessComponent<AllProps>): React.ComponentClass<OriginalProps>;
}

graphql<QueryProps, ComponentProps>(document: Document, operationOptions?: OperationOption): ComponentDecorator<ComponentProps, ComponentProps & InjectedProps<QueryProps>>

(did this pretty quickly, probably not 100% accurate, see react-redux typings)

Version

  • apollo-client@0.5.24
  • react-apollo@0.7.1

Notes

I searched the issues and didn't see anything about this.

I'd be willing to make a PR if such a thing would be acceptable

@tmeasday

This comment has been minimized.

Copy link
Contributor

tmeasday commented Dec 22, 2016

Sure, this sounds reasonable if you can make it work.

@brettjurgens

This comment has been minimized.

Copy link
Contributor Author

brettjurgens commented Dec 22, 2016

Cool, I'll take a look and open a PR

@brettjurgens

This comment has been minimized.

Copy link
Contributor Author

brettjurgens commented Dec 22, 2016

I've got it working with Stateless Components and regular classes, but it fails when using a decorator.

Been through a bunch of variations and can't seem to make it work properly.

export interface InjectedGraphQLProps<T> {
  data?: T;
  loading?: boolean;
}

export interface FixedComponentClass<P> extends ComponentClass<P> {
  new (props?: P, context?: any): React.Component<P, any>;
}

export type ComponentClass<P> = React.ComponentClass<P>;
export type StatelessComponent<P> = React.StatelessComponent<P>;

export interface WrapWithApollo<QueryProps, ComponentProps> {
  (
    WrappedComponent: (
      ComponentClass<ComponentProps & InjectedGraphQLProps<QueryProps>> |
        StatelessComponent<ComponentProps & InjectedGraphQLProps<QueryProps>> | any
    )
  ): (FixedComponentClass<ComponentProps>);
}

export default function graphql<QueryProps, ComponentProps>(/* args */) {
  const wrapWithApolloComponent: WrapWithApollo<QueryProps, ComponentProps> = WrappedComponent => {
     /* body */
  }
}

and this works with:

export interface ITest1Props {
  x: boolean;
}

export interface ITest1QueryProps {
  a: string;
  b: number;
}

export type IProps = ITest1Props & InjectedGraphQLProps<ITest1QueryProps>;

export class Test1 extends Component<IProps, {}> {
  componentWillReceiveProps(props) {
  }
  render (): JSX.Element { return <div>{this.props.data.a}</div>; }
}

export const Test1Out = graphql<ITest1QueryProps, ITest1Props>(null!)(Test1);

class Test1Wrapper extends Component<{}, {}> {
  componentWillReceiveProps(props) {
  }
  render (): JSX.Element {
    return <Test1Out x={true} />;
  }
}


@graphql<ITest1QueryProps, ITest1Props>(null!)
export class Test2 extends React.Component<IProps, {}> {
  render (): JSX.Element {
    return <div>{ this.props.data.a }</div>;
  }
}

class Test2Wrapper extends Component<{}, {}> {
  render (): JSX.Element {
    return <Test2 x={false} />;
  }
}

const Test3: (props: ITest1Props) => JSX.Element = (props: ITest1Props) => <div />;

const Test3Out = graphql<ITest1QueryProps, ITest1Props>(null!)(Test3);

class Test3Wrapper extends Component<{}, {}> {
  render (): JSX.Element {
    return <Test3Out x={false} />;
  }
}

but it fails with:

@graphql<any, any>(null!)
class TestFail extends React.Component<any, any> {
  componentWillReceiveProps(props) {
  }
  
  render() {
    return null;
  }
}

because

Unable to resolve signature of class decorator when called as an expression.
Type 'FixedComponentClass' is not assignable to type 'typeof TestFail'.
Type 'Component<any, any>' is not assignable to type 'TestFail'.
Property 'componentWillReceiveProps' is missing in type 'Component<any, any>'.

i'm sure something like https://github.com/DefinitelyTyped/DefinitelyTyped/blob/types-2.0/react-redux/index.d.ts#L26 is needed for the decorator, but that breaks it for the class/stateless component 😞

i'll keep looking, but any help would be appreciated

@brettjurgens

This comment has been minimized.

Copy link
Contributor Author

brettjurgens commented Dec 22, 2016

actually can just take the thing from react-redux and make the props from react-apollo optional and able to be generated with the InjectGraphQLProps interface

@brettjurgens brettjurgens referenced this issue Dec 22, 2016

Merged

add some typings to graphql HOC #383

6 of 6 tasks complete

@helfer helfer closed this in #383 Jan 31, 2017

@helfer helfer removed the help wanted label Jan 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment