Skip to content
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

Question: codebase full of TS Pick<> #1904

Closed
elie222 opened this issue May 21, 2019 · 11 comments
Closed

Question: codebase full of TS Pick<> #1904

elie222 opened this issue May 21, 2019 · 11 comments
Assignees
Labels
plugins waiting-for-release Fixed/resolved, and waiting for the next stable release

Comments

@elie222
Copy link

elie222 commented May 21, 2019

I'm generating TS types for operations using:
https://graphql-code-generator.com/docs/plugins/typescript-operations

The generated code produces a lot of types with Pick. For example:

{ __typename?: 'Policy' } & Pick<
      Policy,
      '_id' | 'name' | 'createdAt'
    >

This is technically accurate, but a headache to fill the codebase with Pick when passing down properties.

What I find myself doing to fix this is: policy as Policy and assuming I have a full Policy. The downsides are that it doesn't provide full type safety and it's annoying having to add as Policy for each query.

Another idea is to just be selective about the data being passed down in my components, but that doesn't sound fun either.

One last idea that just came to mind is to turn the Pick into a type, and the generator could be updated to do this automatically.

@lukasluecke
Copy link

If you use fragments inside your queries that will generate matching types which you can use inside your code.

@dotansimha
Copy link
Owner

dotansimha commented May 28, 2019

Hi @elie222 ,

Doing policy as Policy is incorrect because it will cast the object into the full object as it delcared in the schema, and you lose the type-safety of the selection set.
You can always do Query['policy']['someField'] to get a nested type in TypeScript (and you can create type alias for it).

As @lukasluecke said, fragments are a good solution for separating GraphQL operations and selection-sets, and we respect this separation by creating a separate type for fragments.

We are thinking about putting the resolved type instead of Picking it in the future.

Keeping open because we might implement the suggestion I mentioned above.

@ozsay
Copy link

ozsay commented Jul 1, 2019

Hi @dotansimha,

We encounter a similar issue with passing the result of a useQuery function (hooks) to a FlatList item renderer. Since the type returned from the useQuery is private to the generated file (Pick of the base type), we can't use it anywhere. So that leaves us with using the base type, and ts raises errors in that case.

Do you have any inputs regarding this? maybe a simple workaround meanwhile?

@dotansimha
Copy link
Owner

@ozsay can you please share an example? I didn't understand how to reproduce it.
You can use an intermediate type to get access to the nested fields. For example: const MyType = MyQuery['user'].

We are working on an alternative for the Picks, but it will just replace the Pick with the actual primitive type.

@ozsay
Copy link

ozsay commented Jul 3, 2019

@dotansimha
for example

schema

type Query {
  users: [User!]!
}

type User {
  id: String!
  firstName: String!
  lastName: String!
}

query

const getUsers = gql`
  query Users {
    users {
      id
      firstName
     }
  }
`;

with this setup, the generated code looks like (with hooks):

import gql from 'graphql-tag';
import * as ReactApolloHooks from 'react-apollo-hooks';
export type Maybe<T> = T | null;
export type MaybePromise<T> = Promise<T> | T;
/** All built-in and custom scalars, mapped to their actual values */
export type Scalars = {
  ID: string;
  String: string;
  Boolean: boolean;
  Int: number;
  Float: number;
  Date: any;
};

export type User = {
  __typename?: 'User';
  id: Scalars['String'];
  firstName: Scalars['String'];
  lastName: Scalars['String'];
};

export type UsersQueryVariables = {};

export type UsersQuery = { __typename?: 'Query' } & {
  users: Array<
    { __typename?: 'User' } & Pick<
      User,
      'id' | 'firstName'
    >
  >;
};

export const UsersDocument = gql`
  query Users {
    users {
      id
      firstName
    }
  }
`;

export function useUsersQuery(baseOptions?: ReactApolloHooks.QueryHookOptions<UsersQueryVariables>) {
  return ReactApolloHooks.useQuery<UsersQuery, UsersQueryVariables>(
    UsersDocument,
    baseOptions,
  );
}

and the code that raises problems:

UsersList Component

