React-apollo: New TypeScript definitions broken + cause critical slowdowns of the language server

Created on 16 Jun 2017  路  19Comments  路  Source: apollographql/react-apollo

Opening this issue from per the request of @jbaxleyiii in https://github.com/apollographql/react-apollo/pull/695#issuecomment-307812329

The Gist

My personal issue in detail can be found in #695. In short, here's the issues I'm experiencing:

1. The nested deeply-dependent type definitions are causing Visual Studio Code to freeze on any file that contains the graphql function provided by this library.

  • I'm running a Linux machine with a 4-year-old intel i7 quad-core processor and that's still not enough to make VSCode not be completely sluggish.*
  • When I "click to definition" on the graphql function, my editor crashes completely.

* Machine specifics: Linux archlinux 4.11.3-1-ARCH #1 SMP PREEMPT Sun May 28 10:40:17 CEST 2017 x86_64 GNU/Linux

2. Types will not compile even after adjusting the code in my application.

3. The tests for the new type definitions are being tested against components that all have the any type for both Props and State, which seems dangerous.


[Click me] Literally every query test case is affected by this potentially dangerous type casting.

Suggestions

Aside from fixing the compile issues and the any type casting described above, perhaps there might be a way to make the type dependency chain not so long? This singular problem I think is the issue for users (like me) who depend on realtime type checking using the typescript language server. (This is a standard feature in Visual Studio Code).

Aside: From the images that @jbaxleyiii posted in his pull-request, it _appears_ that he's also using VSCode. I'm wondering why these issues are not also affecting him? I wonder what type of machine he's running?

Steps to Reproduce

(This should be the easiest thing to reproduce from the list of issues I described above).

  1. Open https://github.com/apollographql/react-apollo/blob/master/test/react-web/client/graphql/queries/api.test.tsx in this repository in Visual Studio Code. Does this compile for you?

Version

Final Notes

I'm happy to help in any way I can. Regardless of these issues, you all deserve a huge high-five for such amazing work on the apollo platform. It seriously has made my API development experience much more enjoyable! Thanks for all you and the rest of apollo team do! 馃槃 馃憦

picture dsifford
👍12

Most helpful comment

I've been able to use the types without loss of information (but have seen the TS server slowdown mentioned in VS code) without casting to any through the following:

Some helper file:

import { QueryProps as QP } from 'react-apollo';
import { ApolloQueryResult, MutationQueryReducersMap } from 'react-apollo';
import { DataProxy } from 'apollo-client/data/proxy';

interface MutationOpts<T> {
    variables?: Object;
    optimisticResponse?: Object | Function;
    updateQueries?: MutationQueryReducersMap;
    refetchQueries?: string[] | /*PureQueryOptions*/any[];
    update?: (proxy: DataProxy, data: { data: T }) => void;
}
export type MutationFunc<T> = (options: MutationOpts<T>) => Promise<ApolloQueryResult<T>>;
export type QueryProps<P, R> = P & {
    data: QP & R;
}

and in my components:

interface MyResults {
  currentUser: IServerUserResponse;
}
interface MyProps {
  exposedProp: boolean;
}
interface MyMutations {
  login: MutationFunc<IServerUserResponse>;
}
interface MyState {
  searchQuery: string;
}
type MyPropsType = QueryProps<MyProps & MyMutations, MyResults>;
export default graphql<MyResults, MyProps>(query, opts)(class ApolloComponent extends React.Component<MyPropsType, MyState> {
...
})

Where IServerUserResponse is an interface describing your GraphQL response for something like a user object as specified by your schema. This method returns a component exposing only MyProps to be used like:

import ApolloComponent from './component';
import * as React from 'react';
React.render(
  <ApolloComponent exposedProp />
);

where omitting exposedProp will trigger an error unless it's optional since it is a member of MyProps

Note that if you were using Apollo's compose you need to use MyPropsType until the last one to get the desired types:

