Type definition of "withApollo" HOC seems wrong

[async3619]

Intended outcome:

First of all, I'd like to apologize my poor english skill. I'll have some time for fixing this poor-english-skill issue first. 😬

When we use withApollo HOC, it should return a new component type without client property at props type like below:

import { withApollo } from "react-apollo";

type Props = WithApolloClient<{}>;

class Foo extends React.Component<Props> {
    public render() {
        return null;
    }
}

const Wrapped = withApollo(Foo); // <-- this component shouldn't need `client` prop now.

const test = <Wrapped />; // <-- this shows no error at all as how I expected.

Actual outcome:

But above code snippet with latest apollo release won't exclude client prop in original props type.

import { withApollo } from "react-apollo";

type Props = WithApolloClient<{}>;

class Foo extends React.Component<Props> {
    public render() {
        return null;
    }
}

const Wrapped = withApollo(Foo);
//    ^^^^^^^
// type of `Wrapped` is now `React.ComponentType<Pick<unknown, never>, any>`.

const test = <Wrapped />; 
//    ^^^^
// this code won't make errors but it treated like a component with `any` as prop type.

I thought it's just an error just because there's no properties at generic argument of WithApolloClient, but It made an error at withApollo(Foo).

I checked type definition of withApollo HOC and I found a problem:

function withApollo<TProps, TResult = any>(
    WrappedComponent: React.ComponentType<WithApolloClient<Omit<TProps, "client">>>,
//                                         ^^^^^^^^^^^^^^^^ (1)
    operationOptions?: OperationOption<TProps, TResult>,
): React.ComponentClass<Omit<TProps, 'client'>>;
//                                   ^^^^^^^^ (2)

Firstly, see (1). We're already expecting that generic TProps should have client property. so I think we shouldn't use WithApolloClient as prop type of WrappedComponent parameter since TProps would have it.

I don't get why we omit client property of TProps even if we'll add it again with WithApolloClient. Are there any reasons?

and see (2). we don't know whether if generic TProps would have client property or not since TProps isn't extending anything. It can accept literally any types and I think that's why prop type of returned component will have unknown.

so I think the approach of this issue would be like this:

function withApollo<TProps extends WithApolloClient<any>, TResult = any>(
    WrappedComponent: React.ComponentType<TProps>,
    operationOptions?: OperationOption<TProps, TResult>,
): React.ComponentClass<Omit<TProps, "client">>;

with above definition, we enforce TProps generic having client property as required one. and just remove it from returned component type. I'm not sure above approach is perfect or not but I believe current one is wrong.

How to reproduce the issue:

Version

  System:
    OS: Windows 10
  Binaries:
    Node: 10.15.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.17.3 - C:\Users\User\AppData\Roaming\npm\yarn.CMD
    npm: 6.9.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: 42.17134.1.0
  npmPackages:
    apollo-cache: ^1.3.2 => 1.3.2
    apollo-cache-inmemory: ^1.6.2 => 1.6.2
    apollo-client: ^2.6.4 => 2.6.4
    apollo-link: ^1.2.12 => 1.2.12
    apollo-link-error: ^1.1.11 => 1.1.11
    apollo-link-http: ^1.5.15 => 1.5.15
    apollo-link-logger: ^1.2.3 => 1.2.3
    apollo-link-schema: ^1.2.3 => 1.2.3
    apollo-server: ^2.6.3 => 2.7.2
    apollo-server-express: ^2.6.3 => 2.7.2
    apollo-utilities: ^1.3.2 => 1.3.2
    react-apollo: ^3.0.1 => 3.0.1

FYI, I'm using typescript 3.5.3 now.

[arieslx]

I got this error too, can u share your solution

the problem in my project is type error，so I try to

class MyMeeting xxx

export default withApollo(MyMeeting as any)

not elegantly but usefull

[async3619]

@arieslx I've redeclared whole withApollo function definition like this:

import React from "react";
import { OperationOption, withApollo as withApolloImpl } from "react-apollo";

import { ApolloClient } from "apollo-client";

export declare type WithApolloClient<P> = P & {
    client: ApolloClient<any>;
};

export function withApollo<TProps extends WithApolloClient<{}>, TResult = any>(
    WrappedComponent: React.ComponentType<TProps>,
    operationOptions?: OperationOption<TProps, TResult>,
): React.ComponentClass<Omit<TProps, "client">> {
    return withApolloImpl(WrappedComponent as any, operationOptions);
}

[hwillson]

If anyone wants to submit a PR that fixes this, we'll definitely review it - thanks!

[async3619]

I don't want to put the blame on anybody but my issue opened almost 6 months ago. I'm really shameful about I don't know how to open pull request even if I'm using git like 2 years.

I don't think this is a big problem/issue (or bug) that break something that working with this module but I think we should care about type errors since this module written on typescript.

Funnily enough, I don't even know whether if this error still occurring with latest typescript version or not. 😂 and I respect all the people who contributed to the project, and I didn't write this comment aggressively either. I hope you guys don't misunderstand my purpose.

...so any updates on this issue?

[blackmagic0]

So I had been following this issue for a while, I managed to work around it by doing the following:

import React from "react";

interface MyProps {
    something: boolean;
}

const MyComponent = ( props : WithApolloClient<MyProps> ) => {
    const x = client.query( ... );
    return ...
}

export default withApollo<MyProps>( MyComponent );

I have since moved on from this ugly approach to just using the hook:

import React from "react";
import { useApolloClient } from "react-apollo";

interface MyProps {
    something: boolean;
}

const MyComponent = ( props : MyProps ) => {
    const client = useApolloClient();
    const x = client.query( ... );
    return ...
}

export default MyComponent;