function UsersList() {
  const { data: { users = [] } = {}, error, loading, refetch } = useUsersQuery(getUsers);

  const renderItem = ({ item } : { item: User }) => <UserComponent user={item} />;

  return (
    <Container>
      <FlatList
        data={users}
        renderItem={renderItem}
        refreshing={loading}
        onRefresh={refetch}
      />
    </Container>
  );
}

User Component

import React from 'react';

import { User } from './generated-code.tsx';

interface Props {
  user: User;
}

export function UserComponent({ user } : Props) {
  return ...
}

The problem is obvious here. the users array that is returned from the query is of type Pick<User, 'id' | 'firstName' while the FlatList is expecting User. Since the generated code doesn't export the pick type, i can't use it to replace User type.

@elie222
Copy link
Author

elie222 commented Jul 3, 2019 via email

@dotansimha
Copy link
Owner

As @elie222 said, @ozsay , you can use UsersQuery["users"] for your props. If you wish to make it more general, use GraphQL fragments and then you'll have UserFragment generated.

@dotansimha dotansimha self-assigned this Jul 4, 2019
@dotansimha
Copy link
Owner

dotansimha commented Jul 4, 2019

@elie222 @ozsay I tried it in: #2107

The output is:

export type MeQuery = {
  __typename?: 'Query';
  currentUser: Maybe<{ __typename?: 'User'; login: string; html_url: string }>;
  entry: Maybe<{
    __typename?: 'Entry';
    id: number;
    createdAt: number;
    postedBy: { __typename?: 'User'; login: string; html_url: string };
  }>;
};

What do you think? (I'll introduce a new config flag preResolveTypes to activate this behaviour).
So now it's not using Pick at all (it also loses the connection of the type to the complete GraphQL type, it's not a big deal because you can easily find it because of __typename).

@RIP21
Copy link
Contributor

RIP21 commented Jul 4, 2019

I'm with @dotansimha and @lukasluecke
You should use fragments for all your data requirements of the component. And you should build a query from the bottom of the component tree up to the place where you do a query. This way it will be nice and alongside.
Example:

# alongside Info component or even same file
fragment Info_User on User {
  firstName
  lastName
  imageUrl
  email
}

# alongside Description component or even same file
fragment Description_User on User {
  description
}

# alongside LatestPosts component or even same file
fragment LatestPosts_User on User {
  posts {
     title
     slug
  }
}

# alongside UserView component or even same file
query UserView_User($id: UUID! ) {
   user(id: $id) {
     ...Info_User
     ...Description_User
     ...LatestPosts_User
   }
}

This approach is recommended in Apollo and it close to the one that Relay is based off.
https://www.apollographql.com/docs/angular/features/fragments/
In Revolut we have following convention:

Fragments:

- Always create a fragment for the data that you will get.
  It will simplify passing typings down to the other pieces of your code.
  `SomeQuery['some']` will give you `Some | null` if `some` field is nullable on the query, or `Array<Some> | null`,
  in the worst case scenario.
  But most of the time, in your helper functions or, nested components, you already filtered `null` out,
  so you just need `Some`, so here fragments are coming to rescue. How to deal with them, look at the next point.
- Name it by the prop in which you will pass this data or the name of
  the component itself **(preferrable, since no HoCs in 2019)**.
  E.g. `Transactions_data` or `MessageBubble_SwiftMessage`. It will generate `Transaction_DataFragment`
  and `MessageBubble_SwiftMessageFragment` types for you, that will be pure
  types to be used in your utility functions of the feature, in your hooks args signatures etc.

@dotansimha dotansimha added the waiting-for-release Fixed/resolved, and waiting for the next stable release label Jul 4, 2019
@dotansimha
Copy link
Owner

Actually, there are 2 issues here:

  • Pick - With the change I added, you can now have a direct primitive instead of Pick<> (you'll still need to include typescript along with typescript-operations because of enums and input types).
  • Intermediate types - you can get them by using typescript-compatiblity and then each selection set will get it's own type (like in 0.18). And @RIP21 is right regarding fragments, this is a powerful feature in GraphQL, and using it right will make it easier to you so you won't need intermediate types at all).

@dotansimha
Copy link
Owner

Available in 1.4.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugins waiting-for-release Fixed/resolved, and waiting for the next stable release
Projects
None yet
Development

No branches or pull requests

5 participants