export default compose(
  graphql<MyResults, MyProps>(query, opts),
  graphql<MyResults, MyPropsType>(mutationQuery, { name: "login" })
)(class ApolloComponent extends React.Component<MyPropsType, MyState> {... })

I will note that clicking to definition on the graphql function does not cause my editor to crash but there is a large delay in loading the definition and that my editor hangs frequently using these generics. I'm not sure its necessarily the fault of the definition file (it is just using language features) however dealing with these types and constantly trying to get types to compile was a long painful nightmare but now I have something that reliably works as it's supposed to, at the very least. I really wish there was a way I did not have to explicitly annotate the types of the graphql<> function and instead they were inferred; I suppose that's a language shortcoming, though.

picture alexzielenski on 18 Jun 2017
👍6

All 19 comments

I've been able to use the types without loss of information (but have seen the TS server slowdown mentioned in VS code) without casting to any through the following:

Some helper file:

import { QueryProps as QP } from 'react-apollo';
import { ApolloQueryResult, MutationQueryReducersMap } from 'react-apollo';
import { DataProxy } from 'apollo-client/data/proxy';

interface MutationOpts<T> {
    variables?: Object;
    optimisticResponse?: Object | Function;
    updateQueries?: MutationQueryReducersMap;
    refetchQueries?: string[] | /*PureQueryOptions*/any[];
    update?: (proxy: DataProxy, data: { data: T }) => void;
}
export type MutationFunc<T> = (options: MutationOpts<T>) => Promise<ApolloQueryResult<T>>;
export type QueryProps<P, R> = P & {
    data: QP & R;
}

and in my components:

interface MyResults {
  currentUser: IServerUserResponse;
}
interface MyProps {
  exposedProp: boolean;
}
interface MyMutations {
  login: MutationFunc<IServerUserResponse>;
}
interface MyState {
  searchQuery: string;
}
type MyPropsType = QueryProps<MyProps & MyMutations, MyResults>;
export default graphql<MyResults, MyProps>(query, opts)(class ApolloComponent extends React.Component<MyPropsType, MyState> {
...
})

Where IServerUserResponse is an interface describing your GraphQL response for something like a user object as specified by your schema. This method returns a component exposing only MyProps to be used like:

import ApolloComponent from './component';
import * as React from 'react';
React.render(
  <ApolloComponent exposedProp />
);

where omitting exposedProp will trigger an error unless it's optional since it is a member of MyProps

Note that if you were using Apollo's compose you need to use MyPropsType until the last one to get the desired types:

export default compose(
  graphql<MyResults, MyProps>(query, opts),
  graphql<MyResults, MyPropsType>(mutationQuery, { name: "login" })
)(class ApolloComponent extends React.Component<MyPropsType, MyState> {... })

I will note that clicking to definition on the graphql function does not cause my editor to crash but there is a large delay in loading the definition and that my editor hangs frequently using these generics. I'm not sure its necessarily the fault of the definition file (it is just using language features) however dealing with these types and constantly trying to get types to compile was a long painful nightmare but now I have something that reliably works as it's supposed to, at the very least. I really wish there was a way I did not have to explicitly annotate the types of the graphql<> function and instead they were inferred; I suppose that's a language shortcoming, though.

alexzielenski picture alexzielenski on 18 Jun 2017
👍6

Any updates on this @helfer @stubailo @jbaxleyiii? Still stuck pre 1.4.0 because of this.

Also, I noticed that one of your tests has a typo in it that _should_ render the test broken. This leads me to believe that these tests aren't actually being ran?

https://github.com/apollographql/react-apollo/blob/master/test/react-web/client/graphql/queries/skip.test.tsx#L101

Here's the typo from the link above:

