React-apollo: Typescript typing for graphql HOC with variables not used in component

Created on 14 Feb 2019  路  8Comments  路  Source: apollographql/react-apollo

Intended outcome:

I do like to split the UI component and the data. That is why when I use react-apollo, I prefer HOC :

ClientForm.component.tsx

export interface ClientFormProps {
  client?: Client;
}

export class ClientForm extends PureComponent<ClientFormProps> {
  public render(): ReactNode {
    return ...
  }
}

ClientForm.container.tsx

export interface Data {
  client?: Client;
}
export interface Variables {
  id: string;
}

export type ContainerProps = Omit<Variables & ClientFormProps, keyof Data>

export default graphql(..., {
  options: (props: ContainerProps): QueryOpts<Variables> => ({
    variables: {
      id: props.id,
    },
  }),
  props: (props: OptionsProps<ContainerProps, Data, Variables>): Data): ({
    client: props.data && props.data.client,
  })
})(ClientForm)

The component is re-usable without the data, if we provide a client props.
And the container is usable directly with an id props.

But this way I have a typescript error :
Because I use id in the options of the container, they expect id to be in ComponentProps.
There is for me no reason to write

export interface ClientFormProps {
  client?: Client;
  id: string;
}

Cause both and are valid.

Actual outcome:

Property "id" is missing in Readonly but required in ...

How to reproduce the issue:
I have a minimal reproduction this way

ClientForm.component.tsx

export type ClientFormProps = DataProps<any>

export class ClientForm extends PureComponent<ClientFormProps> {
  public render(): ReactNode {
    return ...
  }
}

ClientForm.container.tsx

export interface Variables {
  id: string;
}

export default graphql(..., {
  options: (props: Variables & DataProps<any>): QueryOpts<Variables> => ({
    variables: {
      id: props.id,
    },
  }),
})(ClientForm)

Solution
I think the problem is in this type :

export declare function graphql<TProps extends TGraphQLVariables | {} = {}, TData = {}, TGraphQLVariables = {}, TChildProps = Partial<DataProps<TData, TGraphQLVariables>> & Partial<MutateProps<TData, TGraphQLVariables>>, U = TChildProps & TProps>(document: DocumentNode, operationOptions?: OperationOption<TProps, TData, TGraphQLVariables, TChildProps>): (WrappedComponent: React.ComponentType<TChildProps & TProps>) => React.ComponentClass<TProps, any>;

More precisely here :

(WrappedComponent: React.ComponentType<TChildProps & TProps>)

This solve my issue

(WrappedComponent: React.ComponentType<TChildProps & Partial<TProps>>)

But create another for component which are really using TProps.

This can work

(WrappedComponent: React.ComponentType<TChildProps & Partial<TProps>> | React.ComponentType<TChildProps &TProps>)

But there will be issue if someone use two variables in the container, and only one in the component.

But in fact, it should be

(WrappedComponent: Every React.ComponentType with atleast TChildProps and at most TChildProps & TProps

How to do this ?

Version

  • react-apollo@<2.4.1>

All 8 comments

Issues here are reserved for bugs, but one of the following resources should help:

@danilobuerger Hi !
Maybe you were mistaken by the "How to do this" question. I was looking for help to do the PR.

The typing is not correct because it does not cover all possible usecases.
And I call this a bug because some good use of the graphql hoc return compilation errors.

Personally, I don't think its a bug. TChildProps & TProps are being passed to the WrappedComponent so the typings reflect that. I don't think the issue tracker, reserved for bugs only, is the right forum to discuss this. However, PRs are always welcome. So if you need guidance best ask in Spectrum or open a PR with your solution.

@danilobuerger Hi, I can simplify the problem to show you there is something wrong with the typing.

The component props.

export interface ClientFormProps extends DataProps<any> {
  a: string;
}

The container.

export default graphql(loader('./getClient.graphql'))(ClientForm);

The use.

<ClientForm a="foo" />

The error.

Type error: Argument of type 'typeof ClientForm' is not assignable to parameter of type 'ComponentType<Partial<DataProps<{}, {}>> & Partial<MutateProps<{}, {}>>>'.
  Type 'typeof ClientForm' is not assignable to type 'ComponentClass<Partial<DataProps<{}, {}>> & Partial<MutateProps<{}, {}>>, any>'.
    Types of parameters 'props' and 'props' are incompatible.
      Property 'a' is missing in type 'Partial<DataProps<{}, {}>> & Partial<MutateProps<{}, {}>>' but required in type 'Readonly<ClientFormProps>'.  TS2345

    4 | import ClientForm from './ClientForm.component';
    5 | 
  > 6 | export default graphql(loader('./getClients.graphql'))(ClientForm);
      |                                                                     ^
    7 | 

I can't actually create a PR since I don't have the solution.
But this is a BUG cause it's not working in some case. That's the goal of the issue tracker.

Classic HOC typing are this way

<P extends InjectedProps>(component: React.ComponentType<P>) => React.ComponentType<Omit<P, keyof InjectedProps>>;

That's not the same as

(WrappedComponent: React.ComponentType<TChildProps & TProps>) => React.ComponentClass<TProps, any>;

@danilobuerger Hi ! I have another example.
Can you admit the typing is wrong and reopen the issue ? Thanks.

Intended outcome:

No error

Actual outcome:

Property a is missing in {} but required in Readonly<Props>

How to reproduce the issue:

export interface Props {
  a: string;
}

class Foo extends Component<Props> {
  ...
}

export default graphql(..., {
  props: (): {} => ({}),
})(Foo);

PS : Hi @rosskevin, I saw you're a collaborator for this project.
If you have some time to help me fixing the typing of the HOC, it would be great.

I'm also having the same issue. I think this needs to be reopened.

one year later... seems like the typings are still wrong for the graphql HOC 馃槥

Was this page helpful?
0 / 5 - 0 ratings