Type definition of "withApollo" HOC seems wrong #3392
Description
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.