- class Container extends React.Component<any, any> {8
+ class Container extends React.Component<any, any> {
dsifford picture dsifford on 2 Jul 2017
👍1

I'll also note that the current definitions do not include the update mutation option.

alexzielenski picture alexzielenski on 2 Jul 2017

I'll also note that the current definitions do not include the update mutation option.

Am I correct in assuming that this is the reason for #769 and #810?
Getting those typings in order really would be a big step forward.

skosch picture skosch on 3 Jul 2017
👍1

@skosch I don't think so. Those issues seem to be stemmed from incomplete typings and improper explicit types. A lot of our headaches could be solved if the typescript language server had stronger type inference built into it but unfortunately we need to keep annotating the types for invocations of the graphql function in typescript (atleast in my case) to avoid compiler errors.

alexzielenski picture alexzielenski on 3 Jul 2017

Hi, I think the editor problems are strictly not related to react-apollo, but to the experimental decorator feature. I had problems using other decorators from other libs, and switching the syntax from decorator to normal curry solves any perf problem, both with react-apollo and with other decorators. I can reproduce this quite reliably

itajaja picture itajaja on 6 Jul 2017

@dsifford I've started working on some of this now! I'm not 100% how to get around the crashing? Maybe by moving the types to a different file and importing them? I can give that a try and see if it helps!

I'm currently working on the react typings bug that is preventing people from using the latest! I'll keep you posted on progress!

jbaxleyiii picture jbaxleyiii on 18 Jul 2017

@jbaxleyiii 馃檹 You are awesome. Many thanks for all your hard work!

If there's anything you'd like me to assist with, let me know and I'd be happy to help where needed / able.

dsifford picture dsifford on 18 Jul 2017

@jbaxleyiii I finally got around to looking at both your changes and the associated blog post you wrote. Thanks for taking the time to do that.

Unfortunately, it is looking like your changes will still not work for me.

Your blog post describes in great detail on how to apply the changes to stateless functional components. However, it makes no mention on how to apply these changes to stateful component classes (i.e. Component and PureComponent).

There are some areas in my application where I cannot use functional components as the components carry state. In these circumstances, no matter what I do I get the following errors...

Without any type casting (@graphql())

Argument of type 'typeof CardListContainer' is not assignable to parameter of type 'CompositeComponent<{ data?: QueryProps | undefined; mutate?: MutationFunc<{}> | undefined; }>'.
  Type 'typeof CardListContainer' is not assignable to type 'StatelessComponent<{ data?: QueryProps | undefined; mutate?: MutationFunc<{}> | undefined; }>'.
    Type 'typeof CardListContainer' provides no match for the signature '(props: { data?: QueryProps | undefined; mutate?: MutationFunc<{}> | undefined; } & { children?: ReactNode; }, context?: any): ReactElement<any> | null'.

Single any cast (@graphql<any>())

Unable to resolve signature of class decorator when called as an expression.
  Type 'ComponentClass<{}>' is not assignable to type 'typeof CardListContainer'.
    Type 'Component<{}, ComponentState>' is not assignable to type 'CardListContainer'.
      Property 'handleChange' is missing in type 'Component<{}, ComponentState>'.

Double any cast (@graphql<any, any>())

Unable to resolve signature of class decorator when called as an expression.
  Type 'ComponentClass<{}>' is not assignable to type 'typeof CardListContainer'.
    Type 'Component<{}, ComponentState>' is not assignable to type 'CardListContainer'.
      Property 'handleChange' is missing in type 'Component<{}, ComponentState>'.

Triple any cast (@graphql<any, any, any>())

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

I've decided to just stub out these type definitions because of the hassle of using them in my application. Happy to still help improve them, but right now they're just not usable in their current form (it's much easier to just declaratively add in the apollo props to the Props interface given to the component class generic, and IMHO, that way is strong enough for me to be able to catch issues).

dsifford picture dsifford on 29 Jul 2017

@dsifford I'm so sorry to hear this! What about the class methods listed here http://dev.apollodata.com/react/using-with-types.html#classes-vs-functions

Does that help?

jbaxleyiii picture jbaxleyiii on 29 Jul 2017

@jbaxleyiii Thanks for the response. The docs you've linked to provide further clarification. I'm pretty sure that way will work.

However, I'm going to still continue stubbing the types because I still don't see how casting merged props from the decorator into the component is any better than just merging the props and explicitly setting Props on the component class.

There very well might be situations that your way provides a clear benefit, but for my application, the API is fairly simple and doesn't require all the extra complex sugar.

Thanks again for taking the time to respond.

dsifford picture dsifford on 29 Jul 2017

I'm currently running into the same issue with slowdowns of the language server. Is there anything I can do to work around?

domachine picture domachine on 9 Aug 2017

Slowdowns are happening for me too in Sublime Text. Today was the first day it crashed on me!

Not only is it slow, but the example from the documentation doesn't seem to be working. I get the following errors:

components/profile/my-groups.tsx(23,41): error TS2459: Type '(QueryProps & Response) | undefined' has no property 'loading' and no string index signature.
[0] components/profile/my-groups.tsx(23,50): error TS2459: Type '(QueryProps & Response) | undefined' has no property 'error' and no string index signature.
[0] components/profile/my-groups.tsx(23,57): error TS2459: Type '(QueryProps & Response) | undefined' has no property 'self' and no string index signature.

Using the following code:


Click me to see the code

import * as React from 'react'
import gql from 'graphql-tag'
import { graphql } from 'react-apollo'


export const GROUPS_QUERY = gql`
  query self {
    groups {
      name
      name_id
    }
  }
`
type MemberedGroup = {
  name: string,
  name_id: string
}
type Response = { self: { groups: MemberedGroup[] } }


const withGroupData = graphql<Response>(GROUPS_QUERY, {})

export default withGroupData(({ data: { loading, error, self } }) => {
  if (loading) return <div>Loading...</div>
  if (error) return <h2>ERROR!</h2>

  return <div className="my-groups"></div>
})

.

and using the following package versions:

  • apollo-client: 1.9.1
  • react-apollo: 1.4.14
  • typescript: 2.3.4
  • react: 15.6.1
gilbert picture gilbert on 15 Aug 2017

I have some tsserver logs related to the slowdown bug. Three lines, 22 megabytes 馃槷. Summary:

Info 251  request: {"command":"quickinfo-full","seq":"2011","arguments":{"file":"/Users/nmuth/Projects/fulgid/v2.fulgid.io/pages/post/details.js","line":42,"offset":17}}
Perf 252  2011::quickinfo-full: elapsed time (in milliseconds) 17022.3773
Info 253  response: (22MB of JSON)

Here's the full file on Google Drive. Let me know if you can make any sense of it.

And here's the source file as a gist.

beepboopitschloe picture beepboopitschloe on 15 Aug 2017

This issue has been automatically labled because it has not had recent activity. If you have not received a response from anyone, please mention the repository maintainer (most likely @jbaxleyiii). It will be closed if no further activity occurs. Thank you for your contributions to React Apollo!

stale[bot] picture stale[bot] on 5 Sep 2017

+1 here

Techbinator picture Techbinator on 6 Sep 2017

This issue has been automatically labled because it has not had recent activity. If you have not received a response from anyone, please mention the repository maintainer (most likely @jbaxleyiii). It will be closed if no further activity occurs. Thank you for your contributions to React Apollo!

stale[bot] picture stale[bot] on 27 Sep 2017

This issue has been automatically closed because it has not had recent activity after being marked as no recent activyt. If you belive this issue is still a problem or should be reopened, please reopen it! Thank you for your contributions to React Apollo!

stale[bot] picture stale[bot] on 11 Oct 2017

@alexzielenski
You are the only one who gave a ready-made example ... In other places there is not even a hint of difference from
state +props
and
gql`` + options: {variables: {identityId: 'user-id'}}
Now I understand how this works, thanks.
graphql

bitrix1 picture bitrix1 on 10 Mar 2018
Was this page helpful?
0 / 5 - 0 ratings

Related issues

dmitryyankowski picture dmitryyankowski  路  3Comments
thisbejim picture thisbejim  路  3Comments
notadamking picture notadamking  路  3Comments
tuananhtd picture tuananhtd  路  3Comments
tomitrescak picture tomitrescak  路  3Comments